Skip to content

fix(lcm): preserve retry backoff across dirty invalidation - #4808

Closed
ScriptedAlchemy wants to merge 13 commits into
masterfrom
fix/4481-lcm-dirty-failure-backoff
Closed

ScriptedAlchemy wants to merge 13 commits into
masterfrom
fix/4481-lcm-dirty-failure-backoff

Conversation

@ScriptedAlchemy

@ScriptedAlchemy ScriptedAlchemy commented Nov 8, 2026 •

Copy link
Copy Markdown
Owner

Scope

Refs #4481. A retained-summary invalidation may fail while the raw revision remains dirty. Convergence now persists the retry deadline or terminal state for that revision. A genuinely new revision increments its generation, wakes the session and prevents a stale worker from settling newer work. A successful page still leaves outstanding dirty input pending.

Daemon attachment leaves ordinary free-page reclamation to maintenance. Short-lived registered-store owners lack that maintenance turn and reclaim up to 4,096 pages after successful admission. A full optional maintenance queue or a SQLite busy or locked result defers reclamation. A rejected writer, lost authority, read-only database, or other SQLite failure prevents a serving lease. The workflow artifact migration retains its explicit post-upgrade reclamation. The branch also includes the scheduler backend-identity and scripted CLI daemon-binding test fixtures.

Verification

On the source now published at 18c4422c2d, native macOS Bazel passed all 428 rusqlite-runtime tests, all 499 global database tests, and 187 LCM tests (one ignored). Four existing summary-convergence cases and the added end-to-end failure case passed. The six vacuum cases also passed in isolation. The previous head’s hosted formatting failure in repository/attachment.rs has been corrected.

A production convergence-page regression injects a SQLite failure at the final dirty-revision deletion. It checks that the transaction leaves the same revision intact, that the queue delays the next attempt, and that a fresh revision wakes the session and clears the dirty row after the database recovers. The SQLite writer tests exercise real lock contention, a read-only connection, worker rejection, and revoked authority. Writer admission review

The hosted Repository gates and Clippy checks passed. The Linux fleet job reported two five-second settlement deadlines after the durable Prepared phase. The same two cases failed at the same assertions on a different branch, and both passed in the #4791 Linux suite. The automations themselves use isolated stores, and the exact publication code is identical on those heads.

On the unchanged #4808 head, all nine retained-settlement cases passed three times on native macOS. With temporary timing probes, all 437 automation tests passed three more times. The two affected tests each published their durable ledger entry in 69–95 ms across those runs. The artifact provides no per-phase Linux timings. A local experiment removing one directory sync did not reduce the observed upper bound. The experiment was discarded. The subsequent exact-head hosted rerun passed, as recorded below.

Remaining acceptance

The affected macOS host still needs idle CPU, retry convergence, and 24-hour growth measurements on master. #4481 remains open pending that evidence and independent review.

The hosted Linux fleet rerun passed all 183 Bazel test targets on the unchanged head 18c4422c2d. The automation runtime unit target executed in 71.5 seconds, including both previously timed-out settlement cases. The preceding rerun stopped before tests when GitHub returned HTTP 504 for the pinned launcher download. The original #4481 master acceptance remains open.

Mixed fixture ownership review

At 4cee32eb43, the PR incorporates the accepted master snapshot 02e3c8b28b. The shared scheduler fixture and scripted CLI transport fixture both match master. The canonical CLI fixture remains owned by #4834. The proposed diff now consists of 13 LCM/storage production and test paths, all byte-identical to their versions on the previously green 18c4422c2d head.

The session-runtime LCM regression belongs here. It fails the final dirty-row deletion during production convergence, observes that the same input stays delayed, and verifies that a changed revision is admitted and clears the durable dirty row after recovery. The prior mixed-fixture review is addressed. Current-head CI is pending.

On the isolated head 4cee32eb43, all nine focused native macOS Bazel cases passed: the production dirty-revision transaction regression, the LCM restart/backoff regression, short-lived registered-store physical page reclamation, and six vacuum contention, refusal, or page-budget cases. Hosted repository gates also passed. Keep #4481 open for its separate master acceptance.

Keep retryable and terminal outcomes durable when an earlier raw revision
still has invalidation work; only new revisions can rearm those failures.
Cover retry deadlines across restart, terminal parking, recovery on new
raw input, and stale-generation refusal.

Refs #4481
@changeset-bot

changeset-bot Bot commented Nov 8, 2026 •

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 1301327

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

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

@devin-ai-integration devin-ai-integration Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

Newer findings are available below. Devin Review posted a newer report on this PR, in addition to the findings presented here.

🔍 Devin Review: 1 flag

Not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)

Devin Review

devin-ai-integration[bot]

This comment was marked as resolved.

Copy link
Copy Markdown
Owner Author

