Skip to content

fix(storage): preserve idle reclaim and LCM retry backoff - #4851

Open
ScriptedAlchemy wants to merge 16 commits into
masterfrom
fix/4481-retry-storage-gated-successor
Open

ScriptedAlchemy wants to merge 16 commits into
masterfrom
fix/4481-retry-storage-gated-successor

Conversation

@ScriptedAlchemy

@ScriptedAlchemy ScriptedAlchemy commented Nov 9, 2026 •

Copy link
Copy Markdown
Owner

Scope

Refs #4481 — partial repair. HOLD MERGE until independent review, exact-head Bazel CI and prerequisites are satisfied. Do not close #4481 without master-proven native macOS daemon acceptance.

This is the clean, non-force successor to #4808 after its November 9 repository-gates failure found a 74-character final commit header, over the 72-character enforced limit. The authored-header failure also caused the Feature gates aggregate to fail. Since the owner requires no force pushes or rewritten concurrent branches, this replacement is one properly titled commit on current master, not a rewrite of the old branch.

The replacement carries all 13 exact file blobs from #4808 head 1301327af043, based on master d6acbc49f791. A compare of both diffs confirms identical changed-file paths and additions/deletions, with no overlap against the eight commits added to master since #4808's prior base. All old branches and other owners' work are preserved.

Production fix and nonvacuous regressions

  • Preserve durable LCM invalidation retry deadlines and terminal states when the same dirty raw revision remains. A truly new raw generation wakes processing and rejects stale-worker settlement.
  • Stop foreground daemon schema-attachment vacuum; short-lived registered-store owners instead reclaim at most 4,096 SQLite pages and do not admit a failed writer.
  • Defer only optional SQLite contention/maintenance saturation or SQLITE_FULL, while propagating writer shutdown, revoked authority, read-only and other storage failures.
  • Existing behavioral regressions exercise real dirty-revision final-delete failure, persistent backoff across reopening, fresh revision wakeup, actual free-page reduction, real SQLite disk quota errors, rejected writers and authority refusal.

Verification and remaining acceptance

  • This is a byte-for-byte source and tests carry-over, not a fresh claim of test pass. Earlier exact-head native macOS Bazel and hosted Linux passing targets are documented on fix(lcm): preserve retry backoff across dirty invalidation #4808.
  • Exact-head Bazel CI on this new commit is required. Local Bazel is not available; no Cargo was run.
  • Native macOS idle daemon CPU, repeated retry convergence, and 24-hour physical store-size growth still require measured end-to-end acceptance on master. This PR does not claim the whole issue is fixed.
  • Separate relation-recovery scaling work remains in perf(sessions): avoid idle scans of deferred relation receipts #4810, including fragmented-deadline follow-up.

No changes to #4776 or Kev/decision-model drafts; release #4763 stays read-only.


Devin Review

Refs #4481

Rebase the accepted 13-file issue-specific patch intact onto current master
without rewriting any existing owner branch. Persist retry and terminal
settlement across dirty LCM invalidation, bound store page reclamation, and
preserve fatal writer/authority failures while deferring optional SQLite
BUSY, LOCKED, or FULL compaction.

This clean successor replaces the blocked PR #4808 branch, whose last commit
header exceeded the enforced 72-character limit. Carry the exact same 13
file blobs and preserve unrelated master changes.
@changeset-bot

changeset-bot Bot commented Nov 9, 2026 •

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 8b5ee8f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

devin-ai-integration[bot]

This comment was marked as resolved.

Refs #4481. Propagate an actual SQLite rollback failure instead of masking
it with an authority denial; preserve the authority result on success.
Exercise a real SQLite authorization refusal during rollback and verify
cleanup preserves live rows.

Copy link
Copy Markdown
Owner Author

Source and behavioral regression — Refs #4481

Published b1c09c178152 on this issue-specific PR by an exact-old-head (5a9e081ae6b6), non-force fast-forward.

