fix(engine): transactional merge publication and rollback - #140
Merged
Conversation
Merge workers repoint live identities to their output before publishing the replacement snapshot. If apply_merge failed, the async task was cancelled, or the worker panicked, those repoints survived and referred readers to an output that was never authoritative. Add a conditionally-applied VersionRepointTransaction that preallocates its rollback log, records exact displaced entries, and restores an entry only when it still matches the transaction's installed value. This preserves concurrent newer writes and deletes. Commit the transaction only after apply_merge succeeds, and count publication failures in the existing merge failure result. Also make apply_merge restore its exact in-memory input snapshot when manifest persistence fails. Irreversible version-map cleanup now runs only after durable snapshot publication. A delayed-save test exercises the post-RCU failure boundary and verifies the restored inputs survive restart. Tests: cargo test -p xerj-storage version_map::tests::merge_repoint --lib (3 passed); cargo test -p xerj-storage apply_merge_manifest_failure_restores_exact_input_snapshot --lib (1 passed); cargo check -p xerj-engine; cargo clippy -p xerj-storage -p xerj-engine --all-targets -- -D warnings; cargo fmt --all --check.
A merge publication save can fail after changing the in-memory snapshot. If the compensating input-manifest save also fails, restart authority is unknown and callers must not receive the ordinary NotPublished classification. Extend the deterministic snapshot-save fault injector to emit consecutive failures and prove apply_merge_with_repoints returns Indeterminate for the publication-plus-rollback failure sequence. The existing single-failure test continues to prove a successful rollback is classified as NotPublished and survives restart. Test: cargo test -p xerj-storage apply_merge_reports_indeterminate_when_publication_and_rollback_saves_fail --lib.
Promote the merge publication prerequisite with deterministic engine-level coverage at the boundary where workers have repointed identities but have not yet published their output. The suite proves an apply failure restores exact sources, search totals, restart state, and permits a successful retry; real concurrent PUT and DELETE operations survive rollback with their source and tombstone intact across restart; task cancellation drops the transaction and restores input ownership; and a panic immediately after durable publication leaves both snapshot and version map on the output across GET, search, and restart. The multi-batch case exposed a separate control-flow bug: an apply failure continued before replenishing the in-flight queue, suppressing remaining disjoint batches. Top up the queue before continuing so successful batches still publish while the pass reports the failed batch. The regression proves all sources survive, one batch commits, the failure propagates, and restart query totals remain exact. Tests: cargo test -p xerj-engine merge_publication_transaction_tests --lib (5 passed); cargo test -p xerj-storage version_map::tests::merge_repoint --lib (3 passed); cargo test -p xerj-storage apply_merge_manifest_failure_restores_exact_input_snapshot --lib; cargo test -p xerj-storage apply_merge_reports_indeterminate_when_publication_and_rollback_saves_fail --lib; cargo check -p xerj-engine; cargo clippy -p xerj-storage -p xerj-engine --all-targets -- -D warnings; cargo fmt --all --check.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #137 (contains its three commits, rebased onto main after #135 landed; authorship preserved). #137 is a
probelabsfork with maintainer-edit off, so it could not be rebased in place.Makes the production merge path transactional across provisional version-map ownership and durable segment-snapshot publication. Before this, a merge worker repointed live document identities to its output segment before
apply_mergemade that output authoritative — an apply error, cancellation, or panic could leave the global version map aimed at an unpublished segment, so reads reject the still-authoritative input rows as stale and cannot resolve the replacement: missing documents.The repoint is now wrapped in an RAII
VersionRepointTransaction, committed only insideapply_merge_innerimmediately after the durable manifest save, and rolled back on Drop on every earlier exit (apply error, cancel, panic).Verification (adversarial, xhigh)
Every begin→commit ordering was traced for panic/cancel. Rollback restores each version-map entry to its captured prior under a guarded
(seq_no, failed_segment)check, so a concurrent newer PUT/DELETE is never clobbered. The merged output is written bySegmentWriter::finish(seg+sidx only) and never receives.ids/.complete, so it can't be orphan-resurrected → no post-merge duplication. Fault-injection tests drive the exact orderings (failpoints before/mid/after repoint, double snapshot-save failure, partial multi-input disarm). Rebased-on-main gates:xerj-storage+xerj-engine378 + 140 tests, 0 failed; clippy-D warningsclean; fmt clean.Disclosed, non-blocking
ghost_eventsis incremented on repoint but not decremented on rollback (monotonic by design) — a rolled-back merge leaves it slightly inflated, which only keeps delete-aware slow paths on (conservative). A pre-existing success-path transient (worker repoints before apply publishes, so a GET-by-id can momentarily miss a surviving doc during a normal merge) is not introduced or fixed here.