Independent exact-head review blocker for #4481 (head 6eb369f3ac07f61bd73452718270fc3665fb7ef7): this PR is no longer the described two-file LCM retry fix. Commit c3c16d4 adds automation-runner fixtures (Refs #4532, separately owned); commit 6eb369f also changes registered-store schema admission/reclamation and tests. These changes need their own owner/review boundary before a #4481 LCM merge. In particular, deleting run_incremental_vacuum(4096) from ensure_attached_registered_schema removes the only general free-page reclamation on the short-lived RegisteredGlobalDbOwner::admit_and_attach path: unlike admit_and_attach_for_daemon, that path has no periodic compact_registered_store; its special-case reclaim_freed_artifact_pages runs only during the published workflow upgrade. A short-lived writable store with existing freed pages and no further writes can remain physically oversized across repeated opens. This is a source-confirmed missing execution path, not yet a measured end-to-end regression. Please keep foreground daemon admission noncompacting, but establish an appropriate bounded short-lived reclamation route with a nonvacuous open/reopen + physical-page-count regression before landing the storage change, or split the storage change into its separately reviewed owner PR. Do not relax tests/CI or overwrite concurrent branches. The new CI run 42528224780 is still queued on this head; earlier LCM-target passes do not validate this 6-file tip. Refs #4481; issue stays open for native macOS idle CPU and 24-hour store-growth acceptance.

Restore bounded incremental reclamation only after short-lived
schema convergence. The daemon admission path stays noncompacting;
refused stores retain their byte-preserving refusal. Cover physical
page-count reduction after an actual writable reopen.

Refs #4481
devin-ai-integration[bot]

This comment was marked as resolved.

Copy link
Copy Markdown
Owner Author