Independent review of the latest diff found that a revoked write authority during optional SQLite vacuum hid a real transaction.rollback() failure. The writer now returns IncrementalVacuumFailed with the SQLite rollback cause when rollback fails, and retains AuthorityDenied(BeforeCommit) only after a successful rollback. The prior typed disk-full/busy deferrals are unchanged.

The new incremental_vacuum_preserves_rollback_failure_after_revocation regression uses SQLite's real transaction authorizer to deny ROLLBACK while the actual vacuum runs and write authority is revoked; it asserts the caller receives the rollback failure rather than a false authority-only verdict. It then removes the test denial, rolls back the live transaction and checks free-page counts are unchanged. The existing rollback-success regression remains.

Exact-head Bazel workflow is queued; not yet a passing test claim. No local Bazel is installed; Cargo was not invoked. Leave review/merge holds until CI and end-to-end native macOS daemon measurements pass. This is a partial repair, not Fixes #4481.

Copy link
Copy Markdown
Owner Author

Exact-head CI repair — Refs #4481

The previous PR #4851 Clippy job failed on a real source/test compile error, not a transient infrastructure issue:

error[E0425]: cannot find function `incremental_vacuum_error` in this scope
--> crates/tracedecay-rusqlite-runtime/src/writer/worker/mod.rs:2086:13

Root cause: the new full_sqlite_disk_quota_defers_only_optional_vacuum regression calls the module-private vacuum error classifier from the nested auxiliary_scheduling_tests module but did not import it. This also prevents the behavioral tests from reaching execution.

Published d6f2976f2fdfe87c2ec6d3225bf6f14c5dddf017: add incremental_vacuum_error to the existing use super::{...} in that test module. The exact-parent comparison confirms only this file changed (+2/-1); no production behavior or assertion was altered. Pushed with expected_sha=8ea62537017205b56d4abee4e4033cd2a5ac122f, force=false.

New exact-head Bazel CI is pending; do not claim the regression passes until that head is green. Previously completed #4810 exact-head CI is green, but that is a separate branch and does not establish #4851 or #4481's daemon acceptance.

HOLD MERGE pending exact-head verification, independent review and end-to-end native macOS daemon acceptance. #4481 remains open: idle CPU, retry convergence, highly fragmented recovery and 24-hour store-growth acceptance have not been proven on master. No Cargo, CI bypass, peer-owned branch updates, protected draft changes or release actions.

Copy link
Copy Markdown
Owner Author

Exact-head Linux Bazel fleet blocker — Refs #4481 (2026-11-09)

The current #4851 head d6f2976f2fdfe87c2ec6d3225bf6f14c5dddf017 passed Repository gates, Clippy, Dashboard, Feature gates, and the relevant standalone session/runtime targets, but its Linux fleet job 129423090717 finished 180/183 Bazel targets passing, three not passing. I read the actual Bazel test-log artifact, not just the aggregate status:

  1. //crates/tracedecay-global-db:unit_test TIMEOUT at 300.1 seconds. Its captured stream lists successful cases through tests::source_rewrite::rewrite_completion_retires_projected_records_and_rolls_back_a_failed_attempt and ends with Test timed out. It does not identify the blocked test/thread; do not label this a source failure without further evidence.
  2. //crates/tracedecay:hooks_lsp_suite 28/29 passed. a_failed_receipt_drain_is_reported_on_status_and_clears_when_the_spool_drains failed its existing assertion replay must mount before the orchestrator. The daemon child log printed daemon_listening, daemon_http_application_listening, and daemon_ready bootstrap_elapsed_ms=540, but no replay-mount event. The file is not changed in fix(storage): preserve idle reclaim and LCM retry backoff #4851.
  3. //crates/tracedecay:mcp_suite 673/675 passed. Two context_lexical_anchor_eval_test cases failed: one tool_dispatch_deadline_exceeded while tracedecay_context was settling, and one production-composition code-index publication deadline after 20,001 ms (gate capacity=4, admitted=3, waiting=0). These test files are also outside fix(storage): preserve idle reclaim and LCM retry backoff #4851's 13-file patch.

