[PR #344] [MERGED] fix(overlaybd): reclaim cache capacity pinned by open entries #363

New issue

Closed

opened 2026-10-29 14:22:39 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-29 14:22:39 +00:00

Owner

📋 Pull Request Information

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

Base: main ← Head: fix/overlaybd-open-entry-cache-eviction

📝 Commits (1)

0973730 fix(overlaybd): reclaim cache capacity pinned by open entries

📊 Changes

10 files changed (+565 additions, -43 deletions)

View changed files

📝 docs/src/configuration/reference.md (+1 -0)
📝 src/cfg/image.rs (+20 -0)
📝 src/image/cache/service.rs (+1 -0)
📝 src/orchestrator/tests.rs (+1 -0)
📝 src/setup/deps.rs (+22 -0)
📝 storage/overlaybd/src/backend/cache/full_file_cache/cache_entry.rs (+54 -10)
📝 storage/overlaybd/src/backend/cache/full_file_cache/cache_pool.rs (+99 -1)
📝 storage/overlaybd/src/backend/cache/full_file_cache/cache_store.rs (+142 -32)
📝 storage/overlaybd/src/backend/cache/tests.rs (+220 -0)
📝 storage/overlaybd/src/config.rs (+5 -0)

📄 Description

What

Fix the OverlayBD full-file cache eviction cliff described in #320. Entries held open by long-lived sandboxes are no longer pinned forever: under capacity pressure the cache now reclaims blocks of open entries that have been idle past a threshold, so new templates keep being cached even when the pool is at its watermark. Concurrent uncached reads of the same block now share one source fetch. The idle threshold is configurable via image.cache.remote_blocks.open_evict_idle_secs (default 600s, 0 disables).

Why

On a node serving many templates with long-lived sandboxes, every template with a live sandbox kept its memory-snapshot and rootfs layers pinned: eviction only considered entries with open_count == 0, so once pinned bytes reached the watermark there was nothing evictable, is_full latched, and every new template degraded to uncached per-block source reads (no sharing between concurrent readers) until creates exceeded envd.init_timeout_secs. The failure is a cliff: cached templates keep working while every new template degrades at once.

Closes #320

Scope and non-goals

Included:

Read-path barrier: zero-copy cached reads hold a read permit on the entry's refill_eviction_barrier for the lifetime of the returned Bytes (safety prerequisite for evicting open entries).
Two-tier pressure eviction (closed entries first, then block-level eviction of idle open entries) plus a synchronous evict-and-retry-once on ENOSPC in refill admission.
Deduplication of concurrent uncached reads of a block via a per-block shared future.
New optional config knob + generated overlaybd config field + documentation.

Excluded / non-goals (mirrors the issue): no change to default cache sizes, no per-block LRU, no cross-node placement or P2P changes, no change to evict_by_size / evict_global semantics. Unrelated pre-existing finding (reported separately, not fixed here): reference.md documents [image.cache.remote_blocks].max_size_gb default as 100 while code defaults to 10.

Design and behavior changes

Read barrier (cache_entry.rs, cache_store.rs): read_block acquires an OwnedRwLockReadGuard via try_read_owned before checking the bitmap, and the returned Bytes owns the guard (new CachedBlockBytes owner). A failed try-read (eviction pending/running) is reported as a miss, so readers fall back to refill/source instead of observing hole-punched pages. The refill path also uses try_read_owned and skips to source fallback rather than queueing behind a writer.
Two-tier eviction (cache_pool.rs): tier 1 evicts closed entries whole (unchanged). If pressure remains, tier 2 reclaims blocks of open entries whose last_access is older than open_evict_idle, keeping the entry (evict_all_blocks); open handles keep working and refill on demand. Tier 2 skips entries with in-flight refills. Because tier 2 only runs when over the watermark, idle blocks are not reclaimed in normal operation, and active sandboxes are protected by the idle threshold (every hit/miss/refill refreshes last_access).
ENOSPC admission retry (cache_store.rs): previously a full pool made refill fail without triggering synchronous eviction (only the 1s periodic worker ran). Refill now runs eviction_inner once and retries the block once on ENOSPC, so an incoming template gets its blocks cached promptly.
Miss dedup (cache_entry.rs, cache_store.rs): the fallback source read publishes a futures_util::future::Shared future in the entry's miss_fanouts map. Any awaiter's poll drives the fetch (cancellation-safe, no designated loader), every awaiter gets a clone of the result (errors Arc-wrapped, fail-fast to all waiters). No caching or capacity accounting is touched on this path.

Concurrency invariants: eviction's write side of the barrier spans bitmap clear + meta removal + hole punch, so it cannot interleave with refills or with any live read Bytes. Tier-2 vs rename_store_key/evict_entry are mutually exclusive by open_count and the Evicting slot state. Double eviction cannot double-subtract capacity (bytes are computed under the write barrier).

Compatibility and operations

Public API or generated protocol: N/A — no API or generated-code changes.
Configuration or defaults: new optional image.cache.remote_blocks.open_evict_idle_secs (default 600), written to generated overlaybd cacheConfig.openEvictIdleSecs for both remote-blocks and memory-blocks caches. Previously generated/hand-written overlaybd configs load unchanged (serde default applies). Behavior change: blocks of open entries idle longer than the threshold may be reclaimed under capacity pressure; a sandbox touching them again pays a source read. Set to 0 to restore the previous behavior.
Snapshot manifest, artifact layout, or storage format: N/A — cache meta.bin/bitmap format unchanged.
Upgrade and rollback: no migration; rollback simply restores the previous behavior (open entries pinned again).
Host requirements, permissions, ports, or dependencies: none. futures-util was already a dependency of the overlaybd crate.

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:

# macOS (dev machine)
cargo fmt --all -- --check                                   # clean
cargo clippy -p overlaybd --all-targets -- -D warnings       # clean
cargo test -p overlaybd --lib                                # 374 passed, 1 ignored
cargo test -p overlaybd --lib backend::cache::tests          # 57 passed, 1 ignored

# Linux (x86_64 test host)
make clippy                                                  # clean (workspace)
cargo test -p overlaybd --lib                                # 387 passed, 2 ignored
cargo test -p agentenv --lib -- setup::deps::tests           # 25 passed (incl. new generated-config test)
cargo test -p agentenv --lib cfg::image::tests               # 2 passed (incl. new layout test)
make test-ublk                                               # green (ublk/overlaybd/ublk-daemon + registry e2e + MinIO 7/7)

# New tests (all verified to fail before the fix):
# - test_read_falls_back_to_source_while_eviction_holds_write_lock
# - test_concurrent_reads_survive_open_entry_block_eviction (8 readers x 500 iters vs continuous eviction, zero corruption)
# - test_idle_open_entries_are_block_evicted_under_pressure (issue acceptance scenario: pool pinned by idle open entries, new template still populates cache; affected entry refills correctly)
# - test_concurrent_uncached_reads_fetch_source_once (8 concurrent readers -> 1 source fetch, was 8)
# - test_open_entry_is_not_removed_by_eviction (rewritten: knob disabled = previous behavior)

# Snapshot resume benchmark (A/B, same host, verified binaries, agent/ubuntu:24.04_obd):
#   snapshot_resume_cold  154.12ms baseline vs 150.89ms change (mean of 10)
#   snapshot_resume       32.96ms baseline vs 32.52ms change (mean of 10)
# => no measurable resume regression.

Skipped checks and reasons:

make test-unit: not run as a whole; the touched crates' suites above plus make test-ublk cover the change. Full VM-dependent suites need KVM and are outside this change's blast radius.
make -C services test: no services/ changes.
Generated-code regeneration: no schema changes (openapi.yml, protobuf, firecracker/envd clients untouched).
concurrent_resume benchmark (50 concurrent VMs): not measurable on the test host — the 50-VM burst fails envd timeout identically on baseline and fixed builds (pre-existing bench/host limitation, pool pre-warming tried). Cold/hot resume above covers the read-path cost.

Risks and reviewer notes

Not yet validated at production scale. The issue's acceptance scenario is covered by an integration test (test_idle_open_entries_are_block_evicted_under_pressure), but the 800-sandbox workload from #320 has not been rerun. @nblintao offered to test/review — a verification run with a small cache (e.g. max_size_gb = 2) on their node would be valuable.
Read results now hold a read permit for their lifetime: a task must not hold a read Bytes while awaiting evict_all/evict_range on the same entry (self-deadlock; documented on those methods). No production call site does this.
During an eviction of a hot entry, that entry's reads miss to the source for the punch duration (write-preferring lock) — bounded and only under pressure.
ENOSPC retry runs one eviction scan per block under sustained pressure; bounded, and eviction targets are skipped while refills are in flight.
Key files: storage/overlaybd/src/backend/cache/full_file_cache/cache_entry.rs (barrier/owner), cache_pool.rs (tier-2 eviction), cache_store.rs (ENOSPC retry, pread_from_source_dedup), storage/overlaybd/src/config.rs + src/cfg/image.rs + src/setup/deps.rs (config plumbing).

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/344 **Author:** [@huajq](https://github.com/huajq) **Created:** 10/8/2026 **Status:** ✅ Merged **Merged:** 10/10/2026 **Merged by:** [@yingdi-shan](https://github.com/yingdi-shan) **Base:** `main` ← **Head:** `fix/overlaybd-open-entry-cache-eviction` --- ### 📝 Commits (1) - [`0973730`](https://github.com/kvcache-ai/AgentENV/commit/0973730424dc50de9b3e6e50ceb3409797e3eda4) fix(overlaybd): reclaim cache capacity pinned by open entries ### 📊 Changes **10 files changed** (+565 additions, -43 deletions) <details> <summary>View changed files</summary> 📝 `docs/src/configuration/reference.md` (+1 -0) 📝 `src/cfg/image.rs` (+20 -0) 📝 `src/image/cache/service.rs` (+1 -0) 📝 `src/orchestrator/tests.rs` (+1 -0) 📝 `src/setup/deps.rs` (+22 -0) 📝 `storage/overlaybd/src/backend/cache/full_file_cache/cache_entry.rs` (+54 -10) 📝 `storage/overlaybd/src/backend/cache/full_file_cache/cache_pool.rs` (+99 -1) 📝 `storage/overlaybd/src/backend/cache/full_file_cache/cache_store.rs` (+142 -32) 📝 `storage/overlaybd/src/backend/cache/tests.rs` (+220 -0) 📝 `storage/overlaybd/src/config.rs` (+5 -0) </details> ### 📄 Description ## What Fix the OverlayBD full-file cache eviction cliff described in #320. Entries held open by long-lived sandboxes are no longer pinned forever: under capacity pressure the cache now reclaims blocks of open entries that have been idle past a threshold, so new templates keep being cached even when the pool is at its watermark. Concurrent uncached reads of the same block now share one source fetch. The idle threshold is configurable via `image.cache.remote_blocks.open_evict_idle_secs` (default 600s, `0` disables). ## Why On a node serving many templates with long-lived sandboxes, every template with a live sandbox kept its memory-snapshot and rootfs layers pinned: eviction only considered entries with `open_count == 0`, so once pinned bytes reached the watermark there was nothing evictable, `is_full` latched, and every new template degraded to uncached per-block source reads (no sharing between concurrent readers) until creates exceeded `envd.init_timeout_secs`. The failure is a cliff: cached templates keep working while every new template degrades at once. ## Related issue Closes #320 ## Scope and non-goals Included: - Read-path barrier: zero-copy cached reads hold a read permit on the entry's `refill_eviction_barrier` for the lifetime of the returned `Bytes` (safety prerequisite for evicting open entries). - Two-tier pressure eviction (closed entries first, then block-level eviction of idle open entries) plus a synchronous evict-and-retry-once on ENOSPC in refill admission. - Deduplication of concurrent uncached reads of a block via a per-block shared future. - New optional config knob + generated overlaybd config field + documentation. Excluded / non-goals (mirrors the issue): no change to default cache sizes, no per-block LRU, no cross-node placement or P2P changes, no change to `evict_by_size` / `evict_global` semantics. Unrelated pre-existing finding (reported separately, not fixed here): `reference.md` documents `[image.cache.remote_blocks].max_size_gb` default as `100` while code defaults to `10`. ## Design and behavior changes - **Read barrier** (`cache_entry.rs`, `cache_store.rs`): `read_block` acquires an `OwnedRwLockReadGuard` via `try_read_owned` *before* checking the bitmap, and the returned `Bytes` owns the guard (new `CachedBlockBytes` owner). A failed try-read (eviction pending/running) is reported as a miss, so readers fall back to refill/source instead of observing hole-punched pages. The refill path also uses `try_read_owned` and skips to source fallback rather than queueing behind a writer. - **Two-tier eviction** (`cache_pool.rs`): tier 1 evicts closed entries whole (unchanged). If pressure remains, tier 2 reclaims blocks of open entries whose `last_access` is older than `open_evict_idle`, keeping the entry (`evict_all_blocks`); open handles keep working and refill on demand. Tier 2 skips entries with in-flight refills. Because tier 2 only runs when over the watermark, idle blocks are not reclaimed in normal operation, and active sandboxes are protected by the idle threshold (every hit/miss/refill refreshes `last_access`). - **ENOSPC admission retry** (`cache_store.rs`): previously a full pool made refill fail without triggering synchronous eviction (only the 1s periodic worker ran). Refill now runs `eviction_inner` once and retries the block once on ENOSPC, so an incoming template gets its blocks cached promptly. - **Miss dedup** (`cache_entry.rs`, `cache_store.rs`): the fallback source read publishes a `futures_util::future::Shared` future in the entry's `miss_fanouts` map. Any awaiter's poll drives the fetch (cancellation-safe, no designated loader), every awaiter gets a clone of the result (errors Arc-wrapped, fail-fast to all waiters). No caching or capacity accounting is touched on this path. Concurrency invariants: eviction's write side of the barrier spans bitmap clear + meta removal + hole punch, so it cannot interleave with refills or with any live read `Bytes`. Tier-2 vs `rename_store_key`/`evict_entry` are mutually exclusive by `open_count` and the `Evicting` slot state. Double eviction cannot double-subtract capacity (bytes are computed under the write barrier). ## Compatibility and operations - Public API or generated protocol: N/A — no API or generated-code changes. - Configuration or defaults: new optional `image.cache.remote_blocks.open_evict_idle_secs` (default `600`), written to generated overlaybd `cacheConfig.openEvictIdleSecs` for both `remote-blocks` and `memory-blocks` caches. Previously generated/hand-written overlaybd configs load unchanged (serde default applies). Behavior change: blocks of open entries idle longer than the threshold may be reclaimed under capacity pressure; a sandbox touching them again pays a source read. Set to `0` to restore the previous behavior. - Snapshot manifest, artifact layout, or storage format: N/A — cache `meta.bin`/bitmap format unchanged. - Upgrade and rollback: no migration; rollback simply restores the previous behavior (open entries pinned again). - Host requirements, permissions, ports, or dependencies: none. `futures-util` was already a dependency of the overlaybd crate. ## Validation - [x] `make fmt` - [x] `make clippy` - [ ] `make test-unit` - [x] 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 # macOS (dev machine) cargo fmt --all -- --check # clean cargo clippy -p overlaybd --all-targets -- -D warnings # clean cargo test -p overlaybd --lib # 374 passed, 1 ignored cargo test -p overlaybd --lib backend::cache::tests # 57 passed, 1 ignored # Linux (x86_64 test host) make clippy # clean (workspace) cargo test -p overlaybd --lib # 387 passed, 2 ignored cargo test -p agentenv --lib -- setup::deps::tests # 25 passed (incl. new generated-config test) cargo test -p agentenv --lib cfg::image::tests # 2 passed (incl. new layout test) make test-ublk # green (ublk/overlaybd/ublk-daemon + registry e2e + MinIO 7/7) # New tests (all verified to fail before the fix): # - test_read_falls_back_to_source_while_eviction_holds_write_lock # - test_concurrent_reads_survive_open_entry_block_eviction (8 readers x 500 iters vs continuous eviction, zero corruption) # - test_idle_open_entries_are_block_evicted_under_pressure (issue acceptance scenario: pool pinned by idle open entries, new template still populates cache; affected entry refills correctly) # - test_concurrent_uncached_reads_fetch_source_once (8 concurrent readers -> 1 source fetch, was 8) # - test_open_entry_is_not_removed_by_eviction (rewritten: knob disabled = previous behavior) # Snapshot resume benchmark (A/B, same host, verified binaries, agent/ubuntu:24.04_obd): # snapshot_resume_cold 154.12ms baseline vs 150.89ms change (mean of 10) # snapshot_resume 32.96ms baseline vs 32.52ms change (mean of 10) # => no measurable resume regression. ``` Skipped checks and reasons: - `make test-unit`: not run as a whole; the touched crates' suites above plus `make test-ublk` cover the change. Full VM-dependent suites need KVM and are outside this change's blast radius. - `make -C services test`: no `services/` changes. - Generated-code regeneration: no schema changes (`openapi.yml`, protobuf, firecracker/envd clients untouched). - `concurrent_resume` benchmark (50 concurrent VMs): not measurable on the test host — the 50-VM burst fails envd timeout identically on baseline and fixed builds (pre-existing bench/host limitation, pool pre-warming tried). Cold/hot resume above covers the read-path cost. ## Risks and reviewer notes - **Not yet validated at production scale.** The issue's acceptance scenario is covered by an integration test (`test_idle_open_entries_are_block_evicted_under_pressure`), but the 800-sandbox workload from #320 has not been rerun. @nblintao offered to test/review — a verification run with a small cache (e.g. `max_size_gb = 2`) on their node would be valuable. - Read results now hold a read permit for their lifetime: a task must not hold a read `Bytes` while awaiting `evict_all`/`evict_range` on the same entry (self-deadlock; documented on those methods). No production call site does this. - During an eviction of a hot entry, that entry's reads miss to the source for the punch duration (write-preferring lock) — bounded and only under pressure. - ENOSPC retry runs one eviction scan per block under sustained pressure; bounded, and eviction targets are skipped while refills are in flight. - Key files: `storage/overlaybd/src/backend/cache/full_file_cache/cache_entry.rs` (barrier/owner), `cache_pool.rs` (tier-2 eviction), `cache_store.rs` (ENOSPC retry, `pread_from_source_dedup`), `storage/overlaybd/src/config.rs` + `src/cfg/image.rs` + `src/setup/deps.rs` (config plumbing). ## 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:22:39 +00:00