fix(graph-db): seat live activation after Plan 39 - #529
Conversation
|
d836ee8 to
1c08dba
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c08dba594
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| check_request(registration.cancellation.as_ref(), registration.deadline)?; | ||
| let (next, _) = self | ||
| .inner | ||
| .changed | ||
| .wait_timeout(state, OPEN_WAIT_POLL) |
There was a problem hiding this comment.
Recheck cancellation after the close wait
If cancellation or the deadline occurs after the check on line 703 but before this condition-variable wait returns, the next iteration can observe Ready, Faulted, or an absent entry and return a lease or unrelated settlement error without checking the request again. Thus a request waiting on the newly supported Closing state can succeed after cancellation; recheck immediately after waking or at the top of every loop iteration. The analogous owner-attachment wait at lines 821-829 has the same race.
AGENTS.md reference: AGENTS.md:L95-L97
Useful? React with 👍 / 👎.
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
656c873 to
6d5d530
Compare
0c8373f
into
codex/tracedecay-total-redesign-plan
Summary
Live activation never seated after Plan 39's first-publish seat (#525): reconcile/retained-seat looped on
code graph database conflict, and sealed generations stayedexact_scope_generation_not_ready. This PR fixes the two remainingcrates/tracedecay-graph-dbcauses, restacked onto4756385641041fc4c4e954da5d63898a4dd45abd(the current 421 tip, which already carries the 4 GiB write-ceiling scale andGraphBudgetKind::from_namein2af617019).Fixes
Superseded-head CAS now seats the producer's own publication. A publish retried after the durable verified-head CAS already advanced to that exact publication (crash after the linearization point) previously filed its own publication as historical evidence (
remember_verified_generation) and left the projection with no installed verified head — reads answered "graph projection is not recovered into an installed verified head" forever. The retry now installs the lease as the verified head (install_verified_generation), exactly like the CAS-success path, after the same apply → reopen → recovered-digest verification. A publication superseded by the producer's own later journaled publication still seats as historical evidence without displacing the advanced head.In-flight close no longer conflicts the same owner's next attach/publish.
resolveandresolve_owner_attachmentanswered a hardConflictwhenever the registry entry wasClosing, even though aClosingentry always settles within its owning call (removed, restored ready, or retained as a terminal fault). Both now wait for that settlement like an in-flightOpening— bounded by the caller's own cancellation/deadline — and attach remounts once the close settles.Retiringkeeps its hardConflict: an armed pre-close retirement reservation's all-or-none denial is deliberate and may be held across calls.Repaired the base's write-ceiling bound contract.
limits::tests::canonical_write_budget_admits_sealed_generation_bytes(landed in2af617019) asserted the ceiling equals2 × 1 GiBinstead of the intended2 × 2 GiBsealed admission bound, so it failed against the very 4 GiB ceiling it pins. The contract now assertsMAX_GRAPH_BATCH_CANONICAL_BYTES == 2 × sealed boundexactly.Recovered-digest seat proof (isolated tempfile fixtures only; no live
~/.tracedecay)verified_generation_contract::recovered_digest_seats_under_closing_and_superseded_head_conflictruns the complete Plan 39/25 conflict in one journey: with the runtime parked mid-Closing, a seat retry attaches (waits out the close, remounts) and publishes a journaled generation whose expected prior head was superseded by its own durable publication. The test asserts the recovered generation digest recomputed from actual rows equals the journaledexpected_recovered_digest, the verified head is installed and serves the generation's actual row bytes, and the superseded predecessor still seats historically with its own matching digest without displacing the head. Falsified against the base: with this branch's tests but the base's productionregistry.rs/registry/publication.rs, the journey fails; with the fixes it passes.Supporting tests:
verified_generation_contract::first_publish_retry_seats_its_own_publication_already_at_the_head— red before the fix (publish "succeeded" but no head was installed; reads stayed unavailable).verified_generation_contract::superseded_journaled_publication_seats_historically_on_retry.registry_contract::same_owner_attach_waits_for_in_flight_close_instead_of_conflicting— bounded waiters get their own typedDeadlineExceeded(wasConflict); the armed-retirement denial contract is re-asserted by the existing reservation test.registry::tests::closing_runtime_defers_resolution_until_the_close_settles— updated from the superseded deny-without-waiting contract.Full
cargo test -p tracedecay-graph-db --all-features --no-fail-fast: 191 passed. The single failure,registry_contract::canonical_runtime_resolver_locator_opens_through_graph_registry, is environmental (FilesystemLocalityUnverified { filesystem_type: "overlay" }on the dev VM) and fails identically on the pristine base without these commits. rustfmt and clippy clean on the crate.Scope:
crates/tracedecay-graph-dbonly. Does not reopen #525'spublish_verifiedfold; the recovered-digest protocol itself is untouched and proven to seat. The write-ceiling and budget-name work initially drafted here was dropped in favor of the equivalent already landed on the base (2af617019).