Skip to content

feat(storage): totally order table registrations by the manifest version - #686

Merged
azimafroozeh merged 1 commit into
ModernRelay:mainfrom
azimafroozeh:manifest-version-clock-rfc
Sep 8, 2026
Merged

azimafroozeh merged 1 commit into
ModernRelay:mainfrom
azimafroozeh:manifest-version-clock-rfc

Conversation

@azimafroozeh

@azimafroozeh azimafroozeh commented Sep 8, 2026 •

Copy link
Copy Markdown
Collaborator

What & why

Table registrations in __manifest had no total order across native refs. #630 already fixed the bugs that came from that, with a rule in the merge path. This PR is the structural improvement behind it: it gives registrations a total order by construction, so the rule and the guards it needed go away. It closes no issue. RFC 0062 (docs/rfcs/0062-manifest-version-clock.md) ships in the same change; storage stamp v6 to v7.

What it was. A registration is (table_version, native_dataset_branch), and three sites ordered registrations by table_version alone. Lance numbers a branch's versions from its fork point, so that number orders only inside one native ref. The fold kept the greatest number per table, so an adopted registration with a lower number was invisible: the merge reported fast_forward and the target kept stale rows. The row key was that number, so an equal number on another ref replaced the stored row in place. The collision guard refused the equal-number cross-ref adopt with table version N already exists ... with different state. #473 was the first of the family.

How #630 fixed it. adopt_requires_target_lineage: an adopt whose source number is at or below the target's goes through the target-lineage delta writer, so the registered number always grows on the target. Correct and shipped. What it leaves: the order is a rule every registering path must keep, the equal-number case pays a data write for a pointer move, and an equal number on another ref still passes the pin's version compare.

How this one works. The __manifest version that publishes a registration is unique and increasing within a branch's lineage, because every graph branch is a Lance branch of __manifest and the fold compares one lineage. Each registration row now carries that version as the trailing segment of its row key (table_version:{stable:016x}:{inc:016x}:{manifest_version:020}), decoded on scan. The fold, the tombstone filter and the publisher's per-identity reductions select the greatest manifest version. The physical pointer, (published_dataset_version, native_dataset_branch), is unchanged. No column is added: a projected column costs one manifest read per fragment per change-feed page, which tests/changes_cost.rs caps. The Arrow schema of __manifest is unchanged; RFC 0028 §4.5 is amended.

Removed. The collision guard, the owner-branch handoff arm, the number-keyed existing_versions and existing_tombstones lookups, and #630's predicate with its routing. An adopt at a lower or equal number is a pointer switch, or a fork onto the target's own ref when the target has none.

Added.

  • Loud refusals (manifest_internal on read, OmniError::manifest on publish): two rows at one (identity, manifest version), a row stamped above the scanned version, a second Update or Tombstone for one identity in one batch, a registration of a tombstoned identity, a tombstone of a sealed identity, a commit landing at a version other than the one stamped into its rows.
  • The writer's pin TableVersionExpectation gains native_ref: NativeRefPin { Unchecked, Exact(Option<String>) }; check_expected_table_versions refuses a winner at the pinned number on another native ref as ReadSetChanged. This removes the pin gap fix(merge): preserve state when source table versions lag target #630 left open. Sidecar v9 and the 409 payload are unchanged.
  • Stamp v7 in migrations.rs; a v6 graph is refused on open and rebuilt by export and load. The MemWAL v7 to v19 stamps never shipped, so v7 is reused.

Backing issue / RFC

