feat(pool): decay warm-process refill target after idle TTL by epicvinny · Pull Request #182 · kvcache-ai/AgentENV

epicvinny · 2026-08-18T00:46:58Z

What

Adds PoolConfig::idle_ttl to the warm-pool crate: when no warm-process acquisition happens within the TTL, the maintenance worker decays the geometric refill target back to the low watermark and drains the excess warm entries. Wired as [pool.firecracker] idle_ttl_secs (default 600, 0 disables decay).

Why

The pool's fill target only ever ratchets up on acquisitions, so a single burst pins the excess warm Firecracker processes, and the memory they hold, for the lifetime of the server. On bursty shared hosts (business-hours sandbox creation, idle overnight) this wastes memory with no recourse short of restarting the server.

Related issue

Closes #180

Scope and non-goals

Included: idle-TTL decay of the refill target and drain in warm-pool, config plumbing (idle_ttl_secs), docs (config/default.toml, configuration reference), unit tests.
Excluded (non-goals from Warm Firecracker pool never decays its refill target after a burst #180): changing the geometric growth policy; per-entry TTL for parked processes; applying decay to the network-slot or ublk pools (they keep idle_ttl: None since they hold no guest processes).

Design and behavior changes

The maintenance loop tracks the last-acquisition instant and wakes on the TTL boundary.
On expiry, the fill target decays toward the configured low watermark and the excess parked entries are drained through the existing drain path.
Any acquisition resets the idle clock, so busy pools never decay.
idle_ttl_secs = 0 preserves the exact previous ratchet behavior.

Compatibility and operations

Public API or generated protocol: N/A, no API change.
Configuration or defaults: new optional key [pool.firecracker] idle_ttl_secs (default 600). Old builds ignore the key. Operators wanting the historical behavior set 0.
Snapshot manifest, artifact layout, or storage format: N/A.
Upgrade and rollback: safe both ways; rollback simply ignores the new key.
Host requirements, permissions, ports, or dependencies: N/A.

Validation

make fmt (via cargo fmt --check)
make clippy (via cargo clippy -p warm-pool)
make test-unit (scoped: cargo test -p warm-pool, 16 passed, incl. 3 new)
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 --check         # clean
cargo clippy -p warm-pool # clean
cargo test -p warm-pool   # test result: ok. 16 passed; 0 failed
# new tests:
#   idle_ttl_decays_fill_target_and_drains_to_low_watermark
#   maintenance_worker_wakes_on_idle_ttl_and_drains
#   acquisitions_reset_idle_ttl_clock

Skipped checks and reasons: integration tests require root and /dev/kvm, not available on the Windows dev box where this was written. The behavior has also been validated on our staging fleet (GKE KVM nodes) via the fork build.

Risks and reviewer notes

Default 600s means existing deployments gain decay on upgrade. If reviewers prefer opt-in, I can flip the default to 0.
Review focus: crates/warm-pool/src/lib.rs (TTL bookkeeping in the maintenance loop) and src/cfg.rs (config wiring).

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.

The warm pool ratchets its geometric refill target up after bursts of acquisitions and never decays it, so a single burst pins the excess warm processes (and their memory) for the lifetime of the server. Add PoolConfig::idle_ttl: when no acquisition happens within the TTL, the maintenance worker decays the fill target back to the low watermark and drains the excess entries. Acquisitions reset the idle clock. Wire it as [pool.firecracker] idle_ttl_secs (default 600, 0 disables decay), documented in config/default.toml and the configuration reference. The network-slot and ublk pools keep the historical ratchet behavior (idle_ttl: None): they hold no guest processes, so idle decay buys nothing there. Tests: idle_ttl_decays_fill_target_and_drains_to_low_watermark, maintenance_worker_wakes_on_idle_ttl_and_drains, acquisitions_reset_idle_ttl_clock (cargo test -p warm-pool: 16 ok).

github-actions · 2026-08-18T00:49:09Z

✅ OpenCodeReview: Review complete: 0 finding(s) across 7 selected item(s).

decay_fill_target_if_idle reset last_acquisition on expiry, so a single action computation consumed the decay event: if the drain cycle only partially succeeded (e.g. resource cleanup failed mid-cycle) or another caller computed the action first, the next computation fell back to the high watermark and retained the excess for another full TTL. Keep a persistent decaying flag instead: set on expiry, cleared by any acquisition, and cleared only once the pool reaches the decayed fill target, so the drain target stays pinned to the low watermark until the excess is actually drained. PoolConfig::validate now normalizes idle_ttl = Some(Duration::ZERO) to None, matching the documented external config semantics of 0 = never decay. A zero TTL made idle_ttl_remaining always return zero, so the maintenance worker scheduled back-to-back cycles (busy loop) even after reaching the target. Tests: widen TTL/sleep margins so loaded CI workers cannot cross the deadline (pre-expiry margin is now 800ms instead of 50ms), poll the maintenance-worker test with a 10s deadline instead of a fixed 300ms sleep, and cover the zero-TTL normalization and the non-consumable decay.

epicvinny · 2026-08-18T01:38:22Z

One related finding was reported without an inline thread, addressed here for completeness: PoolConfig::validate() now normalizes idle_ttl: Some(Duration::ZERO) to None (00784ce), matching the documented external config semantics of 0 = never decay. Previously a zero TTL made idle_ttl_remaining always return zero, so with maintenance enabled the worker scheduled cycle after cycle (busy loop) even after the pool reached its target. The normalization happens in the single validation point, so all internal uses can assume Some(d) with d > 0.

The decay decision was not an atomic snapshot: last_acquisition, decaying, and fill_target lived under separate locks, so a computation concurrent with an acquisition could observe the new acquisition timestamp together with the stale decaying flag and old fill target and return a low-watermark Drain for demand that had just resumed; the unconditional decaying clear could also erase a newer decay transition started by another caller. Move the three fields into a DemandState struct under a single mutex. compute_maintenance_action and record_acquisition each hold the lock for their full transition, so the decay lifecycle, fill target, and idle clock always move together. Tests: acquisition_clears_decay_state_before_next_computation covers the resumed-demand-mid-drain transition deterministically, and concurrent_acquisition_and_decay_keep_state_consistent races four threads of release/acquire/compute against a 5ms TTL, asserting the fill target stays within the watermarks.

Address remaining review threads on idle-TTL decay: - A computed Drain action could go stale before execution: an acquisition after the computation clears the decaying state via record_acquisition, but the maintenance cycle still drained the previously returned count toward the low watermark. Maintenance drains now claim each resource through WarmPool::try_drain_one_for_maintenance, which re-checks the drain decision under the demand-state and pool locks at execution time, so resumed demand actually cancels an in-progress decay. - Decay completion was decided from a pool length sampled separately from the demand state, so a concurrent release could strand resources above the decayed target for another full TTL. Completion is now marked only when try_drain_one_for_maintenance verifies, under lock, that the pool actually reached the decayed fill target. - TTL tests no longer rely on wall-clock sleeps: DemandState methods take an injected clock and tests rewind last_acquisition directly; only the maintenance-worker condvar test keeps a bounded polling deadline.

…rain lock order

…ed target

… under both locks

… length