[PR #161] [MERGED] fix(overlaybd): clear cache bitmap before destructive eviction steps #218

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

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

Base: main ← Head: fix/overlaybd-cache-eviction-recovery

📝 Commits (1)

c132bda fix(overlaybd): clear cache bitmap before destructive eviction steps

📊 Changes

2 files changed (+63 additions, -5 deletions)

View changed files

📝 storage/overlaybd/src/backend/cache/full_file_cache/cache_entry.rs (+12 -5)
📝 storage/overlaybd/src/backend/cache/tests.rs (+51 -0)

📄 Description

What

Reorder CacheEntry::evict_all_blocks: clear the in-memory block bitmap before removing meta.bin and hole-punching the data file. Pure ordering change; no new logic.

Why

evict_all_blocks currently removes meta.bin, hole-punches the data file (its contents become zeros), and only then clears the in-memory bitmap. If the operation exits unexpectedly between the punch and the bitmap clear — e.g. a filesystem error under disk pressure, or a panic/cancellation in the blocking task — the entry is left with a bitmap that still claims blocks are cached while its data file reads back as all zeros. Every later read then hits the bitmap and serves zeros without ever falling back to the source (header magic/type don't match at the overlaybd layer), with no self-heal until process restart.

Clearing the bitmap first makes every partial-failure mode leave a benign "empty bitmap + intact data" state, where reads simply miss and refetch from the source.

No tracking issue. Found during the review of the 2026-08-11 production cache-poison incident; related to #131 (cache-directory isolation). This PR is defense-in-depth, not the incident's root-cause fix.

Scope and non-goals

In scope: the evict_all_blocks ordering change and one discriminating regression test.
Out of scope: refill-time content validation, open-time entry-consistency validation, and self-heal retry on header-verification failure (follow-up PRs). No metrics/alerting changes.

Design and behavior changes

Failure/crash semantics only: a mid-eviction failure now leaves "empty bitmap + intact data" (miss-and-refetch) instead of "marked bitmap + zeroed data" (poison). The successful eviction path is behaviorally unchanged.

Compatibility and operations

Public API or generated protocol: N/A
Configuration or defaults: N/A
Snapshot manifest, artifact layout, or storage format: N/A (cache entries are rebuildable artifacts)
Upgrade and rollback: no migration; existing on-disk cache entries load as before
Host requirements, permissions, ports, or dependencies: N/A

Validation

make fmt (equivalent: cargo fmt --check passed)
make clippy (cargo clippy -p overlaybd --all-targets -- -D warnings passed)
Relevant unit tests: cargo test -p overlaybd --lib -- backend::cache — 56 passed / 0 failed, including the new regression test test_eviction_clears_bitmap_before_destroying_data, which fails on the pre-fix code
make test-unit (not run in full; change is confined to the overlaybd cache eviction path and covered by the crate-level suite)
make -C services test (no services/ changes)
Benchmarks (no hot-path changes; ordering-only change on the eviction cold path)

Skipped checks and reasons: KVM integration tests not run — the change does not touch device/VM paths; validation was performed on a Linux test host.

Risks and reviewer notes

Very low risk: ordering change only; success-path semantics unchanged.
Review focus: evict_all_blocks in storage/overlaybd/src/backend/cache/full_file_cache/cache_entry.rs; regression test at the end of tests.rs.

Checklist

The PR contains one coherent change and no unrelated formatting or refactoring.
New behavior is covered by tests (discriminating regression test that fails pre-fix).
Logs and examples contain no credentials, tokens, or private registry information.

_{🔄 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/161 **Author:** [@huajq](https://github.com/huajq) **Created:** 8/12/2026 **Status:** ✅ Merged **Merged:** 8/13/2026 **Merged by:** [@yingdi-shan](https://github.com/yingdi-shan) **Base:** `main` ← **Head:** `fix/overlaybd-cache-eviction-recovery` --- ### 📝 Commits (1) - [`c132bda`](https://github.com/kvcache-ai/AgentENV/commit/c132bda185b1fd338d91f4e9c886445d7ecd0635) fix(overlaybd): clear cache bitmap before destructive eviction steps ### 📊 Changes **2 files changed** (+63 additions, -5 deletions) <details> <summary>View changed files</summary> 📝 `storage/overlaybd/src/backend/cache/full_file_cache/cache_entry.rs` (+12 -5) 📝 `storage/overlaybd/src/backend/cache/tests.rs` (+51 -0) </details> ### 📄 Description ## What Reorder `CacheEntry::evict_all_blocks`: clear the in-memory block bitmap **before** removing `meta.bin` and hole-punching the data file. Pure ordering change; no new logic. ## Why `evict_all_blocks` currently removes `meta.bin`, hole-punches the data file (its contents become zeros), and only then clears the in-memory bitmap. If the operation exits unexpectedly between the punch and the bitmap clear — e.g. a filesystem error under disk pressure, or a panic/cancellation in the blocking task — the entry is left with a bitmap that still claims blocks are cached while its data file reads back as all zeros. Every later read then hits the bitmap and serves zeros without ever falling back to the source (`header magic/type don't match` at the overlaybd layer), with no self-heal until process restart. Clearing the bitmap first makes every partial-failure mode leave a benign "empty bitmap + intact data" state, where reads simply miss and refetch from the source. ## Related issue No tracking issue. Found during the review of the 2026-08-11 production cache-poison incident; related to #131 (cache-directory isolation). This PR is defense-in-depth, not the incident's root-cause fix. ## Scope and non-goals - In scope: the `evict_all_blocks` ordering change and one discriminating regression test. - Out of scope: refill-time content validation, open-time entry-consistency validation, and self-heal retry on header-verification failure (follow-up PRs). No metrics/alerting changes. ## Design and behavior changes Failure/crash semantics only: a mid-eviction failure now leaves "empty bitmap + intact data" (miss-and-refetch) instead of "marked bitmap + zeroed data" (poison). The successful eviction path is behaviorally unchanged. ## Compatibility and operations - Public API or generated protocol: N/A - Configuration or defaults: N/A - Snapshot manifest, artifact layout, or storage format: N/A (cache entries are rebuildable artifacts) - Upgrade and rollback: no migration; existing on-disk cache entries load as before - Host requirements, permissions, ports, or dependencies: N/A ## Validation - [x] `make fmt` (equivalent: `cargo fmt --check` passed) - [x] `make clippy` (`cargo clippy -p overlaybd --all-targets -- -D warnings` passed) - [x] Relevant unit tests: `cargo test -p overlaybd --lib -- backend::cache` — 56 passed / 0 failed, including the new regression test `test_eviction_clears_bitmap_before_destroying_data`, which fails on the pre-fix code - [ ] `make test-unit` (not run in full; change is confined to the overlaybd cache eviction path and covered by the crate-level suite) - [ ] `make -C services test` (no `services/` changes) - [ ] Benchmarks (no hot-path changes; ordering-only change on the eviction cold path) Skipped checks and reasons: KVM integration tests not run — the change does not touch device/VM paths; validation was performed on a Linux test host. ## Risks and reviewer notes - Very low risk: ordering change only; success-path semantics unchanged. - Review focus: `evict_all_blocks` in `storage/overlaybd/src/backend/cache/full_file_cache/cache_entry.rs`; regression test at the end of `tests.rs`. ## Checklist - [x] The PR contains one coherent change and no unrelated formatting or refactoring. - [x] New behavior is covered by tests (discriminating regression test that fails pre-fix). - [x] Logs and examples contain no credentials, tokens, or private registry information. --- <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:09 +00:00