[ragnorc]

Refs #643.

The fix: a merge no longer holds its source

A merge held its source's branch gate and every per-table queue on the source from preparation through publication. So every write, index build and delete on the source waited for the whole merge. On a server, a merge that syncs a feature branch from main stalled every write to main for its duration (a probe: an insert on the source did not finish within 3 s while a merge out of it was parked). Merges from one source into several targets also ran one after another.

The source gate dates from #343 (RFC-022), whose comment justified the gates only by the target ("prevents a target delete/recreate from reusing the branch name underneath a plan"). It was hidden while every write took the graph-wide schema key exclusively; #783 made that key shared and exposed it.

The merge does not need the source held. Its input is the tagged source snapshot:

• The merge pins (tags) the captured snapshot, then proves the source was still at that version after tagging. That proof keeps cleanup from having reclaimed it before the tag landed.
• The final re-read before the first effect still refuses a source whose native-ref incarnation or schema identity changed.

So the merge now holds the source's branch gate only through capture, tagging and that proof, and takes per-table queues for the target only. The target gate and queues are unchanged: they stay through publication, keeping a target delete/recreate out from under the plan and serializing merges into one target.

Why not simply drop the source gate: the post-tag "still current" proof would then race concurrent source writes and refuse merges spuriously.

Every later source check compares against the captured snapshot, never the live pin. The pure-insert proof did not: revalidate_proven_pure_insert_source still required the source table's live pin to equal the proven version, so a source write landing during planning refused the merge with ReadSetChanged (found in review). The proof reads the source at the captured, tagged version, so the check now keeps identity, path and native ref and drops the version equality.

Remaining wait: both gates are taken in sorted branch order, so a merge whose source sorts first (main always does) holds the source gate while another merge holds its target. Source writes wait for a merge only until it captures its source.

For review (@ the RFC-022 author): the final re-read refuses source delete/recreate (ABA) up to that point. After it, a source delete now proceeds while the merge publishes. Data stays safe, because the merge's tags protect the source snapshot until publication, after which the target's own pins do. The merged parent's lineage stays readable through the retired branch's archive. merge_target_delete_waits_and_source_delete_does_not_issue_643 exercises this with an edge only the source added, so the target adopts the source's edge:Knows pin. The source is deleted mid-merge, cleanup runs after publication, and the target still reads the edge. If there is a reason the source must stay pinned through publication beyond this, it belongs in that comment.

Tests (failpoints, plus one in schema_apply.rs)

Concurrency

• independent_target_merges_overlap_issue_643: merge A parks after its table effects; a merge between two other branches completes within 10 s.
• same_target_merges_serialize_issue_643: a second merge into the same target waits, then merges on top.
• same_source_merges_overlap_issue_643 (changed): with a merge from s parked, a second merge from s into another target completes. Both land.
• source_write_proceeds_during_merge_issue_643 (new): an insert on the source completes while a merge out of it is parked. The merge publishes the source as captured, so the concurrent insert reaches the target only with the next merge. Its second half parks at candidate validation on a merge whose Person change is proven insert-only, lands a source insert, and requires the merge to publish the captured rows through the certified path; it failed before the pure-insert fix.

Boundaries

• parked_merge_blocks_schema_apply_issue_643: a parked merge holds its shared schema permit; an apply waits, then refuses on the non-main branch.
• merge_target_delete_waits_and_source_delete_does_not_issue_643 (changed): deleting the target waits; deleting the source does not. The merge publishes, and the adopted edge survives cleanup.
• failed_merge_beside_an_independent_merge_issue_643: a merge failing before its manifest commit leaves its target unmoved while an independent merge is parked; the same handle retries it.
• merge_refuses_a_target_recreated_by_another_process_issue_643: another process deletes and recreates the target mid-merge. The merge refuses with ReadSetChanged on branch_identifier:t, and the recreated branch keeps only its own rows.
• cancelled_merge_leaves_the_target_whole_issue_643: aborting a parked merge's task (at authority capture and after the table effects) leaves the target untouched or merged whole; the handle keeps working and cleanup succeeds.

No executable instrument measures overlap, latency or RSS today: #835's was archived with the Rust harness (#894), and #895 tracks an executor that can.

Docs

• docs/dev/writes.md states which locks a merge holds, the sorted-order wait, and that the pure-insert proof uses the captured pin.
• Release note: changelog.d/merge-releases-its-source-after-capture.changed.md.

Local checks

On main c59de336 merged (this push), all pass:

• the merge failpoint tests (29: every issue_643 and branch_merge_ test) and parked_merge_blocks_schema_apply_issue_643;
• the merge and branching integration tests (118 passed, 2 ignored; run with RUST_MIN_STACK=16777216, which composite_flow needs in a local debug build);
• cargo fmt --check, typos, git diff --check, check-docs.py and the PR title check.

