[ragnorc]

Why

Lance 13 creates a branch in two phases. It commits the clone, then writes the ref put-if-absent (put_ref_if_absent). The ref write's conflict reads nothing back, and its error says nothing about the object. create_branch_recoverably read the ref back after a failure. When it found the ref absent, it force-deleted the clone and retried under the same native name. The ref put might still have been in flight and could land after that delete, leaving a live ref that names a deleted clone. Unretired native refs define logical branches, so that is a broken branch.

The storage-outcomes RFC (§2, "Refs and tags") says an ambiguous create whose ref is absent on read-back stays in doubt, and that branch creation mints a fresh native name per attempt. Issue #1067 is the origin.

What

A failed native create reads its ref back and decides from what it can prove:

• Matching contents: this create's, a lost acknowledgement. This is unchanged.
• Mismatching contents: another create's, never deleted. This is unchanged.
• Absent, but may still land: the clone committed, and the failure is not a definite refusal. Lance writes the ref only after its clone commit returns, and that commit's manifest is durable by then. The ref is read again with the pauses of a conditional re-issue (0.2 s doubling to 1.6 s). If it is still absent, the create returns an in-doubt manifest publication (outcome_unknown on the server) and keeps the clone, so a late ref names a whole branch.
• Absent, and cannot land: the store refused the put, or the clone never committed. The clone is reclaimed and the error is Unpublished.

create_branch_recoverably now makes one attempt. Catalog::create_branch_from mints the native name, and it retries once under a fresh one only after an Unpublished failure.

Tests

• branch_control::tests::native_create_reads_its_ref_back_before_deciding_issue_1067 faults the ref put each way:

  • a put that applied with its answer lost: created;
  • a put that lands while the ref is read again: created;
  • a put that is lost, or held and landed later: in doubt, the clone kept, and the late ref opens;
  • a refused put, or a clone whose manifest puts are lost: unpublished, the clone reclaimed.

  No path sends a second ref put.

• omnigraph-catalog:

  • branch_create_retries_a_refused_ref_under_a_fresh_native_name_issue_1067: two ref puts under two names, the first clone reclaimed, one live incarnation.
  • branch_create_whose_ref_may_land_stays_in_doubt_issue_1067: in doubt, no retry, the clone kept; when the held ref lands, the branch lists and opens.

Open question

No later write fences a late ref. Each attempt's ref has its own {logical}.{ULID} name, so a user who repeats an in-doubt create mints a second incarnation. If the first ref then lands, list_graph_branches refuses the whole graph until one incarnation is removed. The user page says to list branches before repeating. Closing this needs one of two things:

• a fence on the logical name, such as refusing a create while a ref-less, versioned clone of the same name exists;
• re-issuing the ref write from OmniGraph, which Lance's API does not allow today.

Validation

• branch_control:: in core (20 tests), the catalog branch_create and issue_1067 tests with the candidate-tag test now on the shared conditional-put session (9), and the storage issue_1067 tests (17): all pass.
• The engine branching::, point_in_time:: and merge_cleanup:: integration suites: 76 passed.
• Clippy -D warnings on storage, core, catalog and engine. The local toolchain is 1.98 and the repository pins 1.97.1; two 1.98-only lints on untouched code were allowed locally.
• cargo fmt --check, check-docs.py and typos.
• Not run locally: the failpoint-feature engine tests, such as native_branch_controls_reclassify_lost_acknowledgements, and the workspace suite. CI runs both.

DST was not run. Its storage injects faults above the adapter, and branch creation's storage calls change only on a failure path, so the cost golden does not change.

[ragnorc]

Revise. The create path no longer deletes a clone whose branch ref may still land. cleanup --keep still does. A late ref can name a deleted clone, which is the same broken branch.

Why

Lance 13 Dataset::create_branch has two phases. It commits the shallow clone. Then Branches::create writes the ref with put_ref_if_absent. A conflict becomes RefConflict and does not read the object. Any other store error is only an error. The put can be in flight after the client has the error.

The old create_branch_recoverably read the ref, found it absent, force-deleted the clone, and tried again under the same native name. A late ref then named a deleted clone. An unretired __manifest ref is a logical branch, so that branch is broken. That breaks the rule that a durable effect stays unreachable until the publication that names it can still open it, and the rule that an unretired native ref defines a live branch.

