Repository navigation
perf(sessions): bound projection queue-head scans per write window - #4793
ScriptedAlchemy wants to merge 9 commits into
Conversation
Avoid repeating ordered scans over permanently parked collision rows for every item in a 32-observation write transaction. Keep strict retry fences, atomic rollback, bounded windows, and restart checkpoints. Refs #4756
|
Bound re-arm write batches to actually retryable rows instead of visiting permanently parked collisions as empty batches on every fresh store mount. Add behavioral coverage for interleaved terminal and deferred queue rows, preserved retry history, and zero no-op batches. Refs #4756
|
Follow-up at d265e31 — Refs #4756, partial restart write-amplification reduction. Root cause: the background store-mount retry re-arm paginated every queue row with a positive deadline, including terminal collision rows whose deadline is The new behavioral regression seeds 12 persisted queue entries (six terminal, three deferred, three already due), asserts exactly three re-arms in two write batches of size two, verifies that terminal rows and error history remain unchanged, and checks that another fresh re-arm finds zero write batches. An independent SQLite execution of the same selection/update showed old = 5 batches / 3 updated rows, new = 2 batches / 3 updated rows, and repeat = 0 batches / 0 rows. This is SQL-level corroboration only, not a Bazel Rust test pass. No checkout or Bazel executable is available in this execution environment, so the Rust regression remains uncompiled here; exact-head CI was queued at review time. This does not establish resolution of the reported 125–160 ms projection writes or four-hour CPU saturation; no macOS end-to-end restart profile is claimed. Keep this PR draft and #4756 open pending that evidence. No changes to #4776, release #4763, other owner branches, or CI policy. |
Capture the last retryable deadline through the reader and stop after its bounded writer window, rather than taking an empty writer transaction at startup or after every completed re-arm. Reject nonpositive batch sizes. Exercise registered writer transaction counters, durable daemon remount, later retries, terminal preservation, and invalid limits. Refs #4756
Use the queue retry deadline index for an inexpensive nonterminal readiness probe. Parked collisions must not acquire a writer on idle drains; new observations still project after such a probe. Refs #4756
|
Published 5466c909 as a narrow additional Refs #4756 improvement (not full issue acceptance). Remaining hot path: after a projection becomes a terminal collision, its queue row remains parked at Fix: the pre-writer probe now checks for nonterminal work and uses a separately installed performance index on Independent checks: SQLite WAL reproduction confirmed that a terminal-only probe returns no work while another connection holds a writer, the new covering index is selected by SQLite, and a subsequently inserted ready row is visible. Synthetic 30K-parked-row query-plan checks showed a covering-index probe rather than a full queue scan; these are not end-to-end TraceDecay CPU measurements. Only This does not prove the reported 125–160 ms historical writes, multi-hour 1.7-core CPU usage, or durable Codex discovery continuation across daemon restarts are resolved. Leave #4756 open and this PR draft. The overlapping #4776 draft, decision-model work, and release #4763 were untouched. |
Use indexed deadline readiness to avoid empty writer transactions while all nonterminal rows await their retry clock. Preserve pending coverage, strict sequence authority, and newly admitted work after the probe. Exercise a persisted terminal + deferred queue across reopening with a concurrent SQLite writer reservation and subsequent durable continuation. Refs #4756
|
Follow-up at Root cause: even after the terminal-only fast path, a queue containing no due observations but one or more deferred retries still entered a SQLite writer transaction on every projection poll. A terminal collision plus deferred nonterminal row is sufficient to reproduce the needless writer lease; repeated polls can contend with foreground session writes without advancing the checkpoint. Fix: query the earliest nonterminal deadline through the PR's existing Falsifiable regression: reopen a real profile SQLite store containing a persisted terminal collision and a deferred observation; hold an unrelated Independent check: an isolated SQLite WAL rehearsal confirmed the read-only earliest-deadline query works while another writer holds a reservation, uses the covering retry-deadline index, and sees a subsequently eligible row. This is SQL-level evidence, not Rust/Bazel or host CPU verification. This is a two-file diff, published via a single-parent, exact expected-old-head, non-force fast-forward from |
Probe the lowest nonterminal observation sequence, not the earliest retry clock anywhere in the queue. A later due row must not acquire a writer while an earlier deferred row fences projection. Keep the probe and batched reads on a partial sequence index excluding permanently parked collisions. Extend the durable reopen regression to prove a later due row cannot bypass the deferred head under another SQLite writer reservation; then project both observations in bounded one-item windows. Refs #4756
|
Published Root cause: the previous read-only projection probe used the minimum retry deadline over all nonterminal rows, not the first nonterminal observation sequence. If sequence 2 was deferred but sequence 3 was already due, each idle poll still took a SQLite writer lease and committed an empty projection window despite sequence 2 fencing the later row. Many terminal parked collisions also imposed repeated ordered-prefix work. Fix: probe the lowest nonterminal sequence and compare its deadline. Use an indexed partial active-sequence lookup excluding permanent Regression: extended the real profile-store restart test with a third, already-due observation behind an earlier deferred one. Two polls while an unrelated connection holds An independent SQLite rehearsal with 30,000 parked rows confirmed the old earliest-deadline probe returned the later due row, the sequence-head probe returned the deferred row, and This single-parent commit was applied by exact expected-old-head, non-force fast-forward, changing only |
Scope
Partial remediation for the expensive session-history re-import in #4756. Refs #4756 (not Fixes): the reported multi-hour CPU saturation, cross-provider startup/restart recovery, and complete bounded-write acceptance criteria remain open.
The registered observation projection store previously executed the same
MIN(observation_sequence)query for each queued item inside a transaction of up to 32 items. The projection queue deliberately retains permanently parked collision rows (next_retry_at_micros = i64::MAX). Thus every projected item could re-walk the same long parked prefix, adding avoidable CPU while the profile/project databases import history.This patch reads one ordered, at-most-
maxcandidate window under the same write transaction instead. It excludes permanently parked rows, stops at the first nonterminal row whose retry deadline has not arrived, respects the active rebuild fence, retains per-item projection/rollback logic and finalhas_moreprobe, and changes no schema or public API. The daemon's existing caller setsmax <= 32; this removes repeated queue-head scans without increasing its transaction window.Behavioral regression
New integration test
batched_projection_resumes_after_restart_without_replaying_parked_or_deferred_headsseeds five durable observations, projects the first, inserts a real terminal-collision queue head, postpones sequence 4, and proves:max=1project exactly one observation, then the next batch finishes the fifth; terminal collisions do not report runnable backlog.Follow-up: eliminate empty-queue writer transactions
Commit
7f8147b274c2also skips the projection write transaction when the queue is truly empty, using a constant-workSELECT 1 ... LIMIT 1reader probe. Active queues still enter the writer transaction, including terminal parked rows, preserving the ordered queue's behavior and avoiding another O(parked-prefix) readiness scan.A behavioral regression holds a separate SQLite
BEGIN IMMEDIATEwriter reservation and requires an empty projection drain to finish without taking the writer lease; after rollback it persists a new observation and proves the next drain projects exactly one message. This is bounded idle write-transaction avoidance, not a demonstrated explanation for the reporter's 125–160 ms historical write transactions. Bazel compilation/tests and macOS end-to-end profile remain required.Performance evidence and limits
Using SQLite's unique observation-sequence index in a synthetic 30,000-parked / 1,000-ready queue, an offline 32-item transaction simulation took approximately 59 ms with the per-item query versus 2.4 ms with the single-window query (~25× in this isolated query workload). This is illustrative, not a TraceDecay end-to-end benchmark or evidence the reporter had 30,000 parked heads.
No Cargo run. This execution environment has no Bazel binary or GitHub checkout, so Rust compile and tests remain unverified pending the repository's normal protected CI checks. Do not merge until Bazel tests, full applicable CI, and review succeed. In particular, this PR does not assert complete fix for #4756: Codex partial discovery position is still process-retained and does not persist its scan cursor across daemon restarts, and per-source admission/write amplification needs additional measured investigation.
No release/infra/credentials, branch-protection changes, or modifications to any peer PR.