Not re-run on this base: clippy, detached_commit_matrix, GQT and DST, which CI runs. On 97f293e8 they all passed, along with the full failpoints, schema_apply, changes and writes suites.

[ragnorc]

Recommendation: request changes for one correctness issue. The source-lock reduction is a useful design change, but the pure-insert path still depends on the live source version.

A merge used to keep its source branch locked for the whole operation. For example, a long merge from main into a feature branch could delay writes to main. This PR keeps that lock only while it captures, tags, and checks the source snapshot. The merge then uses that fixed snapshot. Later source writes belong to a later merge. The target stays locked through publication, so two merges into one target still serialize.

The intended contract is sound: one complete target publication from coherent, retained inputs. The problem is that one optimized path still checks whether the source table is current. A normal source insert during planning can therefore make the merge fail with ReadSetChanged, even though its tagged input remains valid. The added source-write test parks after this check and does not cover that window. See the inline finding.

Tradeoffs and liability:

• The change addresses an unnecessary input lock. It reuses existing tags and snapshot capture, branch gates, and publication checks. It adds no public API, persistent format, or second source of truth. All merge paths must now use the same captured-source rule. The remaining live-pin equality prevents that composition.
• The expected benefit is strongest when frequent writes share a source with long merges into independent targets. Same-target work remains serial. More overlapping planners can increase peak memory and I/O. HTTP admission bounds operation counts, not total RSS. These tests do not establish throughput or memory costs.
• Pinned Lance 11.0.0 tags name exact manifest versions. OmniGraph's collector traces their table pins. Branch retirement retains the physical tree and ancestry archive. Local cleanup also acquires the live target gate. These existing mechanisms support shorter source ownership without a new recovery protocol.
• The deployment boundary remains one mutation-capable writer process unless an external fence proves exclusivity. These process-local gates do not become distributed fencing. Azure keeps its admission wrapper and qualification boundary.
• The PR adds 708 lines and removes 14. Of the additions, 665 are tests. Production merge code grows by 18 net lines. The liability question is which obligations remain. Five changes of this kind should share one lock order and one snapshot identity rule, rather than add mutable-tip checks to each optimization.

After the correction, I expect this design to reduce runtime liability by using an immutable input instead of prolonged source ownership. As written, it adds a refusal that depends on the selected optimization and burdens callers with retries.

Optional contention limitation: a merge whose source sorts before its target holds the source gate while waiting for the target gate. A queued merge can therefore block source writes behind another long merge. This includes main as source. It is remaining contention, not a newly introduced data-integrity defect. Qualify the unconditional “do not wait” wording, and measure this arrival pattern before claiming source latency is independent of merge duration.

Validation at 0ab3af7cd91b4901282e1c5d4f7f13d3278fe4e6:

• Reviewed the complete diff in an isolated checkout. Read the required repository guides and relevant complete upstream documentation. Checked assumptions against OmniGraph and pinned Lance source.
• The nine original issue_643 tests passed locally: eight in failpoints, one in schema_apply.
• Temporary extensions of the existing test owners reproduced both timing cases with production code unchanged. The pure-insert probe showed that it used the certified insertion path. Its concurrent source insert succeeded, then the merge returned ReadSetChanged for branch_merge_source_dataset:node:Person.
• Removing only the live-pin equality made that same probe pass, including captured-row and next-merge assertions. This was a causal check, not a submitted fix. I restored all temporary edits.
• After restoration, all 21 existing branch_merge_ failpoint tests passed. These include captured-source cleanup, source advancement, branch recreation, publication failure, and retained input tags. The checkout is clean.
• Documentation checks passed for 166 Markdown files. AGENTS links, formatting, changed-file spelling, and diff whitespace checks passed. Local builds emitted a compact-unwind linker warning.
• CI for this exact head passed, including workspace tests, RustFS S3, and Azurite. The separate DST workflow also passed. I did not rerun cloud tests or measure production throughput, latency, or RSS.

[ragnorc]

Recommendation: approve this revision. The earlier blocking finding is fixed. I found no new blocking defect at 3a1c7a50457c2b443d3f91002ac121eadcca9dda.

A long merge used to hold its source branch for the whole operation. Writes to that source, and merges from it into other targets, had to wait. This PR releases the source gate after it captures, tags, and checks the input snapshot. The merge uses that fixed input; a later source write belongs to the next merge. The target gate and target table queues still protect publication.

The follow-up fixes the cause of the earlier refusal. An immutable input does not become invalid when the live source advances. The source check now retains table identity, path, and native-ref checks without requiring live-version equality. The common check covers both pure-insert candidate forms. Validation and physical publication read proven.source, opened at the captured pin. This restores the same captured-input rule across the optimized paths and avoids unnecessary retries.

