refactor(core): extract shared DebouncedEvictor; adopt in both caches (PSA-43) #12

Merged

nrupard merged 2 commits from feat/psa-43-debounced-evictor-core into main

2026-06-03 20:52:43 +02:00

nrupard commented

2026-06-03 20:47:18 +02:00

Owner

What

Extract the single-debounced-evictor mechanism PSA-39 introduced for the OCI blob cache into a shared dunite_core::services::debounced_evictor::DebouncedEvictor (re-exported as services::DebouncedEvictor, matching the single_flight precedent), and adopt it in BOTH dunite-oci blob_cache and dunite-download download_cache.

Why

PSA-39 fixed the OCI cache's per-blob tokio::spawn(evict_if_over_cap) (N connection-hungry tasks racing a bounded pool -> PoolTimedOut) with a single task poked through a capacity-1 channel. dunite-download still used the old per-store spawn pattern, so the verticals diverged and download carried the same latent bug. Per the architecture rule, cross-vertical sharing goes DOWN into dunite-core.

The primitive

DebouncedEvictor::new(label, run) spawns one long-lived task that runs the caller's run closure once per coalesced poke, logging Err at warn under the caller-supplied label. poke() is a capacity-1 try_send (bursts coalesce). The task holds only the closure (never a sender), so it exits when the last handle drops. The closure owns the eviction policy; the label owns the wording - no vertical-specific string lives in core.

Adoption

dunite-oci blob_cache: run_evictor + evict_tx replaced by a DebouncedEvictor field built from a closure over the existing free evict_over_cap; fetch_and_store calls self.evictor.poke().
dunite-download download_cache: per-store tokio::spawn replaced the same way; its eviction loop factored into a free evict_over_cap (negative-total guard + oldest(32) batch preserved exactly) shared by evict_if_over_cap and the evictor.
Both keep the public evict_if_over_cap (blocking/startup pass). As with PSA-39, BlobCache::new / DownloadCache::new must now run inside a Tokio runtime.

Acceptance criteria

dunite-core exposes a generic DebouncedEvictor with unit tests: burst coalesces into far fewer runs than pokes; task exits when all handles drop; an erroring closure is logged and does not kill the task.
dunite-oci BlobCache uses it; its existing burst_..._converges test passes unchanged.
dunite-download DownloadCache uses it, with an equivalent burst regression test (8 concurrent stores over cap 250 converge, eviction passes well under 8).
No vertical-specific strings leak into dunite-core (label is caller-supplied).
fmt + clippy -D warnings + cargo test --workspace all green.

Tests

Core: 3 new primitive tests. OCI: existing burst test unchanged. Download: new burst regression test. Full workspace green; both burst tests and the core tests verified non-flaky across 10 runs.

Follow-up

bunyip picks up nothing new here (internal refactor); the bunyip secret_env / acquire_concurrency_only switches from PSA-37/PSA-42 still await a dunite-core release.

#PSA-43

## What Extract the single-debounced-evictor mechanism PSA-39 introduced for the OCI blob cache into a shared `dunite_core::services::debounced_evictor::DebouncedEvictor` (re-exported as `services::DebouncedEvictor`, matching the `single_flight` precedent), and adopt it in BOTH dunite-oci `blob_cache` and dunite-download `download_cache`. ## Why PSA-39 fixed the OCI cache's per-blob `tokio::spawn(evict_if_over_cap)` (N connection-hungry tasks racing a bounded pool -> PoolTimedOut) with a single task poked through a capacity-1 channel. dunite-download still used the old per-store spawn pattern, so the verticals diverged and download carried the same latent bug. Per the architecture rule, cross-vertical sharing goes DOWN into dunite-core. ## The primitive `DebouncedEvictor::new(label, run)` spawns one long-lived task that runs the caller's `run` closure once per coalesced poke, logging `Err` at warn under the caller-supplied `label`. `poke()` is a capacity-1 `try_send` (bursts coalesce). The task holds only the closure (never a sender), so it exits when the last handle drops. The closure owns the eviction policy; the label owns the wording - no vertical-specific string lives in core. ## Adoption - dunite-oci `blob_cache`: `run_evictor` + `evict_tx` replaced by a `DebouncedEvictor` field built from a closure over the existing free `evict_over_cap`; `fetch_and_store` calls `self.evictor.poke()`. - dunite-download `download_cache`: per-store `tokio::spawn` replaced the same way; its eviction loop factored into a free `evict_over_cap` (negative-total guard + `oldest(32)` batch preserved exactly) shared by `evict_if_over_cap` and the evictor. - Both keep the public `evict_if_over_cap` (blocking/startup pass). As with PSA-39, `BlobCache::new` / `DownloadCache::new` must now run inside a Tokio runtime. ## Acceptance criteria - [x] `dunite-core` exposes a generic `DebouncedEvictor` with unit tests: burst coalesces into far fewer runs than pokes; task exits when all handles drop; an erroring closure is logged and does not kill the task. - [x] dunite-oci `BlobCache` uses it; its existing `burst_..._converges` test passes unchanged. - [x] dunite-download `DownloadCache` uses it, with an equivalent burst regression test (8 concurrent stores over cap 250 converge, eviction passes well under 8). - [x] No vertical-specific strings leak into dunite-core (label is caller-supplied). - [x] fmt + clippy -D warnings + `cargo test --workspace` all green. ## Tests Core: 3 new primitive tests. OCI: existing burst test unchanged. Download: new burst regression test. Full workspace green; both burst tests and the core tests verified non-flaky across 10 runs. ## Follow-up bunyip picks up nothing new here (internal refactor); the bunyip secret_env / acquire_concurrency_only switches from PSA-37/PSA-42 still await a dunite-core release. #PSA-43