This is an observed exact-head CI blocker, not evidence yet that the 13-file reclaim/backoff patch caused these three failures, nor evidence they are pre-existing flakes. The native macOS end-to-end idle-CPU, retry-convergence, and 24-hour store-growth requirements remain independently unproven.

Parent workflow 42692895291 was still running Windows when checked, so the correct next verification is to rerun the failed Linux fleet job only after the parent completes. If the same failures reproduce, inspect their real runtime phase timings / readiness ownership and coordinate with the relevant fixture owners rather than raising timeouts or weakening assertions. Keep the #4851 HOLD MERGE and #4481 open. No Cargo, production changes, peer-branch mutations, draft changes, or CI bypass.

Refs #4481

Never reuse a SQLite writer connection if optional page reclamation
leaves an open transaction. Drop the connection and reject queued work
before a later write can accidentally join that transaction. Settle
recoverable vacuum errors normally when SQLite is back in autocommit.

Exercise a real SQLite rollback refusal plus successful rollback,
contention and read-only paths to distinguish fatal and safe outcomes.
Refs #4481

Keep the fail-closed writer fix within the Rust formatting line budget.

Copy link
Copy Markdown
Owner Author

Fail-closed SQLite writer repair — Refs #4481 (2026-11-09)

Published 41bd7ab65311 and its formatting-only successor be64067457c1 to this issue-specific PR through separate exact-expected-old-head, non-force fast-forwards.

Confirmed defect: The preceding rollback-error regression deliberately made SQLite refuse ROLLBACK, then proved connection.is_autocommit() == false. The production writer loop nevertheless returned to normal auxiliary/write scheduling after reporting IncrementalVacuumFailed. That could reuse a connection with an open transaction, unintentionally mixing a later accepted write into the failed vacuum transaction or retaining its SQLite lock.

Repair: Every actual vacuum outcome now checks whether SQLite returned to autocommit. If not, the production worker marks itself Faulted, rejects already queued commands and retires its connection immediately instead of running subsequent writes/checkpoints. The error path drops the transaction before sampling state, allowing automatic rollback on ordinary errors. Normal SQLite contention, read-only refusal, successful revocation rollback and successful vacuum keep the writer available.

Behavioral checks: Extended the existing real SQLite authorizer rollback-refusal regression to assert that the writer must retire, and the actual contention/read-only and successful-rollback tests to assert the opposite. These exercise SQLite connection state rather than synthetic error-string classification. An independent source/diff review found the retirement restricted to the failed-transaction boundary; the prior error propagation is unchanged.

Exact-head Bazel workflow on be640674 is queued, not passing evidence yet. The affected environment has no Bazel or Rust toolchain, so no local Rust build/test was claimed and Cargo was not invoked.

HOLD MERGE. The unrelated Linux fleet failures on previous head d6f2976 and the native macOS idle-CPU, retry-convergence and 24-hour physical-store-size acceptance remain unresolved. #4481 stays open; this is Refs, not Fixes. No protected draft, release, other-owner branch or CI gate was altered.

devin-ai-integration[bot]

This comment was marked as resolved.

Refs #4481

Close buffered writer ingress before retirement, drain already-admitted
commands, and reconcile typed fault replies and queue telemetry. Real
channel regression covers buffered and parked writes plus late admission.

Copy link
Copy Markdown
Owner Author

Source review and targeted repair — Refs #4481

Published b09a1e7263c2 on #4851 via an exact-old-head (1c4193af7ff5), non-force fast-forward. The two changed files are limited to the SQLite writer worker and its rejection logic; no other owner branch was touched.