What

A failed native create now reads its ref back.

• Matching contents are this create. The outcome is created.
• Different contents are left in place.
• An absent ref after a committed clone, when the error is not a definite refusal, is read again for about 3 seconds (MANIFEST_REQUEST_REISSUES, pauses 0.2 s to 1.6 s). If it is still absent, the create returns manifest_publish_in_doubt and keeps the clone.
• An absent ref that cannot land (the store refused the put, or the clone has no version manifest) is reclaimed. The error is Unpublished. Catalog::create_branch_from tries once more, under a new {logical}.{ULID} name.

The catalog does not retry an in-doubt create. HTTP maps that error to 500 outcome_unknown. This matches the storage-outcomes RFC for refs: an ambiguous create whose ref is absent on read-back stays in doubt, and each attempt uses a fresh native name.

GQT cannot inject a lost ref put. The Rust owners are the right tier, and the new issue_1067 tests cover the create path.

Blocking

cleanup with keep_versions and no older_than deletes the clone this function keeps.

cleanup_detached_only sets before_timestamp only from older_than. With no age cutoff, native_fork_inventory does not age-retain any file. A ref-less directory under __manifest/tree is still added to trees. It is not in protected_manifest_branches (those are opened live branches and tags). reconcile_frozen_native_forks passes that set to collect_native_forks, which calls force_delete_branch. That is force_delete_branch_idempotent.

The schema permit is held only until create_branch_recoverably returns. The in-doubt return releases it. The next cleanup in the same process is inside the single-writer boundary. The ref put has already left the single-attempt client (PutMode::Create; branch refs are KeyClass::Mutable). The server can still apply it. If cleanup runs first, the late ref names a deleted clone.

I checked that sequence on this head. An in-doubt create left tree/feature in place. force_delete_branch_idempotent removed it. Landing the held ref then made checkout_branch fail. That temporary test was removed and was not pushed.

With older_than, a tree newer than the cutoff is age-retained. After the cutoff the same delete runs. cleanup --keep alone is a supported policy (cleanup_all_datasets accepts keep_versions with no age).

The class is every reclamation of a clone whose ref put may still land. This change stops the reclaim inside the failed create. It leaves the collector, which already exists to delete ref-less trees. create_branch_recoverably also still reclaims a ref-less tree of the same native name before the attempt. The catalog avoids that by minting a new ULID. Cleanup does not.

Put the rule in one owner: native-tree retention. A versioned ref-less __manifest tree stays until that ref is settled, or an operator removes that native name. cleanup --keep must not treat it as a zombie. Add a regression on the maintenance owner: in-doubt create, cleanup with keep_versions: Some(1) and older_than: None, then land the ref. The branch must open.

Keeping every such tree forever leaks clones whose ref was never sent (the process died after the clone commit, or the refusal path died before reclaim). That leak is the cost of one retention rule. Age does not prove the put cannot land. A per-tree witness written only on the in-doubt path would let the collector tell the two cases apart. A second writer of the ref, with bytes Lance did not mint, is a larger mechanism. This PR is right not to add that writer.

Optional

1. Two live incarnations have no working removal step. A repeated create uses a new ULID. If the first ref lands later, list_graph_branches fails for the whole graph and tells the operator to run cleanup. cleanup_graph_branches calls that same list and stops. The user page says the listing refuses until one copy is removed, and does not say how. Name a removal that does not depend on list_graph_branches. This is the open question in the PR body. The error text makes the recovery look available when it is not.

2. The new Branches/tags fence in docs/dev/lance.md does not name the Lance change that removes the poll. The branch-ref read row does: delete the retry at the bump whose from_path reads a ref in one get. Add the same kind of sentence for a create_branch that settles the ref put itself.

Tradeoffs and liability

The poll is the right local substitute for a re-issue. Lance does not return the BranchContents bytes, so OmniGraph cannot send the same put again. Up to 3 seconds on the failure path, with the schema permit held, is acceptable. Branch create is rare. The poll does not fence the key.