The surrounding contract still holds. Capture keeps the global gate order and checks both inputs after tagging. Final validation still checks source incarnation/schema and exact target authority. Merge-input tags name exact manifest versions. The collector traces their table pins. Branch retirement preserves the physical tree and ancestry archive. These mechanisms protect the captured input when the source changes or is later deleted.

I checked those assumptions against crates.io Lance 11.0.0, source commit ab6b5bbe46009ed78746b444df8db59a8bc5d842. Its tag implementation resolves an explicit version separately from the latest version. I also read the repository guides and complete relevant upstream pages.

Tradeoffs and long-term liability:

• The expected benefit is strongest for frequent source writes and merges into independent targets. Same-target merges remain serial. The updated guide correctly states that a source-first merge can still hold its source while it waits for the target. main always sorts first.
• More overlap can raise concurrent I/O and memory use. These tests prove progress and snapshot behavior; they do not measure latency, throughput, or peak RSS. The overlap benchmark work in #895 remains useful, including queued same-target arrivals.
• The production change adds 20 net lines and no public API, persistent state, or new recovery protocol. It removes prolonged source ownership and an invalid live-tip dependency. The added obligation is to keep source capture, retention, and target publication roles explicit. The 742 added test lines cost maintenance, but cover concurrency and failure boundaries. Five similar changes should still share one gate order and one immutable-input rule; this change does.
• The supported environment remains one mutation-capable writer process per cluster. Process-local gates do not provide distributed fencing.

I surveyed the other 29 open PRs for overlap and direction. #825 shortens a separate coordinator-cache critical section; #818 caches exact committed table versions. Both fit this immutable-input design, but their performance numbers do not establish a combined speedup. Draft #677 proposes broader snapshot/adoption changes and does not change today's contract. The admission work in #893/#897/#899 remains a separate deployment boundary. Future Lance work also stays separate: 14.0.0-beta.13 includes a reversal of premature row-lineage stabilization. This PR needs no new Lance format or MemWAL facility. A future dependency upgrade must requalify retained snapshots and insertion proofs against that implementation.

Behavioral proof, using the same source_write_proceeds_during_merge_issue_643 regression:

Production commit	Local result
c59de33616a2fce202d0288e5f2c110cdea20c57 — current PR base, before source release	1 failed: source insert hit the 10-second timeout while the merge was parked.
8a0a8a076a55f75835bb7ba93c98c61d1d6fb2a4 — before the pure-insert fix	1 failed: source insert succeeded; merge refused with ReadSetChanged on branch_merge_source_dataset:node:Person.
3a1c7a50457c2b443d3f91002ac121eadcca9dda — reviewed head	1 passed: merge kept the captured rows; the next merge included the later insert.

Command on all three commits:

RUSTUP_TOOLCHAIN=1.97.1 \
CARGO_TARGET_DIR=/tmp/review-928/target \
RUSTFLAGS='--cfg tokio_unstable --cfg tokio_unstable' \
cargo test --locked -p omnigraph-engine --features failpoints \
  --test failpoints -j 3 source_write_proceeds_during_merge_issue_643 \
  -- --exact --nocapture

Only the test was ported to the earlier commits; their production code stayed unchanged. Both failures occurred in the running test. The new half extends the existing fixture and proves that the optimized insertion path ran. GQT cannot park a merge at this seam, run a concurrent source write, and assert its progress. The existing Rust failpoint owner is appropriate. All temporary edits are restored; the isolated checkout is clean at the reviewed head.

Additional local checks at that head, with the same build environment and RUST_MIN_STACK=16777216:

cargo test --locked -p omnigraph-engine --features failpoints -j 3 \
  --test failpoints -- issue_643 branch_merge_ --nocapture
# 29 passed, 0 failed, 0 ignored
cargo test --locked -p omnigraph-engine --features failpoints -j 3 \
  --test schema_apply parked_merge_blocks_schema_apply_issue_643 \
  -- --exact --nocapture
# 1 passed, 0 failed, 0 ignored
cargo test --locked -p omnigraph-engine --features failpoints -j 3 \
  --test integration -- merge_ branching:: --nocapture
# 112 passed, 0 failed, 2 ignored

The two ignored cases are existing large-row hydration stress tests. Formatting, AGENTS links, changed-file spelling, diff whitespace, and documentation validation passed (250 Markdown files). The plain Python docs invocation first lacked dependencies; the configured docs virtual environment passed. Builds emitted a compact-unwind linker warning.

CI, GQT, and DST passed for this revision, including workspace, RustFS, and Azurite checks. The workspace checkout log names synthetic merge commit 9f866eec420a14fe66bd68fd087a25e757df2f67; the local results above are from the exact PR head. The later PR-title run passed after the earlier failure. I did not run local cloud qualification or production performance measurements.