nrupard added 1 commit

2026-06-03 20:47:18 +02:00

refactor(core): extract shared DebouncedEvictor; adopt in blob_cache and download_cache (PSA-43)

Checks / fmt + clippy + test (pull_request) Successful in 25s

Details

76230840c1

PSA-39 gave dunite-oci a single debounced background evictor (one task poked through a capacity-1 mpsc channel) so a burst of concurrent blob stores coalesces into one eviction pass on one connection instead of N detached tasks racing a bounded pool (PoolTimedOut). dunite-download still used the old per-store `tokio::spawn(evict_if_over_cap)` pattern, so the two verticals had diverged and download carried the same latent connection-hungry bug.

Extract the mechanism into `dunite_core::services::debounced_evictor::DebouncedEvictor` (re-exported as `services::DebouncedEvictor`, matching the `single_flight` precedent). It owns only the debounce-and-task machinery: `new(label, run)` spawns one long-lived task that runs the caller's closure once per coalesced poke and logs `Err` at warn under the caller-supplied `label`; `poke()` does a capacity-1 `try_send`. The task holds only the closure (never a sender), so it exits when the last handle drops. The closure owns the eviction policy and the label owns the wording, so no vertical-specific string lives in core.

Both caches now hold a `DebouncedEvictor` field built in their `new` from a closure over a free `evict_over_cap(store, cache_dir, max_bytes)` fn, and `fetch_and_store` calls `self.evictor.poke()` instead of spawning. dunite-oci's `run_evictor`/`evict_tx` and dunite-download's per-store `tokio::spawn` are gone; both keep their public `evict_if_over_cap` (blocking/startup pass) delegating to the shared free fn. As with PSA-39, `BlobCache::new`/`DownloadCache::new` must now run inside a Tokio runtime.

Tests: dunite-core covers the primitive directly (a burst of 100 pokes coalesces into far fewer runs; the task exits when all handles drop; an erroring closure is logged and does not kill the task). dunite-oci's existing burst regression test passes unchanged. dunite-download gains an equivalent burst regression test (8 concurrent stores over a cap of 250 converge with eviction passes well under 8). fmt + clippy -D warnings clean; full `cargo test --workspace` green; both burst tests and the core tests verified non-flaky across 10 runs.

#PSA-43

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

nrupard added 1 commit

2026-06-03 20:51:07 +02:00

test(core): pin DebouncedEvictor burst test to current_thread (PSA-43 review)

Checks / fmt + clippy + test (pull_request) Successful in 29s

Details

create-release / create-release (pull_request) Has been skipped

Details

80eaca80fb

Code review: the burst-coalescing test asserts that 100 synchronous pokes (no await between) collapse to a single queued run, which holds only when the background task cannot be scheduled mid-burst. `#[tokio::test]` already defaults to a current-thread runtime, so the test was correct, but the invariant was implicit. Pin `flavor = "current_thread"` explicitly so it does not depend on the macro default. No behavior change.

#PSA-43

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>