Repository navigation
fix(retention): verify SQL page progress and expose GC frontiers (#4787) - #4798
ScriptedAlchemy wants to merge 17 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.
| // An advancing local key is not proof that its checkpoint was stored. | ||
| // Verify inside the same writer so a refused or rewritten checkpoint | ||
| // rolls back this page instead of committing a false progress report. | ||
| let persisted = frontier(conn).await?; |
There was a problem hiding this comment.
Reviewed the checkpoint fix here: read-back must remain inside this writer transaction. A monotone local key (or even a successful INSERT row count) is not sufficient when a checkpoint write is ignored or subsequently rewritten. This comparison now routes failure through the existing transaction rollback rather than returning false progress. The new fault-injection regressions exercise both cases, actual pre-error compaction, rollback, and recovery; their Rust/Bazel execution is still pending.
| 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
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
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. |
Refs #4787 Validate the selected candidate hashes are absent inside the same writer transaction after DELETE. An AFTER DELETE trigger can reinsert all 48 rows while SQLite reports 48 affected deletes, producing false progress and repeat page work. The readback fails closed so the caller rolls back. Add a 52-body database-backed regression proving reinserted pages are refused without losing candidates, then recover as distinct 48+4 pages.
Refs #4787 Remove an extra blank line introduced with the candidate-count helper.
|
Refs #4787 — published a focused false-progress guard on issue-specific PR #4798: Newly proven failure class: the existing GC candidate discharge check used Nonvacuous regression: 52 distinct referenced canonical bodies reach candidate collection; a fixture trigger re-enqueues every deleted candidate. The 48-row page must fail with the new typed DB integrity diagnostic and preserve all 52 candidates across rollback. After removing the fixture trigger, two committed pages must discharge 48 then 4, delete no referenced bodies, and become idle. A disposable SQLite analog independently verified the old false-positive (DELETE rowcount=48, all 52 candidates still present) and post-trigger recovery (48 deleted, 4 remain); that is not Bazel execution. New exact-head Bazel CI is queued, not a pass. Local Bazel is unavailable; no Cargo was used. Previous head |
|
Refs #4787 — verified the latest issue-specific PR head Reviewed current root |
Repair three hosted automation_runner_test failures by supplying the configured executable identity required by the production prefilter. Use the same identity for the prefilter and retained runner, preserving manual-schedule, exact-ledger, live-lock and lock-race assertions. Add a production-prefilter regression for unconfigured refusal, deduplication and same-occurrence recovery after binding, with zero backend calls and no effectful terminal. Do not relax backend admission, change timeouts, or alter the graph publication changes. Refs #4532
|
Refs #4787 — new issue-PR scope/validation blocker on current head The new single-parent commit after The preceding exact-head CI run 42522861982 was cancelled when the head moved (not a passing Rust/Bazel run). New exact-head CI 42527459668 is queued; no relevant tests on |
Scope — Refs #4787 (partial, not closure)
Publish the retention-only five-file transactional progress diagnostic and fail-closed guard against
masterso it can be reviewed and tested independently of the held, mixed draft #4776. No Kev/decision-model changes are included or moved; do not merge or auto-merge #4776.has_more.Verification and limitation
master; only the above retention implementation, logs, and regressions changed.Fixes #4787claim. Leave the issue open, and do not auto-merge pending validation or qualification.