Checklist

  • Change is focused (one storage-format change: the registration clock, its stamp bump, its read-side refusals, and the pin's native ref)
  • Tests added/updated for behavior changes (GQ cases: merge_adopt_equal_version_return_trip, guard issue_473_merge_net_zero_branch; fix(merge): preserve state when source table versions lag target #630's three merge_adopt_* cases stay green without its predicate. db/manifest/tests.rs: later clock wins at an equal Lance number, lower Lance number on another ref wins the fold, batch duplicate refused, tombstoned and sealed identities refused, pin on one ref refused when the winner sits on another, warm fold and fresh reopen agree. crossversion_upgrade.rs: current_v7_refuses_and_rebuilds_genuine_v6_and_v6_refuses_v7 under OMNIGRAPH_V6_BIN, the v0.9 journey rewritten to refuse and rebuild)
  • Public docs updated if user-facing surface changed (docs/user/operations/upgrade.md, docs/releases/v0.11.0.md, docs/dev/versioning.md, recovery.md, writes.md, testing.md)
  • Reviewed against docs/dev/invariants.md: no Hard Invariant weakened, no deny-list item hit (2 unchanged, 3 strengthened by the native-ref pin, 8 stays loud through the read-side refusals, 12: the clock is __manifest's own version)

Local verification

  • cargo test --workspace --no-fail-fast: 99 targets ok; two pre-existing failures, equally red on an untouched base: blob::tests::external_blob_file_policy_rejects_special_files (sandbox refuses UnixListener::bind) and omnigraph-gqt --lib tests::branch_list_shape_and_rows_are_blessed_like_a_read_step (stack overflow, dg/fix-rust-min-stack)
  • cargo test -p omnigraph-engine --features failpoints --test failpoints: 152 passed in the workspace run with branch_merge_phase_b_failure_recovered_on_non_main_target red before its re-assertion below; that test passes alone after it
  • cargo test -p omnigraph-gqt --test gq_logic_tests -- merge_adopt_: 4 passed; -- issue_473_merge_net_zero: passed
  • cargo test -p omnigraph-engine --test changes_cost: passed (the column design failed it, 21 to 23 reads)
  • cargo test -p omnigraph-cli --test crossversion_upgrade: compiles and skips locally (OMNIGRAPH_V09_BIN, OMNIGRAPH_V6_BIN unset); CI's Test Workspace and V5 ↔ V7 Format Fence run them
  • cargo clippy --workspace --all-targets, cargo clippy -p omnigraph-engine --features failpoints --all-targets, cargo fmt --all --check, python3 scripts/check-docs.py: clean

Notes for reviewers

  • Behavior change: a merge adopting a source at or below the target's Lance number registers the pointer switch and the target reads the adopted state; the collision message no longer exists.
  • v7 is a cutover for every 0.9.x and 0.10.x deployment: omnigraph export --branch <b> per live branch, init and load into a new graph; branch topology, history and snapshots do not carry across. A converter is possible (_row_last_updated_at_version holds every row's manifest version) and out of scope; RFC 0062's one unresolved question.
  • Against fix(merge): preserve state when source table versions lag target #630: its branching.rs, merge_fast_forward.rs and failpoints.rs tests are re-asserted under the clock. A target owning its ref takes the source as a pointer switch at the source's version; a lazy target takes the existing fork arm. The lazy iteration of branch_merge_preserves_state_when_native_versions_differ stops after its reads: a write on a ref forked from another branch's ref fails with NotFound on the parent's _indices/ (Index reads fail with Not found: .../tree/<parent>/_indices/... through a branch created from another branch lance-format/lance#7840), on main today for a newer source as well; recorded as a known gap in the RFC's decision log. In branch_merge_phase_b_failure_recovered_on_non_main_target the two main-source cases are pointer switches with no phase-B effect: no sidecar, target untouched, the retry completes. blob_changed_only_adopt_uses_known_present_update's main-to-branch arm asserts a pointer switch: nothing staged, nothing probed, the branch reads main's external descriptor as stored.
  • Accepted cost: a pointer switch off a branch's own fork ref orphans that ref; the next first-touch write pays reclaim_orphaned_fork_and_refork. fix(merge): preserve state when source table versions lag target #630's delta route never orphaned a ref.
  • Remaining pin gap: recovery slot replays and version-0 registrations pin Unchecked; heal_pending_recovery_sidecars_for_write replays every pending sidecar before any writer runs, so nothing lands in between. Recording the read ref needs sidecar v10, a follow-up.
  • graph_commit rows keep the manifest version in the overloaded table_version column at this stamp; retiring that belongs to draft RFC 0058.
  • The v0.9 end-to-end test opened a v6 graph in place, which v7 refuses; its journeys now run on a root rebuilt from a 0.9.0 export, and the in-place backup/rollback block is gone.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@azimafroozeh
azimafroozeh merged commit dcee785 into ModernRelay:main Sep 8, 2026
23 of 26 checks passed
@azimafroozeh
azimafroozeh deleted the manifest-version-clock-rfc branch September 8, 2026 13:03
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