Follow-up to the short-lived-store reclamation review finding: published cc32c50 (Refs #4481). The admitted short-lived writable owner now runs one bounded 4,096-page incremental vacuum after schema convergence; daemon admission continues without foreground reclamation, and typed reset/refusal remains byte-preserving. The new real registered-store regression creates ~8 MiB of freed pages, reopens through the short-lived admission path and requires both the physical page count and freelist count to fall. Independent Git diff review found only the intended two paths in this commit. This is a source fix, not a claimed Bazel pass or a 24-hour store-size result: exact-head CI is queued, and native macOS daemon measurements remain outstanding. The mixed non-#4481 fixture changes already in this PR still need ownership review before merge.

A short-lived registered store has no daemon compactor, but a failed
incremental vacuum after schema convergence is optional maintenance,
not a reason to reject a valid database owner. Report the failure and
allow a later open to retry page reclamation without masking required
schema-convergence errors.

Refs #4481
devin-ai-integration[bot]

This comment was marked as resolved.

Only SQLite vacuum contention may defer short-lived admission.
Thread typed compaction deferral through the runtime and retain fatal
writer, reply, and authority failures; cover classification in regression.

Refs #4481

Copy link
Copy Markdown
Owner Author

Published b63afbe on this exact expected-old-head, non-force fast-forward (Refs #4481).

Independent root-cause review confirmed the writer-admission finding: the prior admit_and_attach handler suppressed all run_incremental_vacuum errors. The physical writer reported closed/unavailable and revoked write authority through the same generic error channel as ordinary SQLite vacuum failure. An apparently serving store could therefore be returned with no usable write lane.

The new patch distinguishes deferrable SQLite-vacuum failure / busy maintenance queue from fatal writer shutdown, failed replies, and revoked write authority at the writer, repository, and store-runtime boundaries. Short-lived admission defers only typed maintenance contention; writer/authority failures now propagate. The regular required-compaction entry point still propagates every failure. A focused error-classification regression checks fatal writer and authority cases versus deferrable vacuum cases. The existing real-registered-store regression continues to check physical page/freelist reduction after a short-lived reopen.

Verification limit: the local execution environment has no Bazel or native macOS daemon. These Rust tests have not been executed locally; exact-head CI 42544645963 is pending. A true end-to-end writer-revocation admission regression remains additional desirable coverage; the classification test is not a substitute for the daemon CPU or 24-hour bounded-growth acceptance. Master has independently advanced to 83694e0 (merged fixture #4821), but this commit does not alter peer-owned source paths. Do not merge on this report alone; #4481 remains open.

Copy link
Copy Markdown
Owner Author

Refs #4481 — new exact-head CI blocker, not a diagnosed #4481 source regression (PR head 18c4422c2d8d03f5ad9a1504b1a745ab35c8e9dc).

CI run 42549652638, Linux fleet job 128953370614 finished 182/183 Bazel targets passing, including //crates/tracedecay-global-db:unit_test, //crates/tracedecay-lcm:unit_test, //crates/tracedecay-rusqlite-runtime:unit_test, and //crates/tracedecay-session-runtime:unit_test. The sole red target is //crates/tracedecay-automation-runtime:unit_test: 435 passed and two timed out:

  • automation::effect_runtime::settlement::tests::retained_user_job_rebinds_and_recovery_retires_only_terminal_corrupt_spool — published phase: Elapsed(()) at settlement/tests.rs:2906.
  • automation::effect_runtime::settlement::tests::retained_pair_attempts_second_leg_and_keeps_both_guards_until_both_finish — second pair owner must finish: Elapsed(()) at settlement/tests.rs:2542.

The prior master fleet run at 91c72aba reports that automation-runtime target passed (cached); its three red automation-runner fixture assertions are different tests. This PR does not change the failing tracedecay-automation-runtime settlement implementation/tests, so the cause could be unrelated scheduling/host contention or a transitive interaction. Do not claim these new failures are pre-existing or caused by #4808 without isolation. The concurrent automation owner should investigate separately; do not mix speculative fixes into this #4481 patch. Current CI is still running Windows/other jobs; #4810 is queued on its unchanged issue-specific head.

Keep merge held. There is still no verified native macOS idle-CPU or 24-hour bounded-store daemon acceptance on master. No merge/closure or CI-rule relaxation is warranted.

Copy link
Copy Markdown
Owner Author

Refs #4481 — new exact-head CI evidence on 18c4422c2d8d03f5ad9a1504b1a745ab35c8e9dc: the follow-up Linux fleet job 128999582289 did not reach the prior two automation settlement assertions. It failed during Bazel external dependency loading at 2026-11-09 05:48–05:50 UTC: the pinned hermeticbuild/hermetic-launcher finalize-stub-x86_64-linux GitHub release download returned HTTP 504 Gateway Time-out. This is the same external dependency outage independently affecting #4810's Clippy/Dashboard jobs; it is not evidence of a fresh Rust failure, nor evidence that the previous settlement timeouts are fixed.

The parent run still has Linux and Windows work queued/in progress, so the appropriate next action is targeted retry of the failed job after the run completes, keeping the pinned dependency, CI policy, and tests unchanged. No source change, CI bypass, merge, or #4481 closure is justified by this attempt; native macOS daemon idle CPU, retry convergence, and 24-hour store bounds remain to be proven on master.

@ScriptedAlchemy
ScriptedAlchemy enabled auto-merge (squash) November 9, 2026 08:05
…aults

Refs #4481

Classify SQLite FULL on optional incremental vacuum as a maintenance
failure, not lost writer authority. Preserve fatal read-only, closed
writer and revoked-authority refusal. Exercise an actual SQLite FULL
quota error and the repository dispatch boundary.

Copy link
Copy Markdown
Owner Author

Refs #4481 — published 1301327 as a non-force exact-old-head fast-forward on this issue-specific PR.

Additional root cause: the earlier classification fix only deferred SQLite BUSY / LOCKED. An optional 4,096-page vacuum can itself return SQLITE_FULL under disk/WAL pressure after schema admission, even though a smaller ordinary operation might still succeed. That result went through IncrementalVacuumFailed → RepositoryDispatchError::Writer → reclaim_short_lived_pages(...)?, unnecessarily refusing the admitted owner. This was still the disk-pressure edge case discussed in the prior review.

The focused three-file patch classifies only SQLITE_FULL from the vacuum as typed deferred maintenance, alongside contention. Closed writer, revoked write authority, read-only SQLite, other SQLite failures, and mandatory-compaction failures still propagate. One regression generates an actual SQLite SQLITE_FULL using a restricted in-memory SQLite page quota and checks classification; another asserts the repository dispatch boundary defers that result and preserves read-only/writer/authority refusal. The existing real registered-store reopen-and-reclaim test remains unchanged.

Independently reviewed the resulting three-file commit diff and parent (8b432925). Exact-head Bazel CI is pending; local Bazel/native macOS daemon are unavailable, so no Rust pass or end-to-end idle CPU/store-growth result is claimed. Issue stays open and no peer branches or protected drafts were touched.

ScriptedAlchemy added a commit that referenced this pull request Nov 9, 2026
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.

Copy link
Copy Markdown
Owner Author

Refs #4481 — superseded by #4851, not merged.

Exact-head Repository gates rejected this head because its last commit header is 74 characters versus the enforced 72-character maximum; Feature gates then correctly failed. Amending this head would violate the non-force/exact-head branch-preservation requirement.

#4851 preserves all 13 exact file blobs and the exact same per-file diff stats from this PR, on current master in one valid 57-character-header commit; there was no file overlap with master's intervening eight commits. Successor CI is queued, not passed. The previous branch remains intact for review/history, but this duplicate PR is closed to avoid wasting CI and accidentally landing the blocked commit history.

Do not close #4481: native macOS idle CPU, repeated retry convergence and 24-hour bounded-store acceptance remain unproven on master.

auto-merge was automatically disabled November 9, 2026 18:34

Pull request was closed

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.

1 participant