The change removes a same-name retry that deleted the clone. It adds an in-doubt outcome, the poll, and a kept clone. The kept clone is not durable against the next cleanup --keep. Five more conditional-ref fixes in this shape would each grow a poll and a "keep the object" comment, while the collector still deletes anything with no ref. Candidate tags already did the read-back half. The collector half is still missing, so the next case will miss it too.

branch_tree_has_version is a directory listing used as proof that the clone commit landed. V1 and V2 manifests both end in .manifest, so the extension check matches Lance 13. The commit handler already re-issues an ambiguous manifest put before create_branch returns. A manifest that is still invisible is treated as "the ref was never sent", and the tree is reclaimed. That can race a late manifest file that has no ref. It does not by itself create a live broken branch.

#1176 re-issues conditional control-object writes and edits this same RFC section and the storage-failures page. The split is consistent: re-issue when OmniGraph owns the put, read back when Lance owns it. Expect a doc conflict. #1181 edits cleanup tests. It does not change this retention rule.

Validation

Head 8f9a9edc47edeff7dac877a239e22c341fc881b9. Merge base da1c46368c33876d61615add0ceb6778f9ce2b58.

At the merge base, a temporary omnigraph-core test held every feature.json put, called create_branch_recoverably, and required the clone to remain so a later landing could open the branch. Restored after the run. Not pushed.

CARGO_TARGET_DIR=/tmp/pr1183-base-target cargo test -p omnigraph-core --locked --lib \
  late_ref_after_ambiguous_put_opens_the_kept_clone_issue_1067 -- --test-threads=1 --nocapture

Failed in 0.01 s. Panic: the clone a late ref would name must stay. The old function had reclaimed the clone before return.

At the head:

CARGO_TARGET_DIR=/workspace/target cargo test -p omnigraph-core --locked --lib \
  native_create_reads_its_ref_back_before_deciding_issue_1067 -- --test-threads=1 --nocapture

ok. 1 passed, 6.24 s. That test includes the lost acknowledgement, the put that lands during the reread, the held put, the refused put, and the clone that never commits.

CARGO_TARGET_DIR=/workspace/target cargo test -p omnigraph-catalog --locked --lib branch_create_ -- --test-threads=1 --nocapture

ok. 2 passed, 3.05 s. branch_create_retries_a_refused_ref_under_a_fresh_native_name_issue_1067 and branch_create_whose_ref_may_land_stays_in_doubt_issue_1067.

On this head, a temporary test then called force_delete_branch_idempotent after the in-doubt return and landed the ref. checkout_branch failed. ok. 1 passed, 3.02 s. That test was removed and was not pushed. It shows the delete. It is not a run of Omnigraph::cleanup.

CI on 8f9a9edc47edeff7dac877a239e22c341fc881b9 is green, including Test Workspace, Lint (clippy), GQ Logic Tests, DST pinned suite, RustFS S3, and Azurite. Test Workspace includes these unit tests. Those jobs do not run cleanup after an in-doubt create.

Limitations

No full cleanup --keep 1 journey against an in-doubt graph. The deletion decision is from cleanup_detached_only, native_fork_inventory, and reconcile_frozen_native_forks on this head. No S3 timing measurement of how late a timed-out put_if_absent can land. No DST run here. CI's DST pinned suite passed on this commit. The author reports no storage-cost golden change, and the success path does not add a storage call. I did not regenerate the golden.

Inline comments

crates/omnigraph-core/src/branch_control.rs:943 (RIGHT), blocking. This return keeps the clone only until the function returns. cleanup with keep_versions and no older_than sets no age cutoff (cleanup_detached_only). native_fork_inventory puts a ref-less __manifest tree in the deletion set. collect_native_forks calls force_delete_branch, which is force_delete_branch_idempotent. A held ref put produced this in-doubt error, that delete removed tree/feature, and landing the put made checkout_branch fail. Retain that tree in the collector until the ref is settled or an operator removes that native name. cleanup --keep must not delete it.

docs/user/operations/storage-failures.md:46 (RIGHT), optional. This paragraph says the copied state is kept. cleanup --keep deletes that state while the ref can still land. When two copies exist, list_graph_branches tells the operator to run cleanup, and cleanup_graph_branches calls that list and stops. Name a removal that does not call list_graph_branches.