Skip to content

store: a saturated rewind reserve is released only by the TTL, so one busy period can zero out savings process-wide for ~2.8h #190

Description

@amiddavid

The remaining gap

#188 bounded the rewind reserve properly — an entry cap, a byte budget (stash_max_bytes), and an evictable floor so the exemptions cannot consume the whole cache — but a slot is still released only by the TTL, and the store is a single process-wide instance (cmd/context-guru-proxy/main.go builds cfg.NewStore() once).

So a busy period can hold the reserve until entries age out. With the default sliding ttl_seconds: 10000 that is up to ~2.8 h after the last touch, and while the reserve is saturated every new removal is refused across every session sharing the process. The failure mode is not broken markers — #188 fixed that — it is savings falling to zero mid-run.

What #188 deliberately did not do

Per-session partitioning was considered and declined. The reasoning, recorded so it can be revisited rather than re-litigated: no session ever reads another's payload (offload.OwnsKey enforces that), so what is shared is the budget, not the data — and a fair-share cap needs a share number as arbitrary as the max_entries default it would replace, plus an ownership rule for a content-hash key two sessions can legitimately both stash. Sizing against the real resource (bytes) was judged the more honest fix for the same failure.

That narrows the window; it does not close it.

How it was found

Raised in the #188 review (finding on store.go:307) and answered there as partially addressed. Filed so the open half survives the PR.

The options, with their trade-offs

  1. Age-based last-resort eviction. Under saturation, allow a payload past some age to be evicted, counted as an explicitly reported broken promise (a new counter, or cg_stash_missing_total). Restores liveness at the cost of reintroducing bounded and measured irreversibility. This is the direct opposite of #188's thesis, which is why it needs a decision rather than a patch — but "bounded and reported" is a different thing from #187's "unbounded and silent".
  2. Per-session share cap. No session may hold more than a fraction of the reserve; a runaway session refuses for itself while others keep offloading. Bounds the blast radius without any irreversibility. Costs the two design problems named above.
  3. A shorter TTL for payloads specifically. A payload's life is only needed while a marker referencing it can appear in a live request, which is a shorter horizon than a frozen decision's. Cheap, but shortens the window in which a long-idle session can still expand.
  4. Release on session end. The honest version of (3), but there is no explicit session-end signal — sessions are derived per request — so it reduces to an idle timeout.

What should gate the choice

A re-run of iteration 024 on #188's code, reading stash_refused, stash_live/stash_capacity and stash_bytes/stash_max_bytes. If refusals stay at zero the reserve is correctly sized and this is theoretical; if savings collapse mid-run with refusals climbing, the measurement says which of the four is needed and at what threshold. Picking now would be another guess at a default.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions