Repository navigation
fix(retention): verify SQL page progress and expose GC frontiers (#4787) - #4798
ScriptedAlchemy wants to merge 31 commits into
Conversation
Report candidate discharge and replay SQL frontiers for each scanned page. Reject incomplete candidate discharge or nonadvancing replay pages. Add >48-row behavioral regressions for zero-reclamation backlogs. Refs #4787
|
Admit a size-bounded metadata prefix before running SQL JSON eligibility checks, including rows that cannot be compacted. Resolve admitted IDs in one query under the existing writer transaction. Preserve oversized metadata while advancing past it without parsing or blocking later rows. Expose examined metadata bytes through the page report and tracing span. Add database-backed regressions for large ineligible backlogs and an oversized stored document followed by valid compactable work. Refs #4787
Published replay CPU-amplification fix — Refs #4787Follow-up commit 001b1ab is now on this issue-specific PR. It changes only Confirmed defect and correction
The fix first reads identities/sizes, admits a contiguous prefix under a 4 MiB metadata-parsing budget, then evaluates eligibility in one batched primary-key query under the same writer transaction. No per-row query loop is added. Individually oversized metadata is left byte-for-byte intact and advanced past without parsing; later valid work remains reachable. Existing body-verification budget, sanitizer receipts, guarded writes, rollback behavior, and 48-row cap remain in place.
Behavioral evidenceA disposable SQLite 3.46.1 probe executed the original and replacement SQL, instrumenting actual
Added two real Rust database-backed regressions: (1) large ineligible documents require multiple bounded ticks, strictly advance the durable frontier, quiesce, and preserve complete messages/receipts; (2) oversized stored metadata stays unchanged while a subsequent valid row is actually compacted. Existing corruption rollback, stale-write, representation-preservation, and 48+4 progress regressions are unchanged. Validation and ownershipReviewed the exact two-file diff in a separate pass and verified one commit ahead of The SQLite probe is not a repository Rust/Bazel pass. No local Bazel toolchain was available; no Cargo was invoked. New-head CI run https://github.com/ScriptedAlchemy/tracedecay/actions/runs/42434994670 is pending. The relevant Rust suite is The original affected-host 90–110% CPU incident is not yet proven resolved. Its consecutive canonical candidate/frontier and actual tick/commit measurements remain required, and the cold-store JSON cursor is a separate authority. Keep #4787 open; no |
Apply only the two exact Rustfmt changes identified by the blocked repository formatting gate on the last PR head. Refs #4787
|
Refs #4787 — repaired the first exact-head CI failure on this isolated PR. The Bazel New exact-head CI run 42437993937 is queued; no passing Bazel/Rust test is claimed yet, and the previous run's Linux/Clippy work is not an accepted pass. Keep #4787 open pending exact-head tests and affected-host SQL-frontier/CPU/cadence evidence. No changes to draft #4776, release #4763, other owner work, or scheduled tasks. |
Read back the replay frontier in the page's writer transaction before reporting progress. Refuse ignored or rewritten checkpoint writes so the existing transaction owner rolls back metadata changes rather than committing a page that will be visited again. Idle pages do not write. Add real database regressions for an ignored initial checkpoint, a rewritten existing checkpoint, rollback and successful recovery, and write-free idle pages. Preserve existing parsing and body budgets. Refs #4787
ScriptedAlchemy
left a comment
ScriptedAlchemy
left a comment
There was a problem hiding this comment.
Retention implementation review — Refs #4787
Reviewed the published checkpoint repair at 91ffd680d7ec128dbf5cd14d9bcc721b4e457a21, including its two-file diff against 2a42006. This is a code update on this PR, not merely a proposed change.
The previous guard checked an advancing local after value but did not establish that set_gc_meta actually stored it. The new read-back occurs inside the same writer transaction, before returning a successful report; an ignored or rewritten checkpoint now returns an error through the existing rollback owner. Empty pages no longer initialize or rewrite the checkpoint. Existing metadata/body budgets and sanitizer checks remain intact.
The added database-backed Rust regressions cover an ignored initial checkpoint, an existing checkpoint rewritten after INSERT, and an idle page whose checkpoint writes are explicitly forbidden. The refusal tests prove that compaction happened before the failure, then verify restoration of the complete message/receipts, the prior checkpoint and summary revision, followed by successful compaction and quiescence once the injected fault is removed.
Validation remains open: the disposable SQLite reproduction passed 20 assertions, reproducing repeated identical 48-row pages under both injected checkpoint faults and recovery through 48+4 pages after removing the faults. That is not a Rust/Bazel test pass, and these injected faults are not established as the affected host's cause. No local Bazel was available; no Cargo was run. Exact-head CI run 42442366451 was queued when checked. Require the relevant Bazel unit tests and applicable CI on this head, rather than inheriting an earlier head's green checks.
Not an approval or incident-resolution claim. Keep Refs #4787: closure still requires the original CPU/frontier acceptance evidence in master. Consecutive committed frontiers/candidate discharge and measured tick timing are needed to distinguish a stalled page from an advancing backlog. No auto-merge; the separate mixed #4776 remains draft and excluded from merging.
| pub rows_skipped_oversized: u64, | ||
| /// Metadata bytes admitted to SQL eligibility parsing, including ineligible | ||
| /// rows and any prefetched rows deferred by the separate body byte budget. | ||
| pub metadata_bytes_examined: u64, |
There was a problem hiding this comment.
Measurement/acceptance note: this is the admitted metadata size for eligibility parsing, including prefetched rows the body budget may defer. It is not total physical I/O, unique committed bytes, or a CPU measurement; the initial size-discovery query runs before this admission. Keep the scope explicit when evaluating #4787, and correlate it with committed frontiers and actual page/tick timings before claiming the sustained CPU incident is resolved.
Refuse an unchanged bootstrap page or a checkpoint that did not persist, even when its SQL UPDATE reports one affected row. Roll back enrolled refs and seed writes through the existing writer transaction on refusal. Add a database-backed 52-owner regression proving repeated 48-row pages fail closed when a trigger rewrites their frontier, then resume through 48+4 pages and quiesce without collecting the live shared body. Refs #4787
Published canonical-body bootstrap checkpoint guard — Refs #4787Commit Bug class: the former Fix: after each nonempty bootstrap stage, require a changed frontier, then read back all bootstrap progress fields within the same registered writer transaction and reject mismatches. The existing rollback owner aborts enrolled owner references and body-candidate seeds together. No SQL state is reset or discarded; the durable owner/candidate invariants remain unchanged. Regression added: 52 raw owners sharing one still-live canonical body, with an Validation: compared the exact two-file commit diff against parent Still open: this is a verified failure class and rollback guard, not proof this trigger or checkpoint rewrite caused the reported 90–110% production CPU. Correlate consecutive committed GC/replay frontiers and candidate discharge with per-tick duration, rollback diagnostics, and affected-host CPU before a |
Apply the exact formatting required by the failing Bazel rustfmt gate in the new canonical-body frontier rollback regression. Refs #4787
|
Refs #4787 — fixed the next exact-head CI blocker on isolated PR #4798. Bazel CI run 42443962770 failed only the repository Rust formatting gate at Exact-head CI run 42446728946 is queued, not passing yet. Repository Rust/Bazel regressions, affected-host 90–110% CPU and consecutive SQL frontiers/tick timings still require validation. Keep this PR unmerged until the relevant gates pass, keep issue #4787 open until master-proven acceptance, and leave mixed draft #4776 and release #4763 untouched. |
Bound replay eligibility admission by the conservative body verification budget before parsing metadata. Deferred rows no longer incur repeated JSON validation on successive ticks. Assert exactly-once metadata admission across database-backed multi-page body-budget regressions. Refs #4787
|
Refs #4787 — published focused replay-CPU work to this issue-specific PR at Defect found in the previous bounded-parsing fix: Falsifiable behavioral regression: extended the real Rust An independent disposable SQLite size-admission analog with four ~700 KiB canonical/replay rows reproduced the discrepancy: the old 48-row admission submitted 2,800,244 bytes for eligibility despite processing only two rows (1,400,122 bytes of those rows); the corrected admission submits 1,400,122 bytes and resumes the remaining two on the next page. This is not a repository Rust/Bazel or affected-host CPU pass. No Bazel toolchain/checkout is available locally; no Cargo was run. Exact-head CI run 42451037206 is queued; exact-head Rust tests/formatting/Clippy must still pass. Keep issue #4787 open and retain |
Measure each canonical-body GC and replay-metadata pass from admission through successful writer commit. Include elapsed milliseconds and admitted metadata bytes in the existing daemon progress events so an affected host can distinguish stalled page retries from costly but advancing backlogs. Preserve the transactional progress guards. Refs #4787
|
Refs #4787 — published focused instrumentation on this issue-specific PR, commit a1b2bcf. The new daemon retention page events report wall-clock I independently checked the exact three-file, single-parent diff against 8e044d4, then published the update with This is a diagnostic improvement, not an affected-host incident resolution. Affected-host CPU samples and consecutive committed SQL frontier/candidate/tick measurements are still needed. Keep issue #4787 open and PR #4798 under validation, without auto-merge. Protected mixed #4776 remains draft and excluded from merging; release #4763, production, and other issue-owner branches are unchanged. |
Apply the exact three rustfmt transformations in the latest failed repository gate without changing retention behavior. Refs #4787
|
Refs #4787 — published CI remediation Reviewed concurrency: #4813 also edits |
|
Refs #4787 — exact-head Bazel Rust validation is now available on CI run 42458529667, head Relevant results: New CI blocker, not a proven #4787 regression:
The separate Linux and Windows legs were still queued/in progress on last check. No CI was weakened and no unrelated owner branches were edited. Keep this PR unmerged until required gates are resolved and independently reviewed. The underlying affected-host 90–110% CPU incident still lacks post-fix repeated-tick SQL-frontier, candidate-count, wall-time and CPU observations; this exact-head Bazel pass establishes code regressions, not production incident resolution. Issue #4787 remains open. #4776 must remain draft/unmerged; #4763 read-only. |
Use SQLite record byte lengths without materializing raw text. Defer storage-kind reads and unnecessary canonical lookups until admission, and do not read canonical size scalars beyond an oversized BLOB. Skip eligibility parsing when the stored representations already exceed the page budget, while preserving the rows and advancing the frontier. Add an isolated Linux database regression measuring actual read syscall bytes for oversized inline, metadata, and canonical payloads. Verify subsequent valid compaction, complete byte preservation, and quiescence. Refs #4787
ScriptedAlchemy
left a comment
ScriptedAlchemy
left a comment
There was a problem hiding this comment.
Refs #4787 — independent review of the new replay read-cost patch at a945a8b85b4a1f3f146872ff226d3319014b9b21 (one nonforce fast-forward after 3924b363). This changes only the replay size/admission path and a Linux process-I/O behavioral regression; it does not move or duplicate the protected #4776 work.
I exercised the exact new size-discovery SQL in a disposable SQLite 3.46.1 database with 52 inline rows, multibyte/NUL text and a deliberately lowered SQLITE_LIMIT_LENGTH=1024: pages returned 48, then 4, then 0, with exact byte lengths. An ordinary full-content read failed at that limit. A separate file-backed overflow fixture (8 MiB inline body, 8 MiB metadata, 8 MiB canonical BLOB, mmap off) observed only ~8–16 KiB /proc/self/io rchar per isolated size-probe query; that corroborates the avoided full-text materialization, not a daemon CPU benchmark. The older LENGTH(CAST(... AS BLOB)) implementation fails the tight-limit probe.
Review scope: the new conditional canonical join, query budget admission, durable frontier guard and the child-process read-cost regression; no independent correctness issue found in those paths. The new read_cost test is Linux-only and exact-head Bazel CI is still queued, so this is not Rust/Bazel/macOS/Windows execution evidence or proof the affected-host 90–110% CPU incident is resolved. Require exact-head CI and real successive committed frontiers/candidates, per-tick timing, rollback evidence and CPU samples before closing #4787. No approval to merge is implied.
Apply the exact Bazel rustfmt_check_2024 correction at read_cost.rs:62. No runtime or test behavior changes. Refs #4787
|
Refs #4787 — published CI-format repair Exact-head blocker: run 42470152899 failed The superseding exact-head CI run 42477719995 is queued, not a pass. No local Bazel executable available; no Cargo used. The earlier |
Refs #4787 Use a fresh exact-SQL test connection for each measured replay page, so SQLite cannot reuse a warmed per-connection page cache. Remove the cache/mmap/shrink PRAGMAs rejected by the registered SQL authority. Keep the byte-read thresholds, 48-row semantics, frontier assertions, post-repair replay equivalence, and idle checks unchanged.
|
Refs #4787 — repaired the exact-head Linux Bazel regression in the isolated issue PR. CI run 42477719995, head Published New exact-head Bazel CI 42491853590 is pending. I did not run Cargo or claim local Bazel/native macOS tests. The older fleet run also had three scheduler tests known red on master and other broad-suite timeouts; those are not solved by this retention-only correction. Keep #4798 unmerged until exact-head tests and review pass. Original 90–110% affected-host CPU condition still lacks consecutive post-fix candidate discharge, durable SQL frontiers, tick durations and CPU measurements. Issue #4787 remains open; #4776 remains draft/unmerged, #4763 read-only, #4813 separately owned. |
|
Refs #4787 — exact-head Bazel CI update for The issue-owned regression now passes: Remaining new CI blockers: Linux fleet still fails No new retention-source defect was established from these failures and no unrelated owner code was altered. Keep #4798 unmerged and #4787 open. The original ~90–110% affected-host CPU still requires successive committed GC candidate discharges and replay SQL frontiers, per-page/tick wall-time/CPU observations, and stable host CPU after the fix before incident closure. Protected mixed #4776 remains DRAFT/unmerged; #4813 is separately owned; #4763 and production/infra/tags/credentials remain untouched. |
Preserve the failed-mount fixture repair from 4984a54 byte-for-byte in its own reviewable change above the shared scheduler repair #4821. The rollback journey uses the existing session-retrieval opener, checks the authoritative session path, and verifies both reopened projects bind their own session stores. No deadline or assertion is removed, and the malformed graph must still refuse admission. This separates CI fixture work from the retention-specific #4798 review without reverting fixes, rewriting history, or editing #4813. It is not evidence that the affected-host retention CPU incident is resolved. Refs #4787.
Join the reviewed #4821 and #4827 prerequisite ancestry without changing the existing #4798 tree. Both prerequisite file blobs already match this head exactly. Preserve all retention source, behavioral tests, and concurrent history while allowing a focused six-file retention review above the prerequisites. Do not merge into the prerequisite branch. Land prerequisites in master, retarget, revalidate, and retain the issue acceptance hold. No changes from #4813, held #4776, or decision-model work are moved. Refs #4787
ScriptedAlchemy
left a comment
ScriptedAlchemy
left a comment
There was a problem hiding this comment.
Refs #4787 — reconciled and verified the issue-specific review scope.
af1045000305250ac1cefd94e87ea1ddeefc201c was published with force=false, expected prior head 4984a54fc4cf6cd71d2191d60a80639b1f0b183f. The first-parent source tree is IDENTICAL (3f493dd3a148e21f7bbfe8f9f59a14392d00dd8e): GitHub compare reports no changed files. The second parent is the explicit fixture prerequisite #4827, itself based on existing #4821. Compare against that prerequisite now reports exactly six retention files, and the live PR confirms six changed files.
The scheduler fixture is the exact existing #4821 blob 7342786381bcaf3c08454d2dc2711a6c731e7151; the failed-mount fixture is the exact existing #4798 blob 51efc83f65cffb7e27cb3a13eb5297684850f79d, now reviewed separately in #4827. No peer commit was reverted, no assertion or deadline relaxed, and no retention implementation/test removed. This is a scope/ancestry correction, NOT a new CPU-performance claim.
Correction to the earlier blocker: #4813's admission changes and this PR's outcome-log additions occupy separate maintenance.rs hunks. Filename overlap alone did not establish a merge conflict. #4813's current branch and implementation are untouched; combined behavior remains subject to integration tests.
Landing order: #4821 into master; retarget #4827 to master and validate/land through its own review; then retarget #4798 to master and revalidate. DO NOT merge #4798 into its temporary prerequisite branch. Do not auto-merge. The normal PR workflow has no base-branch filter, so this stack does not intentionally skip Bazel CI.
No local Bazel executable is available; no Cargo used. Prior source-equivalent run 42536602973 had passed Repository gates but had not completed Linux at inspection. New exact-head CI is still required. Affected-host consecutive committed SQL-frontier/candidate progress, actual tick timing/CPU and final master acceptance remain missing. Keep #4787 open and retain all protected #4776/Kev and release holds.
Expose the phase-qualified raw, observation and canonical-body SQL frontiers before and after each successful bootstrap page. Populate these only after the same-transaction checkpoint readback succeeds; collection and idle pages cannot masquerade as bootstrap progress. Log the frontiers with the already measured page cost and discharge counts. A real SQLite-backed regression checks three distinct phases, the durable cursor after each tick, retained live content, collection and quiescence. The existing 52-owner rollback/48+4 regression now asserts its committed 0->48->52 frontier values. Refs #4787; not incident acceptance. No CI, dependency, release, or concurrent writer-admission change.
ScriptedAlchemy
left a comment
ScriptedAlchemy
left a comment
There was a problem hiding this comment.
Refs #4787 — published focused bootstrap SQL frontier diagnostics at ce112eff, then independently re-read the complete three-file commit diff and compared it with the current master implementation.
Previously unobservable: canonical-body GC emitted roots_scanned=48 / bodies_scanned=48 while bootstrapping but no before/after raw/observation/body SQL checkpoint. A repeated page could not be distinguished from consecutive committed progress in the daemon log without separate database access. This change logs phase-qualified bootstrap_frontier_before and bootstrap_frontier_after only after save_progress has read back the persisted checkpoint inside the writer transaction. Collection/idle pages use not_scanned (and retain the existing candidates_discharged and elapsed_ms fields). It does not change GC eligibility, page limit, or scheduling.
Behavioral regression: a real SQL test fixture seeds a referenced raw owner, observation and canonical body; three consecutive ticks must show raw:0→1, observations:0→1, and bodies:→<actual hash>, matching each independently re-read SQL checkpoint. The next tick must discharge the candidate without deleting the referenced body, then quiesce with no fabricated frontier. The existing trigger-rewrite rollback + 52-owner 48+4 recovery regression now also requires committed raw:0→48→52 readings. The patch touches only canonical GC source/tests and the existing daemon outcome log, leaving #4813 writer-admission hunks and the prerequisite fixture PRs unchanged.
Validation hold: local checkout/Bazel is unavailable, so the new Rust regression has not executed here; no Cargo was used. Exact-head hosted Bazel run was queued at inspection. Require nonzero //crates/tracedecay-global-db:unit_test and applicable daemon/Clippy/format checks before merge. This improves diagnosis, not proof that the originally reported affected-host 90–110% CPU is resolved. Consecutive host CPU/tick/frontier evidence and complete acceptance on master are still absent; keep #4787 open. #4776 and Kev drafts remain protected, #4763 read-only.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Refs #4787 — new merge/verification dependency blocker (2026-11-09 UTC). Current #4798 head |
Rows skipped by replay eligibility were not counted as skipped, so the reported 48 scanned / 0 compacted / 0 skipped contradicted what the page actually examined. Include those intact rows in the skip count; keep oversized skips as a subset and leave the durable frontier, eligibility, body reads and scheduling unchanged. Assert the real SQLite-backed 48+4 ineligible pages, five preserved representation kinds and multi-tick metadata-budget pages report true skipped counts while preserving content and quiescence. Refs #4787
ScriptedAlchemy
left a comment
ScriptedAlchemy
left a comment
There was a problem hiding this comment.
Refs #4787 — published and independently diff-reviewed 9a1c8a27d0 as a two-file retention diagnostics correction, exact expected-old-head dfd0c6a106, nonforce fast-forward.
Confirmed issue-symptom defect: replay compaction increments rows_scanned and commits a strictly advancing store-ID frontier when an eligibility check rejects a structured, external or otherwise noncompactable representation, but did not count that preserved row in rows_skipped. Thus a legitimate page could log 48 scanned / 0 compacted / 0 skipped—exactly the ambiguous pattern reported in #4787—even while it committed progress. The new final else counts these ineligible, preserved rows; rows_skipped_oversized remains a subset. No SQL eligibility, body reads, cursor, byte budget, scheduling, or retention/deletion behavior changes.
The existing real database-backed Rust regressions now require (a) five distinct preserved representation rows -> five skips, (b) 52 ineligible rows -> 48 + 4 scanned and skipped with durable frontiers 0→48→52 and a subsequent idle tick, and (c) each admitted bounded large-ineligible-metadata page -> rows_skipped == rows_scanned, with bytes/frontiers, original messages and receipts preserved. The previous implementation fails these new count assertions. I checked the published commit diff: exactly those two files, one added count branch, documentation and assertions; no test or policy weakening.
Verification hold: local Bazel/checkout is unavailable; no Cargo. New exact-head CI #42566057533 is pending. Require nonzero bazel test //crates/tracedecay-lcm:unit_test --test_output=errors and the other issue-specific/Bazel gates. No affected-host 90–110% CPU or consecutive process/tick evidence is available: this fixes truthful diagnostics, not a proven CPU root cause or issue closure.
Separate integration blocker: #4827 CI #42555076056 completed the Linux fleet with 182/183 targets passing; the unrelated runtime_acceptance_suite::production_lsp_negotiates_and_projects_canonical_context test failed at acknowledge daemon LSP frame: DeadlineExceeded (140/141 tests in that target). //crates/tracedecay:unit_test passed, so the failed-mount fixture is not the observed red target. The prerequisite #4834's explicit Linux/Windows run #42560828022 is in progress. Do not weaken the LSP assertion or infer baseline causality before comparing runs.
#4798 is still stacked behind #4827 → draft #4834 and must not merge into that prerequisite or bypass its review. #4813 is separately owned; protected #4776 and Kev/decision-model #4797 remain DRAFT/unmerged, release #4763 read-only. No master merge, production changes or schedule changes.
Mounted session stores can commit identical 48-row pages in the same maintenance cycle. Previously their canonical and replay operator lines had no store identity, so interleaved advancing SQL frontiers appeared to be the same stalled page. Include the exact mounted database path in success and refusal events without changing retention or scheduling. A subprocess regression exercises the actual operator stderr with two stores both reporting 0->48, requires four uniquely attributable log lines, and rejects vacuous zero-test success. Refs #4787.
ScriptedAlchemy
left a comment
ScriptedAlchemy
left a comment
There was a problem hiding this comment.
Refs #4787 — source review of exact new head bfba4837.
Confirmed diagnostic defect: MaintenanceCoordinator::run_tick iterates up to eight sorted mounted stores and can emit canonical-GC and replay-metadata page results for different session databases in one cycle. Both operator events lacked a store identifier. Two independent databases each advancing 0→48 therefore produced indistinguishable reported frontiers, making an apparently repeated 48-row page insufficient evidence of a same-store livelock.
Published one-file fix: pass the exact mounted RegisteredGlobalDb::db_path() through both full and continuation reporting paths. Add store to successful canonical/replay page events and to their degraded/refusal events. This changes only logging and source-level test coverage; neither database/frontier, deletion eligibility, 48-row budget, writer admission nor tick scheduling changes. The independently inspected diff is exactly one file (84 additions / 4 deletions), with no changes to #4813 or the prerequisite branches.
Behavioral regression: an isolated child test calls the actual operator logger for two distinct mounted paths that both report 48 rows and exactly the same SQL frontier 0→48. It requires four emitted stderr events, exactly one uniquely attributable event of each type per store, and nonzero libtest execution; previously the store-disambiguation assertions would fail. No output-string/source-shape inventory or synthetic DB CPU benchmark is substituted for affected-host evidence.
Validation hold: the Rust test has not executed on this head: local Bazel/checkout is unavailable; no Cargo. Exact-head hosted CI is queued. Require nonzero //crates/tracedecay:unit_test, other two retention unit targets, Clippy, rustfmt and peer review before merge. This solves log attribution, not the original 90–110% CPU incident; consecutive same-store committed SQL frontiers, per-tick elapsed/work counts and affected-host CPU/restart/quiescence remain necessary. Keep #4787 open, Refs only.
Held #4776 / Kev and #4797 remain draft/nonmergeable; #4763, production, other owner branches and schedules unchanged.
Keep selected unreferenced body candidates queued if the underlying DELETE was ignored. Verify the body is absent before discharging its candidate; otherwise rollback the page instead of silently leaking unreclaimed bodies. Exercise 52 real candidates with a vetoed DELETE, rollback, recovery, 48+4 committed reclamation, and an idle final tick. Refs #4787
|
Refs #4787 — published and independently diff-reviewed source fix at Confirmed failure mode: Fix: Before deleting candidate rows, verify every selected body that still exists is actually owned by a canonical reference. A surviving unreferenced body makes the transaction return a typed integrity error, so the caller rolls back rather than claiming progress. The new Rust regression uses 52 real inserted candidates and a vetoing SQLite trigger: it requires refusal and preservation of all 52 candidate/body rows, then removes the trigger and asserts successful 48+4 deletion/discharge, zero remaining candidates, and an idle final tick. The same SQL logic passed an independent SQLite rollback/recovery probe, not the repository Rust/Bazel suite. Exact-head CI run 42763155068 is queued. Bazel is not installed in this runtime, and Cargo was not used. This closes a real silent-GC-leak hole, not the original observed 90–110% CPU root cause: affected-host consecutive same-store SQL frontiers, real tick cadence and CPU, restart/quiescence, and final |
Replace per-owner and per-body bootstrap INSERT submissions with one indexed INSERT SELECT per bounded page. Keep the selected last identity, sparse-key semantics, eligibility predicate and transactional checkpoint. Add real SQLite executor regressions for bounded write submissions, 52-row sparse frontiers, live-body preservation, eligibility at the page tail, and late statement failure with full rollback and successful retry. Preserve the concurrent vetoed-delete repair without rewriting history. Refs #4787
ScriptedAlchemy
left a comment
ScriptedAlchemy
left a comment
There was a problem hiding this comment.
Published bootstrap batching repair — Refs #4787
Pushed eb9bd17 on top of the concurrent c3f2d94. This changes the canonical-body bootstrap implementation and adds one test module; the PR now has seven retention-specific paths rather than six. No prerequisite branch was changed.
Source defect and correction
A full eligible raw-owner bootstrap page, or a canonical-body candidate-seeding page, submitted 48 separate awaited INSERTs through the SQLite executor before its checkpoint write. Replace those per-row INSERTs with one indexed INSERT OR IGNORE ... SELECT bounded by the last identity selected for the page. Selection and insertion stay inside the same writer transaction. Sparse IDs, the inline/canonical ownership predicate, the 48-row cap, frozen upper bounds, checkpoint verification and rollback remain intact. The observation-reference bootstrap is unchanged.
The independent SQLite mechanism reproduction compared identical 52-row inputs: full eligible pages used 49 → 2 execute submissions, including the checkpoint; four-row tails used 5 → 2. Sparse raw frontiers advanced 0 → 96 → 104 with identical enrolled owners. Late insertion failure rolled back the page and checkpoint, and retry recovered. Mixed ineligible tail rows preserved the page boundary. These are SQL-call counts, not measured Rust worker timings or an affected-host CPU reduction; read queries and commit calls are not included in that count. A nonempty all-ineligible raw page may now issue an empty batched INSERT.
Committed behavioral regressions
bootstrap_write_tests.rs forwards the real executor to a real SQLite transaction and counts write submissions; it does not replace the database with a mock or assert source strings.
bootstrap_pages_batch_writes_and_preserve_sparse_frontiers: 52 real canonical bodies and sparse raw owners, distinct 48+4 raw/body pages, bounded write submissions, exact durable frontiers/owners, 48+4 candidate discharge, zero deletion of live bodies, original BLOB preservation and idle completion.bootstrap_batch_refusal_rolls_back_and_recovers: inject a late SQLiteRAISE(FAIL)separately during raw-owner and body-candidate seeding; require refusal, exact checkpoint rollback, no committed partial entries, body preservation and successful 48+4 retry.bootstrap_batch_eligibility_does_not_change_the_page_boundary: external and directly stored-text owners at the end of a full page do not expand the admitted range or consume the following four owners.
Review, publication and validation limits
I inspected the detached commit diff in a separate review pass before attaching it. It contains only the module declaration and two bootstrap changes plus the new regressions. The concurrent vetoed-body-delete guard and its existing tests remain unchanged. Publication used force=false and exact expected old head c3f2d94d2bcc4b7c70e074e06e3b7b294c2452aa; the remote ref was then verified at eb9bd17.
The new Rust tests have not executed locally. No Bazel executable/complete checkout is available here; no Cargo, direct rustc or rustfmt was used. Exact-head CI 42764859366 is queued. Require nonzero execution of the new cases under bazel test //crates/tracedecay-global-db:unit_test --test_output=errors, the existing retention suites, pinned formatting/Clippy and required hosted checks. This COMMENT is not independent reviewer approval or a merge authorization.
The separate queued-writer head-of-line blocker reviewed on #4834 is not repaired by this patch. The original sustained 90–110% CPU incident still needs same-store committed frontiers, real tick/CPU and restart/quiescence measurements and acceptance on master. Keep #4787 open, Refs only. No merge, protected #4776/Kev draft change, release #4763, production/infra, credential, tag or schedule action was performed.
Group validated observation reference inserts into bounded storage-worker submissions instead of one asynchronous request per referenced hash. Preserve Rust reference parsing, identity order and transactional rollback. Prove 48+4 pages across the 128-statement boundary, durable SQL frontiers, partial-batch refusal/rollback, reference deduplication and quiescence. Refs #4787
ScriptedAlchemy
left a comment
ScriptedAlchemy
left a comment
There was a problem hiding this comment.
Refs #4787 — independent follow-up review of exact head 9b50818107afea5486193022122c3a45f9a5b40d.
The previous observation bootstrap performed one awaited INSERT OR IGNORE per distinct canonical hash in the selected 48-owner page. This commit retains the existing collect_body_refs validation/deduplication and selected-ID frontier, but uses the engine's owned WriteStatement / Transaction::execute_statements path in at most 128 pending writes per submission. The existing writer transaction owns all groups and the checkpoint; a late refusal rolls everything back. No SQL page-size, retention cadence, policy, or test assertion was relaxed.
Reviewed the two-file commit against eb9bd17c and its real production caller. The new database-backed Rust cases require 48+4 observation pages with 141 then 153 distinct indexed references, 128+13 writes on the first page across two batch submissions plus the checkpoint, exact 0→96→104 persisted observation frontiers, body preservation, idle quiescence, and rollback after a deliberately refused insert following a completed first batch, then recovery.
Independent disposable on-disk SQLite reproduction passed the injected refusal/rollback and recovery: first 48-owner page = 141 refs / 3 submissions including checkpoint, next 4-owner page = 12 additional refs / 2 submissions, no repeated page after frontier 104, all 52 canonical bodies retained across reopening. This reproduction measures submission count rather than production daemon CPU or storage-worker duration, and is not Rust/Bazel CI.
Hold merge/issue closure: exact-head Bazel CI is queued at https://github.com/ScriptedAlchemy/tracedecay/actions/runs/42768531776; no Bazel executable is available in this runtime, no Cargo was used. The original affected-host 90–110% CPU incident still requires consecutive same-store committed SQL frontiers/candidate counts, actual per-tick CPU and cadence, restart/quiescence, and verified acceptance on master. This is partial work only; no Fixes #4787 claim. Protected #4776/Kev drafts and release #4763 unchanged.
After batched bootstrap writes, verify the selected raw owner references, Rust-validated observation references, and canonical-body candidates exist inside the writer transaction. A suppressed INSERT must not commit a frontier that can drop a live body or permanently abandon an orphan. Exercise RAISE(IGNORE) for all three phases, rollback, 48+4 recovery, and live-data preservation. Refs #4787
ScriptedAlchemy
left a comment
ScriptedAlchemy
left a comment
There was a problem hiding this comment.
Refs #4787 — source-integrity review of exact published commit 6edf06957b7bff930d19e6956f6573ac90d22da7 against 9b50818107afea5486193022122c3a45f9a5b40d.
Confirmed failure class: The three canonical-body bootstrap phases previously treated successful INSERT OR IGNORE submissions as proof of derived ownership/candidate enrollment. A SQLite BEFORE INSERT ... RAISE(IGNORE) trigger can suppress one selected entry without returning an error; the old path would still commit the raw/observation/body bootstrap frontier. Missing raw/observation references could then make live bodies appear orphaned; missing candidates could strand unreferenced bodies behind the committed cursor.
Published correction: After raw-owner batching, observation WriteStatement batches, and body-candidate seeding, verify the actual selected identities in the same guarded SQLite writer transaction before save_progress. Observation verification uses the exact Rust-validated/deduplicated owner/hash pairs rather than reparsing original observation documents; legitimate already-existing derived entries remain valid. Any missing item produces a typed error and rolls back the entire page. The 48-row page cap, eligibility, and checkpoint authority remain unchanged.
New database-backed Rust regressions: Deliberately suppress one raw reference, one observation reference after a full 128-statement group, and one 48th candidate; require refused checkpoint, full rollback, recovery of distinct 48+4 pages and preservation of all 52 live body blobs. Existing duplicate, sparse-key, candidate discharge and idle tests remain intact.
Independent SQLite (not Bazel) validation: Three in-memory trigger-refusal scenarios each found the omitted entry; rollback retained no partial derived state; subsequent recovery indexed 52 entries in bounded pages. The exact SQL verification against 141 distinct observation pairs also detected one suppressed owner and passed after restoring the three references. There is no local repository checkout or Bazel executable, so no claim of Rust/Bazel execution or affected-host CPU reduction. New exact-head hosted CI is pending.
This is a partial safety/progress repair, not resolution of the original sustained 90–110% CPU incident. #4787 must stay open pending actual same-store committed frontier/tick/CPU/restart evidence and master-proven acceptance. Protected #4776/Kev drafts and release #4763 remain untouched; publication used exact-old-head 9b50818107af and force=false.
Scope — Refs #4787, partial; keep issue open
Six retention-only files above explicit CI prerequisites. The former eight-file diff included two independent test-fixture repairs. Those repairs remain byte-for-byte in the tree and in the original history; they have not been reverted or overwritten.
Review/landing order:
Do not merge this PR into the prerequisite branch. Land the prerequisites in master through their own reviews, retarget this PR to master, verify its diff and exact-head CI again, then consider the ordinary merge gate. No auto-merge is enabled. Do not undo the fixture repairs or transplant held #4776/decision-model work to reduce this stack.
Retention implementation retained
octet_lengthavoids materializing oversized text merely to discover its size.Scope reconciliation and concurrent work
Commit
af1045000305250ac1cefd94e87ea1ddeefc201cjoins only the already-present fixture prerequisites. Its first parent is the exact prior #4798 head4984a54fc4cf6cd71d2191d60a80639b1f0b183f; its second parent is #4827ee44f8c16b736c36075eaadfe4bee6551194951a. At reconciliation, the full source tree was3f493dd3a148e21f7bbfe8f9f59a14392d00dd8e, identical to the prior head. GitHub comparison confirms zero changed files versus that parent, and exactly six retention files versus the prerequisite. Publication uses a nonforce expected-old-head fast-forward, not a rebase or reset.The previous overlap warning was too broad: #4813 changes maintenance writer admission, while this PR adds fields to the two outcome log functions at separate hunks. A shared filename is not by itself a merge conflict. #4813 remains independently owned; none of its writer-admission implementation is imported or changed here. The scheduler fixture has one existing review owner, #4821, whose blob matches this tree exactly. Combined runtime behavior still needs normal integration testing when both source PRs land.
Validation and acceptance boundaries
1f0446809fpassed 11 canonical-body GC cases, 11 replay-metadata cases, and 4 registered-store retention cases. The daemon composition library built successfully. These runs exercised writer-transaction rollback after ignored/rewritten checkpoints, candidate-removal refusal, recovery across bounded pages, oversized metadata and idle quiescence.9a1c8a27d0, all 11 replay-metadata cases passed on native macOS, including the peer correction that counts admitted ineligible rows as skipped. Ondfd0c6a106, all 34 native macOS LCM retention tests passed, including all 11 replay-metadata cases. Both ignored and rewritten checkpoint writes roll back earlier changes and recover. The budget regression verifies that only the admitted prefix contributes bytes. The application consumes the budget once, before eligibility parsing. Checkpoint reviewFixes #4787claim or automatic issue closure.Protected mixed #4776 and all Kev/decision-model PRs, including #4797, remain draft and excluded from merging. Release #4763 remains read-only. No production, infrastructure, tags, credentials or task schedules are changed.
Review order remains #4834 → #4827 → this PR. The #4834 Linux fleet passed all 183 targets on
7264f0d6c2, including both affected binaries uncached. Targeted native Windows passed both selected suites. Its Clippy job encountered an upstream HTTP 504 downloading the pinned build tool. The prior Linux run covered9a1c8a27d0before the prerequisite updated. Merge still requires independent review, completed CI and revalidation against master.The CLI fixtures from #4834 are now in the #4827 prerequisite. At
b3f839a836, GitHub still shows exactly the six retention files and the feature patch ID matches the prior head.The previous verification run checks
189b07186eagainst #4827 at4ecf593ba3. This head includes the Linux launcher download prerequisite. The 6-file feature diff has the same paths and stable patch ID after updating the prerequisite. Atddabf4a972, the initial and continuation passes both attach the owning session database path to canonical-body and replay-metadata progress. The native macOS daemon regression captured four actual operator log lines for two different stores sharing the same frontier values and verified exactly one event of each type per store. The Bazel formatter and four additional frontier cases passed. Current hosted workflow