Root cause: the preceding fatal-vacuum retirement closed the SQLite connection but skipped accepted requests still in its bounded ingress receivers. Those callers observed a dropped reply instead of the typed Unavailable { reason: Faulted } outcome, while admitted queue metrics stayed inflated. The fix closes receiver channels first to stop racing sends, drains accepted writes with explicit fault settlement, and rejects buffered exact-SQL/vacuum commands before retiring. I checked the pinned asupersync 0.6.0 MPSC implementation: close() disallows subsequent sends while try_recv() can still drain buffered messages.

Behavioral regression: a real MPSC channel holds an unread accepted write while another is already parked in the fair queue. Both receive typed fault replies; queued operations/bytes return to zero, completion counts match admitted work, and a late send is refused. This test is not a source-shape assertion.

Exact-head Bazel workflow #42729796684 has started in queued state. No passing result is claimed. Bazel is unavailable in this execution environment; Cargo was not run. Independent native macOS idle-CPU, retry-convergence, fragmented receipt scaling and 24-hour bounded-store acceptance are not proven on master (aa03f27e). HOLD MERGE; keep #4481 open.

Use explicit rollback for vacuum execution and commit failures so a failed
rollback cannot masquerade as a retryable busy or disk-full condition.
Preserve automatic SQLite rollback on disk-full and retire writers when a
transaction cannot be closed. Add real-SQLite contention, refusal, recovery,
and data-retention regressions.

Refs #4481

Copy link
Copy Markdown
Owner Author

Explicit SQLite cleanup on vacuum failure — Refs #4481

Published 3d62dded1695 to this issue-specific PR using an exact-old-head (b09a1e7263c2), non-force fast-forward. Only writer/worker/mod.rs changed; no other owner branch was modified.

Root cause: optional incremental_vacuum previously relied on Transaction::drop after execution failure and consumed Transaction::commit() after commit failure. If rollback was refused while the transaction remained live, that cleanup error was discarded and a transient SQLITE_BUSY/disk-full classification could be returned even as the worker retired. The existing fail-closed worker retirement remains in place.

The writer now explicitly rolls back on both error paths, preserves rollback failures as fatal diagnostics, distinguishes SQLite's automatic rollback on SQLITE_FULL, and closes successful explicit commits without a redundant rollback. Tests exercise real rollback-journal reader contention, authorized/denied ROLLBACK, retry with retained data after recovery, PRAGMA refusal, and automatic disk-full rollback. They assert whether the worker must retire.

Independent verification: four independent real-SQLite reproduction cases passed on SQLite 3.46.1. These are not Rust/Bazel evidence. Local Bazel and the affected native macOS daemon are unavailable. New exact-head Bazel CI #42737462258 is queued; no passing result is claimed. Hold merge and keep #4481 open until CI and measured native macOS idle CPU, retry convergence, and 24-hour physical-store growth are proven on master. No Cargo was used.

ScriptedAlchemy and others added 7 commits November 9, 2026 18:20
Apply the four Bazel rustfmt_check_2024 hunks from CI run 42737462258.
Retain test logic, admissions, rollback observations and assertions.

Refs #4481
An execution-batch fault formerly dropped later selected batches, losing
caller replies and leaving queue telemetry inflated. Fail those accepted
requests with typed Faulted outcomes before retiring the worker.

Refs #4481
Align the existing behavioral regression with rustfmt so the Bazel
repository gate can complete and exercise the source fix.

Refs #4481
Refs #4481

Faulted actors must not retain SQLite connections until registered
senders disconnect. Close ingress and drain accepted work on retirement.
Exercise the live writer and verify typed replies and zero queued bytes.
Refs #4481

The vacuum worker kept a named begin-transaction Result live through its
failure arm, so inspecting the connection there conflicted with the
mutable borrow (E0502). Reduce the begin result to an owned error in a
let-else scrutinee, preserving settle-then-report ordering. The fault
regression also needs the QueueItem trait in scope and a pinned-rustfmt
wrap. No retry, vacuum, authority, or assertion behavior changes.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

daemon: high idle CPU from session refresh scheduler, store over size bound, repeated corruption retries

1 participant