perf(#4842): freeze the CLI/cron sidebar cache key during streaming (stop the per-poll re-query that pins CPU) - #4908
nesquena-hermes wants to merge 1 commit into
Conversation
/api/sessions re-ran the expensive CLI/cron session projection on every poll while a turn was streaming — a major cause of the multi-second sidebar latency + 100% CPU on cron-heavy installs (continuing #4672/#4808/#4889). Root cause: the CLI/cron projection is gated by _CLI_SESSIONS_CACHE, whose key folds in _sqlite_file_stat_cache_key -> _sqlite_content_fingerprint (MAX(rowid) FROM messages). During a live turn the gateway writes a message row per streamed delta, so that fingerprint advances on essentially every ~5s poll — busting the cache and re-running the full candidate-join + projection (and the lineage-metadata pass) on every poll, contending for the same SQLite/global lock the streaming worker holds. The route-level session-list cache already froze its key during streaming (#4808 _session_list_cache_streaming_freeze_marker), but that freeze never reached this inner CLI-sessions cache, so the heavy CLI/cron query still re-ran whenever the outer cache validated. Fix: while any stream is active, the CLI-sessions cache key folds in the same stable streaming-freeze marker (keyed only on the set of active stream ids) instead of the volatile content fingerprint, and the cache TTL widens to a streaming window. The projection is reused across polls mid-stream and rebuilt at most once per window. The instant a stream starts/stops the marker changes, so freshly-finished rows surface promptly. Structural mutations (cron completion, new/renamed/archived sessions, attention) now also clear the CLI cache via the existing session-list-changed listener — those signals never fire per streamed token, which is what makes the freeze safe. Idle behavior is byte-identical (freeze only engages with active streams). Applies to both the active-profile and all-profiles cache paths. Adds tests/test_issue4842_cli_sessions_streaming_freeze.py (6 tests): marker idle/stable/changes semantics, the core 'key stable across message writes while streaming' guarantee, idle key still advances with the fingerprint, streaming TTL > idle TTL, and the structural-mutation listener clears the CLI cache.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| ### Fixed | ||
|
|
||
| - **`/api/sessions` no longer re-runs the expensive CLI/cron session projection on every poll while a turn is streaming**, a major cause of the multi-second sidebar latency and 100% CPU on cron-heavy installs (#4842, continuing #4672/#4808/#4889). The CLI/cron sidebar projection is cached, but its cache key folded in a state.db content fingerprint (`MAX(rowid) FROM messages`) that advances on every streamed message row — so during a live turn the frontend's ~5s poll always missed the cache and re-ran the full candidate-join + projection (and the lineage-metadata pass), contending for the same SQLite/global lock the streaming worker holds. The route-level session-list cache already froze its key during streaming (#4808), but that freeze never reached this inner CLI-sessions cache. Now, while any turn is streaming, the CLI-sessions cache key folds in the same stable streaming-freeze marker (keyed only on the set of active stream ids) and its TTL widens, so the heavy projection is reused across polls and rebuilt at most once per streaming window instead of once per poll. Structural sidebar mutations (cron completion, new/renamed/archived sessions, attention) clear the cache directly, so nothing user-visible lags under the freeze; idle behavior is unchanged. |
There was a problem hiding this comment.
Per this repo's policy, CHANGELOG.md is maintained exclusively by the release process via release: vX.Y.Z commits authored by the release agent — individual contributor PRs don't touch it directly. If the release agent picks up ## [Unreleased] sections from PR bodies or separate tooling, this direct edit may conflict with or duplicate that output.
Rule Used: Do not flag missing CHANGELOG.md updates on indivi... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| try: | ||
| from api.models import clear_cli_sessions_cache | ||
| clear_cli_sessions_cache() |
There was a problem hiding this comment.
clear_cli_sessions_cache silently also clears sidecar_metadata_cache on every structural mutation
clear_cli_sessions_cache() unconditionally calls clear_sidecar_metadata_cache() as well (by design for "explicit reset" paths like test isolation). Wiring it into _on_session_list_changed now clears the sidecar projection cache on every cron completion, rename, archive, or attention event — not just when the CLI cache itself needs a cold start. The sidecar cache is stat-keyed and self-invalidating, so no data is lost, but it forces an unnecessary cold rebuild on the first post-mutation access. If sidecar reads are on the hot path, consider calling _CLI_SESSIONS_CACHE.clear() directly here (with the lock) rather than going through clear_cli_sessions_cache, to avoid the sidecar side-effect.
|
I profiled this locally against the PR head ( On the profile-switch path, the The long tail has moved elsewhere. The slowest post-switch request in my traces was So on this path, I am no longer seeing the repeated CLI/cron sidebar rebuild as the bottleneck. |
|
Follow-up from local profiling on the I can no longer reproduce Switching The remaining delay in these traces is elsewhere. The slowest post-switch request was So on this path, the old sidebar/session rebuild bottleneck is no longer showing up locally. If nobody still has a repro on the intended streaming/sidebar path, this PR looks safe to close. |
|
Read the full diff at What this actually changesThe inner CLI/cron cache key is built in _streaming_marker = _cli_sessions_streaming_freeze_marker()
db_state_key = _streaming_marker if _streaming_marker is not None else _sqlite_file_stat_cache_key(db_path)The new Two things I checked that make the freeze safe
Reviewer validation + the remaining tailrodboev's two profiling passes against this head ( VerdictCI is green across the 3.11/3.12/3.13 matrix + browser-smoke + lint, the new |
|
Shipped in v0.51.665 (Release XU, just deployed) — greenlit by Nathan to move through. Freezes the inner _CLI_SESSIONS_CACHE key during streaming (the layer #4672/#4808/#4889 didn't reach), so /api/sessions stops re-running the heavy CLI/cron projection on every poll mid-stream. Gate: Codex SAFE + Opus SHIP (applied one doc-only accuracy fix re the 30s TTL backstop for externally-driven changes), suite 10614. Verified on prod (freeze marker live). |
… — TTL is the backstop for externally-driven changes Opus SHIP-WITH-FIXES (no code change required): the comment/docstring overstated that the session-list-change listener covers ALL structural mutations. In-app mutations fire it, but externally-driven changes (scheduled cron completion, external CLI writes) don't — for those the 30s streaming TTL is the load-bearing backstop (≤30s bounded, self-healing lag). Comment-only accuracy fix; logic unchanged (both gates: perf fix correct, freeze effective, staleness bounded).
v0.51.665 — Release XU: freeze CLI/cron sidebar cache key during streaming (nesquena#4908, nesquena#4842) # Conflicts: # CHANGELOG.md
2 participants
Summary
A structural performance fix for #4842 (continuing the #4672 / #4808 / #4889 line):
/api/sessionsre-runs the expensive CLI/cron session projection on every poll while a turn is streaming, which is the dominant cost in the multi-second sidebar latency + 100% CPU that cron-heavy installs keep reporting.This is a PR candidate — opening for review, not auto-merge. It addresses a layer the prior fixes did not reach (see "Why the earlier fixes didn't fully land" below).
Root cause
The CLI/cron sidebar projection is cached in
_CLI_SESSIONS_CACHE. Its key is built in_resolve_cli_sessions_context→_sqlite_file_stat_cache_key→_sqlite_content_fingerprint, which folds inMAX(rowid) FROM messages. During an active chat turn the gateway/CLI writes a message row per streamed delta, so that fingerprint advances on essentially every/api/sessionspoll. The frontend polls every ~5s while streaming → every poll misses the cache → the full candidate-join + projection (and thevisible_lineage_metadatapass) re-runs, contending for the same SQLite / global lock the streaming worker holds.A reporter's profiler trace on v0.51.645 shows this directly:
get_cli_sessionsat ~5000ms andvisible_lineage_metadataat ~1200ms per request, with the main thread blocked inread_importable_agent_session_rows → _project_agent_session_rows → cur.fetchall().Why the earlier fixes didn't fully land
_session_list_cache_streaming_freeze_marker). It never reached this inner CLI-sessions cache, so the heavy CLI/cron query still re-ran whenever the outer cache validated.Fix
Propagate the same streaming-freeze idea to the inner cache:
_cli_sessions_streaming_freeze_marker()inapi/models.py— keyed only on the set of active stream ids (mirrors the route-level Performance issues are back #4808 marker)._resolve_cli_sessions_context(and the all-profiles path) fold that stable marker into the cache key instead of the volatile content fingerprint — so per-token message writes no longer bust the cache._cli_sessions_cache_ttl_seconds()widens the TTL (5s → 30s) while streaming, so the fixed poll cadence can't force a rebuild on every poll._on_session_list_changed(the existing structural-mutation listener) now also clears the CLI cache. Structural signals — cron completion, new/renamed/archived sessions, attention — fire this listener; per streamed token never does. That is what makes the freeze safe: real changes still surface promptly, but the streaming write storm doesn't.Net effect: while a turn streams, the heavy CLI/cron projection is reused across polls and rebuilt at most once per streaming window instead of once per poll. The instant a stream starts/stops, the marker changes and the just-finished turn's rows are picked up. Idle behavior is byte-identical (the freeze only engages when there are active streams).
Tests
tests/test_issue4842_cli_sessions_streaming_freeze.py(6 tests):Nonewhen idle, stable for the same stream set (order-independent), changes when the set changes;Regression sweep (all green):
test_issue4842_cron_projection_perf,test_issue4385_cron_archive_reappears,test_cli_sessions_cache_fingerprint,test_issue3930_source_filter_pushdown,test_claude_code_session_import(incl.test_get_cli_sessions_cache_invalidates_when_sqlite_wal_changes),test_session_sidebar_cache,test_session_events,test_session_events_http_integration.Honest scope note
I could not reproduce the full 5s locally — my dev state.db's cron sessions carry ~0 messages each, so the join is cheap here; the reporter's clearly carry far more. This fix is targeted at the mechanism the trace points at (per-poll re-query under streaming write contention), which is sound and net-positive for every large/cron-heavy install regardless of that one reporter. It is not claimed as a verified end-to-end cure for that specific user until it can be confirmed against their data shape.
Refs #4842.