[PR #124] [MERGED] feat(overlaybd): move background download into the shared full-file cache #195

New issue

Closed

opened 2026-10-29 14:20:53 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-29 14:20:53 +00:00

Owner

📋 Pull Request Information

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

Base: main ← Head: bk_cache

📝 Commits (1)

8daf172 feat(overlaybd): move background download into the shared full-file cache

📊 Changes

21 files changed (+2937 additions, -1969 deletions)

View changed files

📝 CLAUDE.md (+1 -1)
📝 Cargo.lock (+1 -1)
📝 config/default.toml (+3 -3)
📝 config/oss_default.toml (+3 -3)
📝 docs/src/configuration/reference.md (+23 -11)
📝 src/cfg.rs (+23 -3)
📝 src/setup/deps.rs (+5 -0)
📝 storage/overlaybd/Cargo.toml (+0 -1)
➕ storage/overlaybd/src/backend/cache/bk_download.rs (+780 -0)
📝 storage/overlaybd/src/backend/cache/full_file_cache/cache_entry.rs (+37 -17)
📝 storage/overlaybd/src/backend/cache/full_file_cache/cache_pool.rs (+322 -96)
📝 storage/overlaybd/src/backend/cache/full_file_cache/cache_store.rs (+254 -70)
📝 storage/overlaybd/src/backend/cache/mod.rs (+3 -0)
📝 storage/overlaybd/src/backend/cache/tests.rs (+773 -1)
➖ storage/overlaybd/src/bk_download.rs (+0 -1561)
📝 storage/overlaybd/src/config.rs (+163 -10)
📝 storage/overlaybd/src/download_gate.rs (+84 -9)
📝 storage/overlaybd/src/image/image_file.rs (+266 -126)
📝 storage/overlaybd/src/image/image_service.rs (+130 -29)
📝 storage/overlaybd/src/lib.rs (+0 -2)

...and 1 more files

📄 Description

What

Move OverlayBD background download from a per-ImageFile downloader (private .download staging file, whole-layer overlaybd.commit promotion, SwitchFile swap) into one scheduler per FileCacheBackend that fills the shared full-file cache directly. Completed cache blocks become visible to foreground reads immediately, and foreground/background fetches of the same block are deduplicated by the per-block loader election.

Why

The old design fetched the same bytes twice (foreground page-in cache vs. the background .download staging file) and only made downloaded data usable after a whole-layer switch. Filling the shared cache instead removes the duplicate fetch and lets every block serve foreground reads as soon as it lands.

N/A

Scope and non-goals

Included:

New cache-level BkDownloadScheduler (storage/overlaybd/src/backend/cache/bk_download.rs) and its wiring through FileCacheBackend, ImageService, and ImageFile.
Download granularity stays decoupled from the cache block size: download.blockSize (default 16 MiB) is one source request; contiguous missing cache blocks within a chunk coalesce into a single read, already-cached blocks are skipped, and blocks are published to the cache bitmap as they land.
New download.maxConcurrentFiles knob (default 8); validation bounds for all download knobs including the backend-wide scratch budget.
Deletion of the old per-ImageFile bk_download.rs (1561 lines).

Non-goals / intentionally excluded:

No overlaybd.commit promotion and no switch-to-local after a full download. Cache-only completion: the cache is a bounded, evictable working set; evicted blocks are re-fetched on demand. Disk pressure is not solved by materializing layers into another local file of the same total size.
No full-layer P2P publication on download completion (no longer a complete local layer artifact to reference); the publish plumbing is left inert and will be removed in a separate cleanup.
Foreground read path and cache eviction policy are unchanged apart from the safety fixes called out below.

Design and behavior changes

Scheduler semantics:

Submission registers tasks (dedup by cache_id, shared across sandboxes) and never fails under load; only execution is bounded — maxConcurrentFiles layer tasks per backend and maxInflightBlocks concurrent chunk fetches. Ready tasks dispatch round-robin per device key.
Pending tasks hold no cache-entry reference and remain evictable; a running task re-opens its cached file and pins the entry only for the run. Registry cleanup is pointer-checked so a finishing task cannot remove a newer registration for the same blob.
envd-ready gating, configured delay, maxMBps throttling, per-chunk hedge timeout with reissue, and try_cnt retry with backoff are preserved from the old downloader; retries re-enumerate missing chunks only.

Safety fixes (found in review, each covered by a targeted test):

Only the primary backend's Drop stops downloads; clones never inherit scheduler ownership. Shutdown is an idempotent, caller-independent drain that closes both semaphores; the run guard is built before spawn, so abort-before-first-poll cannot leak the active counter.
Foreground misses always join the per-block loader election, so same-block fetches stay deduplicated at any chunk concurrency (DEFAULT_MAX_REFILLING now gates only range prefill).
Gate floor admission is atomic and re-checked after a block slot is acquired.
Failed/timed-out chunk reads stage through task scratch and never leave unaccounted physical pages in the mmap; capacity accounting is batched one update per run instead of per cache block.

Compatibility and operations

Public API or generated protocol: N/A (no API change; generated code untouched).
Configuration or defaults: new optional download.maxConcurrentFiles (default 8). download.blockSize remains the background chunk size (default now 16 MiB) and is aligned down to whole cache blocks at runtime. Validation now bounds blockSize/concurrency/maxInflightBlocks/maxConcurrentFiles; previously accepted extreme values are rejected at config load.
Snapshot manifest, artifact layout, or storage format: no change to committed snapshot formats. Background downloads no longer produce overlaybd.commit files for remote layers; pre-existing local commit files still open through the existing local-layer path.
Upgrade and rollback: forward/backward compatible — cached blocks persist via the existing checkpoint format; rolling back simply resumes the old staging behavior and ignores the cache fills.
Host requirements, permissions, ports, or dependencies: none new. The ring dependency is dropped from the overlaybd crate (the trailing whole-file hasher is gone).

Validation

make fmt
make clippy
make test-unit
Relevant Rust integration tests
make -C services test (required when services/ changes)
Generated clients/server regenerated with the documented make target
Documentation updated
Benchmarks or performance comparison completed

Commands and results:

cargo fmt --all -- --check                                    # clean
cargo clippy -p overlaybd -p uvm-ublk -p uvm-ublk-daemon -p agentenv --all-targets -- -D warnings
                                                              # clean
make test-unit                                                # 687 passed; 1 pre-existing failure
                                                              # (privileges::tests::scoped_spawn_clears_child_capabilities_without_mutating_caller
                                                              #  fails on this host's capability environment, also without this change)
cargo test -p overlaybd --lib                                 # 308 passed

Skipped checks and reasons:

Rust integration tests (tests/integration): require KVM/network namespaces; covered instead by the real OSS E2E below on the same build.
make -C services test: no services/ changes.
Generated clients/server: no OpenAPI/protocol changes.

Real OSS experiment (Aliyun OSS bucket agentenv-oss-validation, same dataset shape as PR #540): ubuntu 24.04 guest, 8 GiB RAM, 4 GiB dirtied (random), OSS-backed snapshot, release build, public endpoint, caches cleared per round.

16 MiB chunks	This PR	PR #540
c4 steady window	249–263 MB/s	199.8 / 231.1 MB/s
c8 steady window	404–449 MB/s	386.7 / 442.8 MB/s
envd-ready → full cache (c8)	~12 s	~12–14 s
in-guest 4 GiB page-in after fill	4.5 s	39.6 s
in-guest 4 GiB page-in, bk OFF	404.7 s	485.8 s

Risks and reviewer notes

Highest-risk files: backend/cache/bk_download.rs (scheduler lifecycle), backend/cache/full_file_cache/cache_store.rs (background_refill_range/refill_block_run election + publication), cache_pool.rs (scheduler ownership, pressure accounting), download_gate.rs (atomic floor admission).
The cache is intentionally a bounded working set: under capacity pressure, background-filled blocks can be evicted and are re-fetched on demand; background refill backs off with ENOSPC retries while the cache is full.
ImageFile::drop no longer stops downloads — tasks are shared per blob and finish independently; reviewers should confirm this matches sandbox churn expectations.
The P2P publish plumbing (p2p_publish_url) is now inert; kept to keep this PR focused, removal is a follow-up.

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/124 **Author:** [@huajq](https://github.com/huajq) **Created:** 8/5/2026 **Status:** ✅ Merged **Merged:** 8/6/2026 **Merged by:** [@yingdi-shan](https://github.com/yingdi-shan) **Base:** `main` ← **Head:** `bk_cache` --- ### 📝 Commits (1) - [`8daf172`](https://github.com/kvcache-ai/AgentENV/commit/8daf1723a941b65997f1f46c5bd157c58f457daa) feat(overlaybd): move background download into the shared full-file cache ### 📊 Changes **21 files changed** (+2937 additions, -1969 deletions) <details> <summary>View changed files</summary> 📝 `CLAUDE.md` (+1 -1) 📝 `Cargo.lock` (+1 -1) 📝 `config/default.toml` (+3 -3) 📝 `config/oss_default.toml` (+3 -3) 📝 `docs/src/configuration/reference.md` (+23 -11) 📝 `src/cfg.rs` (+23 -3) 📝 `src/setup/deps.rs` (+5 -0) 📝 `storage/overlaybd/Cargo.toml` (+0 -1) ➕ `storage/overlaybd/src/backend/cache/bk_download.rs` (+780 -0) 📝 `storage/overlaybd/src/backend/cache/full_file_cache/cache_entry.rs` (+37 -17) 📝 `storage/overlaybd/src/backend/cache/full_file_cache/cache_pool.rs` (+322 -96) 📝 `storage/overlaybd/src/backend/cache/full_file_cache/cache_store.rs` (+254 -70) 📝 `storage/overlaybd/src/backend/cache/mod.rs` (+3 -0) 📝 `storage/overlaybd/src/backend/cache/tests.rs` (+773 -1) ➖ `storage/overlaybd/src/bk_download.rs` (+0 -1561) 📝 `storage/overlaybd/src/config.rs` (+163 -10) 📝 `storage/overlaybd/src/download_gate.rs` (+84 -9) 📝 `storage/overlaybd/src/image/image_file.rs` (+266 -126) 📝 `storage/overlaybd/src/image/image_service.rs` (+130 -29) 📝 `storage/overlaybd/src/lib.rs` (+0 -2) _...and 1 more files_ </details> ### 📄 Description ## What Move OverlayBD background download from a per-ImageFile downloader (private `.download` staging file, whole-layer `overlaybd.commit` promotion, `SwitchFile` swap) into one scheduler per `FileCacheBackend` that fills the shared full-file cache directly. Completed cache blocks become visible to foreground reads immediately, and foreground/background fetches of the same block are deduplicated by the per-block loader election. ## Why The old design fetched the same bytes twice (foreground page-in cache vs. the background `.download` staging file) and only made downloaded data usable after a whole-layer switch. Filling the shared cache instead removes the duplicate fetch and lets every block serve foreground reads as soon as it lands. ## Related issue N/A ## Scope and non-goals Included: - New cache-level `BkDownloadScheduler` (`storage/overlaybd/src/backend/cache/bk_download.rs`) and its wiring through `FileCacheBackend`, `ImageService`, and `ImageFile`. - Download granularity stays decoupled from the cache block size: `download.blockSize` (default 16 MiB) is one source request; contiguous missing cache blocks within a chunk coalesce into a single read, already-cached blocks are skipped, and blocks are published to the cache bitmap as they land. - New `download.maxConcurrentFiles` knob (default 8); validation bounds for all download knobs including the backend-wide scratch budget. - Deletion of the old per-ImageFile `bk_download.rs` (1561 lines). Non-goals / intentionally excluded: - No `overlaybd.commit` promotion and no switch-to-local after a full download. Cache-only completion: the cache is a bounded, evictable working set; evicted blocks are re-fetched on demand. Disk pressure is not solved by materializing layers into another local file of the same total size. - No full-layer P2P publication on download completion (no longer a complete local layer artifact to reference); the publish plumbing is left inert and will be removed in a separate cleanup. - Foreground read path and cache eviction policy are unchanged apart from the safety fixes called out below. ## Design and behavior changes Scheduler semantics: - Submission registers tasks (dedup by `cache_id`, shared across sandboxes) and never fails under load; only execution is bounded — `maxConcurrentFiles` layer tasks per backend and `maxInflightBlocks` concurrent chunk fetches. Ready tasks dispatch round-robin per device key. - Pending tasks hold no cache-entry reference and remain evictable; a running task re-opens its cached file and pins the entry only for the run. Registry cleanup is pointer-checked so a finishing task cannot remove a newer registration for the same blob. - envd-ready gating, configured delay, `maxMBps` throttling, per-chunk hedge timeout with reissue, and try_cnt retry with backoff are preserved from the old downloader; retries re-enumerate missing chunks only. Safety fixes (found in review, each covered by a targeted test): - Only the primary backend's `Drop` stops downloads; clones never inherit scheduler ownership. Shutdown is an idempotent, caller-independent drain that closes both semaphores; the run guard is built before spawn, so abort-before-first-poll cannot leak the active counter. - Foreground misses always join the per-block loader election, so same-block fetches stay deduplicated at any chunk concurrency (`DEFAULT_MAX_REFILLING` now gates only range prefill). - Gate floor admission is atomic and re-checked after a block slot is acquired. - Failed/timed-out chunk reads stage through task scratch and never leave unaccounted physical pages in the mmap; capacity accounting is batched one update per run instead of per cache block. ## Compatibility and operations - Public API or generated protocol: N/A (no API change; generated code untouched). - Configuration or defaults: new optional `download.maxConcurrentFiles` (default 8). `download.blockSize` remains the background chunk size (default now 16 MiB) and is aligned down to whole cache blocks at runtime. Validation now bounds `blockSize`/`concurrency`/`maxInflightBlocks`/`maxConcurrentFiles`; previously accepted extreme values are rejected at config load. - Snapshot manifest, artifact layout, or storage format: no change to committed snapshot formats. Background downloads no longer produce `overlaybd.commit` files for remote layers; pre-existing local commit files still open through the existing local-layer path. - Upgrade and rollback: forward/backward compatible — cached blocks persist via the existing checkpoint format; rolling back simply resumes the old staging behavior and ignores the cache fills. - Host requirements, permissions, ports, or dependencies: none new. The `ring` dependency is dropped from the overlaybd crate (the trailing whole-file hasher is gone). ## Validation - [x] `make fmt` - [x] `make clippy` - [x] `make test-unit` - [ ] Relevant Rust integration tests - [ ] `make -C services test` (required when `services/` changes) - [ ] Generated clients/server regenerated with the documented `make` target - [x] Documentation updated - [x] Benchmarks or performance comparison completed Commands and results: ```text cargo fmt --all -- --check # clean cargo clippy -p overlaybd -p uvm-ublk -p uvm-ublk-daemon -p agentenv --all-targets -- -D warnings # clean make test-unit # 687 passed; 1 pre-existing failure # (privileges::tests::scoped_spawn_clears_child_capabilities_without_mutating_caller # fails on this host's capability environment, also without this change) cargo test -p overlaybd --lib # 308 passed ``` Skipped checks and reasons: - Rust integration tests (`tests/integration`): require KVM/network namespaces; covered instead by the real OSS E2E below on the same build. - `make -C services test`: no `services/` changes. - Generated clients/server: no OpenAPI/protocol changes. Real OSS experiment (Aliyun OSS bucket `agentenv-oss-validation`, same dataset shape as PR #540): ubuntu 24.04 guest, 8 GiB RAM, 4 GiB dirtied (random), OSS-backed snapshot, release build, public endpoint, caches cleared per round. | 16 MiB chunks | This PR | PR #540 | |---|---|---| | c4 steady window | 249–263 MB/s | 199.8 / 231.1 MB/s | | c8 steady window | 404–449 MB/s | 386.7 / 442.8 MB/s | | envd-ready → full cache (c8) | ~12 s | ~12–14 s | | in-guest 4 GiB page-in after fill | 4.5 s | 39.6 s | | in-guest 4 GiB page-in, bk OFF | 404.7 s | 485.8 s | ## Risks and reviewer notes - Highest-risk files: `backend/cache/bk_download.rs` (scheduler lifecycle), `backend/cache/full_file_cache/cache_store.rs` (`background_refill_range`/`refill_block_run` election + publication), `cache_pool.rs` (scheduler ownership, pressure accounting), `download_gate.rs` (atomic floor admission). - The cache is intentionally a bounded working set: under capacity pressure, background-filled blocks can be evicted and are re-fetched on demand; background refill backs off with ENOSPC retries while the cache is full. - `ImageFile::drop` no longer stops downloads — tasks are shared per blob and finish independently; reviewers should confirm this matches sandbox churn expectations. - The P2P publish plumbing (`p2p_publish_url`) is now inert; kept to keep this PR focused, removal is a follow-up. ## 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:20:53 +00:00