Evict terminal chat runs to bound ChatRunRegistry memory#61
Draft
TYRMars wants to merge 1 commit into
Draft
Conversation
ChatRunRegistry retained every terminal (completed/failed/cancelled) run in its in-memory map for the whole process lifetime, leaking one entry per distinct conversation_id ever served. Add lazy eviction on the existing mutation paths (no background task): - TTL: terminal runs are dropped 5 minutes after completion, leaving a window for late WS reconnects to replay final frames. - LRU backstop: cap retained terminal runs at 256, dropping the oldest by terminal timestamp, to guard against a burst completing inside the TTL window. Active runs are never evicted. Stamp terminated_at on the first terminal transition (mark_terminal) and sweep from try_start / terminal push_frame / update. Closes #50.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #50.
ChatRunRegistrynever evicted terminal (completed/failed/cancelled) runs from its in-memoryinnermap, so memory grew unbounded over the lifetime of a long-lived server — one retained entry per distinctconversation_idever run, each holding up toMAX_EVENTS_PER_RUNbuffered records plus a live broadcast channel.Fix
Lazy eviction on the existing mutation paths — no background task:
TERMINAL_RETENTION_MS(5 min) after they finish, leaving a window for late WS reconnects to replay the final frames and observe channel closure before the state disappears.MAX_RETAINED_TERMINAL(256), dropping the oldest by terminal timestamp — guards against a burst of conversations all completing inside the TTL window.Active runs (
terminated_at == None) are never counted against the cap or evicted. The terminal timestamp is stamped exactly once on the first terminal transition viamark_terminal, which also drives the one-shot abort-handle cleanup. The sweep (prune_terminal_locked) runs fromtry_startand from the terminal branch ofpush_frame/update, all already under theinnerwrite lock, so it adds no new locking and only fires on run start / completion (not per token-delta).Tests
Added coverage in
chat_runs::tests(via aforce_prunetest seam that injects the clock so TTL is testable without sleeping):terminal_run_is_evicted_after_ttl— kept within the window, swept past itactive_run_is_never_evicted— even with an absurd clockcancelled_run_is_evicted_after_ttlterminal_runs_are_capped_by_lru_backstoprestarting_an_evicted_conversation_succeedscargo test -p harness-server chat_runs::(11 passed) andcargo clippy -p harness-server --all-targets -- -D warningsboth green.Generated by Claude Code