[PR #177] [MERGED] fix(snapshot): parallelize OSS layer uploads and skip publish-time re-hash #229

New issue

Closed

opened 2026-10-29 14:21:15 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-29 14:21:15 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/kvcache-ai/AgentENV/pull/177
Author: @huajq
Created: 8/17/2026
Status: ✅ Merged
Merged: 8/18/2026
Merged by: @yingdi-shan

Base: main ← Head: feat/oss-upload-throughput

📝 Commits (1)

2007bdb fix(snapshot): parallelize OSS layer uploads and skip publish-time re-hash

📊 Changes

7 files changed (+246 additions, -25 deletions)

View changed files

📝 src/sandbox/firecracker/overlaybd_snapshot.rs (+17 -10)
📝 src/sandbox/firecracker/sandbox.rs (+3 -2)
📝 src/sandbox/ublk/mod.rs (+1 -1)
📝 src/sandbox/ublk/overlaybd.rs (+37 -10)
📝 src/snapshot/repository/backends/oss/client.rs (+15 -2)
➕ storage/overlaybd/src/lsmt/file/digest_tracking.rs (+171 -0)
📝 storage/overlaybd/src/lsmt/file/mod.rs (+2 -0)

📄 Description

What

Speed up OSS snapshot layer uploads and remove a redundant full-file hash from the publish path:

Upload layers with an explicit 64 MiB multipart part size and up to 8 concurrent parts per file (previously the default writer: service-minimum part size, strictly serial).
Attach sha256/size descriptors to freshly compacted layers at write time (streaming, zero extra I/O) so publish can register them by descriptor instead of re-reading the whole layer to hash it.

Why

Production snapshot publishes of large sandboxes could not finish within client deadlines: single-stream multipart uploads to the OSS internal endpoint top out at ~100 MB/s, so a ~215 GiB rootfs layer needs ~37 min of pure upload, plus a full 215 GiB re-read for hashing before every upload. Retrying on every 30 min client timeout made the failure deterministic. Measured against an isolated OSS validation bucket with an 8 GiB object: serial default 64.6 MiB/s, serial 64 MiB chunk 80.8 MiB/s, 64 MiB chunk x8 concurrent 697-800 MiB/s (~10x).

N/A (no tracking issue; root-caused from production incident analysis)

Scope and non-goals

Included:

oss/client.rs: 64 MiB CHUNK_SIZE via writer_with().chunk(), UPLOAD_CONCURRENCY=8.
create_commit_args_with_digest() + compact_layers / memory-snapshot layer builder: record the descriptor in the staged image.json lowers (rootfs compaction and memory layer), so publish uses import_managed_layer_with_descriptor (HEAD existence check).
lsmt/file/digest_tracking.rs (new): DigestTrackingFile (file-order streaming sha256 over write_at) and OrderedWriter (forces ordered compaction writes), exported via overlaybd::index_file.

Excluded (follow-ups, intentionally not here):

Moving the >32-layer runtime-suffix compaction out of the snapshot hot path.
Cancellation-safe publish on client disconnect; memory-layer compression; Firecracker discard support.

Design and behavior changes

Multipart part size is now explicit (64 MiB), keeping a single object under the 10,000-part S3 limit up to ~625 GiB; parts upload up to 8-way concurrently (bounded by opendal's concurrent task queue; worst-case extra memory is concurrency x part size per in-flight upload).
Digest correctness: DigestTrackingFile hashes only strictly in-order writes; any out-of-order write poisons the tracker and yields no descriptor, in which case publish falls back to the previous explicit file hash. OrderedWriter forces compact_to's ordered branch (reads stay 32-way concurrent, writes serialize) so raw outputs also hash correctly. The zfile path required file-level (not CompactWriter-level) hashing because CompactWriter sees pre-compression bytes.
Staged image.json lowers for locally produced layers now carry non-empty digest/size when they could be computed inline; previously always empty for these entries.

Compatibility and operations

Public API or generated protocol: N/A (no API/OpenAPI changes).
Configuration or defaults: N/A (part size and concurrency are compile-time constants).
Snapshot manifest, artifact layout, or storage format: staged image.json lower entries gain digest/size values; consumers already tolerate them (descriptor-aware import path existed and is used for restacked layers today). OSS object keys remain content-addressed by the same sha256 convention.
Upgrade and rollback: safe in either direction; descriptor absence only means the old explicit-hash path is used. Rollback loses the upload speedup only.
Host requirements, permissions, ports, or dependencies: N/A.

Validation

make fmt (cargo fmt --all -- --check)
make clippy (cargo clippy -p overlaybd -p agentenv --all-targets -- -D warnings)
make test-unit equivalent: cargo test -p overlaybd --lib; cargo test -p agentenv --lib (see notes)
Relevant Rust integration tests (not run: require KVM/ublk host changes beyond this patch)
Benchmarks or performance comparison completed

Commands and results:

cargo test -p overlaybd --lib
  → 310 passed; 0 failed
cargo test -p agentenv --lib compact_layers
  → 1 passed (raw/lz4/zstd digest == sha256(output file))
cargo test -p agentenv --lib overlaybd_snapshot
  → 9 passed; 0 failed
cargo clippy -p overlaybd -p agentenv --all-targets -- -D warnings
  → clean
cargo test -p agentenv --lib
  → 723 passed; 2 failed (both pre-existing environment issues:
     privileges capability test, acr docker-credential-helper test;
     unrelated to this change)

Upload benchmark (temporary harness, not committed; isolated OSS
validation bucket, 8 GiB object):
  writer() default                 64.6 MiB/s
  writer_with().chunk(64MiB)       80.8 MiB/s
  + .concurrent(8)                799.8 / 697.6 MiB/s (two runs)

Skipped checks and reasons:

make -C services test: no services/ changes.
Generated clients/server: no generated-code changes.
Documentation: no user-facing docs affected.

Risks and reviewer notes

Multipart part-size change affects only new uploads; in-flight or previously uploaded objects are unaffected. Concurrent part uploads use more memory (<= 8 x 64 MiB in-flight per upload).
Ordered-write forcing removes out-of-order concurrent chunk writes for the digest-enabled compaction paths; reads remain parallel, and unordered behavior elsewhere is unchanged.
Digest poisoning path is exercised implicitly by falling back to explicit hashing; reviewers may want to double-check lsmt/file/digest_tracking.rs absorb-order logic and the zfile write-order assumption (validated by the lz4/zstd compact_layers test).
local_layer_config in overlaybd_snapshot.rs is now test-only (#[cfg(test)]) since all production call sites use the descriptor variant.

Checklist

The PR contains one coherent change and no unrelated formatting or refactoring.
New behavior is covered by tests, or I explained why testing is impractical.
Logs and examples contain no credentials, tokens, or private registry information.
I did not manually edit generated code without updating its source and regenerating it.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/kvcache-ai/AgentENV/pull/177 **Author:** [@huajq](https://github.com/huajq) **Created:** 8/17/2026 **Status:** ✅ Merged **Merged:** 8/18/2026 **Merged by:** [@yingdi-shan](https://github.com/yingdi-shan) **Base:** `main` ← **Head:** `feat/oss-upload-throughput` --- ### 📝 Commits (1) - [`2007bdb`](https://github.com/kvcache-ai/AgentENV/commit/2007bdbad20dc0cc81bca469363706da5eeb6a6e) fix(snapshot): parallelize OSS layer uploads and skip publish-time re-hash ### 📊 Changes **7 files changed** (+246 additions, -25 deletions) <details> <summary>View changed files</summary> 📝 `src/sandbox/firecracker/overlaybd_snapshot.rs` (+17 -10) 📝 `src/sandbox/firecracker/sandbox.rs` (+3 -2) 📝 `src/sandbox/ublk/mod.rs` (+1 -1) 📝 `src/sandbox/ublk/overlaybd.rs` (+37 -10) 📝 `src/snapshot/repository/backends/oss/client.rs` (+15 -2) ➕ `storage/overlaybd/src/lsmt/file/digest_tracking.rs` (+171 -0) 📝 `storage/overlaybd/src/lsmt/file/mod.rs` (+2 -0) </details> ### 📄 Description ## What Speed up OSS snapshot layer uploads and remove a redundant full-file hash from the publish path: - Upload layers with an explicit 64 MiB multipart part size and up to 8 concurrent parts per file (previously the default writer: service-minimum part size, strictly serial). - Attach sha256/size descriptors to freshly compacted layers at write time (streaming, zero extra I/O) so publish can register them by descriptor instead of re-reading the whole layer to hash it. ## Why Production snapshot publishes of large sandboxes could not finish within client deadlines: single-stream multipart uploads to the OSS internal endpoint top out at ~100 MB/s, so a ~215 GiB rootfs layer needs ~37 min of pure upload, plus a full 215 GiB re-read for hashing before every upload. Retrying on every 30 min client timeout made the failure deterministic. Measured against an isolated OSS validation bucket with an 8 GiB object: serial default 64.6 MiB/s, serial 64 MiB chunk 80.8 MiB/s, 64 MiB chunk x8 concurrent 697-800 MiB/s (~10x). ## Related issue N/A (no tracking issue; root-caused from production incident analysis) ## Scope and non-goals Included: - `oss/client.rs`: 64 MiB `CHUNK_SIZE` via `writer_with().chunk()`, `UPLOAD_CONCURRENCY=8`. - `create_commit_args_with_digest()` + `compact_layers` / memory-snapshot layer builder: record the descriptor in the staged `image.json` lowers (rootfs compaction and memory layer), so publish uses `import_managed_layer_with_descriptor` (HEAD existence check). - `lsmt/file/digest_tracking.rs` (new): `DigestTrackingFile` (file-order streaming sha256 over `write_at`) and `OrderedWriter` (forces ordered compaction writes), exported via `overlaybd::index_file`. Excluded (follow-ups, intentionally not here): - Moving the >32-layer runtime-suffix compaction out of the snapshot hot path. - Cancellation-safe publish on client disconnect; memory-layer compression; Firecracker discard support. ## Design and behavior changes - Multipart part size is now explicit (64 MiB), keeping a single object under the 10,000-part S3 limit up to ~625 GiB; parts upload up to 8-way concurrently (bounded by opendal's concurrent task queue; worst-case extra memory is concurrency x part size per in-flight upload). - Digest correctness: `DigestTrackingFile` hashes only strictly in-order writes; any out-of-order write poisons the tracker and yields no descriptor, in which case publish falls back to the previous explicit file hash. `OrderedWriter` forces compact_to's ordered branch (reads stay 32-way concurrent, writes serialize) so raw outputs also hash correctly. The zfile path required file-level (not CompactWriter-level) hashing because CompactWriter sees pre-compression bytes. - Staged `image.json` lowers for locally produced layers now carry non-empty `digest`/`size` when they could be computed inline; previously always empty for these entries. ## Compatibility and operations - Public API or generated protocol: N/A (no API/OpenAPI changes). - Configuration or defaults: N/A (part size and concurrency are compile-time constants). - Snapshot manifest, artifact layout, or storage format: staged `image.json` lower entries gain `digest`/`size` values; consumers already tolerate them (descriptor-aware import path existed and is used for restacked layers today). OSS object keys remain content-addressed by the same sha256 convention. - Upgrade and rollback: safe in either direction; descriptor absence only means the old explicit-hash path is used. Rollback loses the upload speedup only. - Host requirements, permissions, ports, or dependencies: N/A. ## Validation - [x] `make fmt` (cargo fmt --all -- --check) - [x] `make clippy` (cargo clippy -p overlaybd -p agentenv --all-targets -- -D warnings) - [x] `make test-unit` equivalent: cargo test -p overlaybd --lib; cargo test -p agentenv --lib (see notes) - [ ] Relevant Rust integration tests (not run: require KVM/ublk host changes beyond this patch) - [x] Benchmarks or performance comparison completed Commands and results: ```text cargo test -p overlaybd --lib → 310 passed; 0 failed cargo test -p agentenv --lib compact_layers → 1 passed (raw/lz4/zstd digest == sha256(output file)) cargo test -p agentenv --lib overlaybd_snapshot → 9 passed; 0 failed cargo clippy -p overlaybd -p agentenv --all-targets -- -D warnings → clean cargo test -p agentenv --lib → 723 passed; 2 failed (both pre-existing environment issues: privileges capability test, acr docker-credential-helper test; unrelated to this change) Upload benchmark (temporary harness, not committed; isolated OSS validation bucket, 8 GiB object): writer() default 64.6 MiB/s writer_with().chunk(64MiB) 80.8 MiB/s + .concurrent(8) 799.8 / 697.6 MiB/s (two runs) ``` Skipped checks and reasons: - `make -C services test`: no `services/` changes. - Generated clients/server: no generated-code changes. - Documentation: no user-facing docs affected. ## Risks and reviewer notes - Multipart part-size change affects only new uploads; in-flight or previously uploaded objects are unaffected. Concurrent part uploads use more memory (<= 8 x 64 MiB in-flight per upload). - Ordered-write forcing removes out-of-order concurrent chunk writes for the digest-enabled compaction paths; reads remain parallel, and unordered behavior elsewhere is unchanged. - Digest poisoning path is exercised implicitly by falling back to explicit hashing; reviewers may want to double-check `lsmt/file/digest_tracking.rs` absorb-order logic and the zfile write-order assumption (validated by the lz4/zstd compact_layers test). - `local_layer_config` in overlaybd_snapshot.rs is now test-only (`#[cfg(test)]`) since all production call sites use the descriptor variant. ## Checklist - [x] The PR contains one coherent change and no unrelated formatting or refactoring. - [x] New behavior is covered by tests, or I explained why testing is impractical. - [x] Logs and examples contain no credentials, tokens, or private registry information. - [x] I did not manually edit generated code without updating its source and regenerating it. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-29 14:21:15 +00:00