perf(branches): preserve snapshots and reuse verified views - #662
Merged
Conversation
This was referenced Sep 6, 2026
aaltshuler
force-pushed
the
codex/repro-untouched-table-merge
branch
from
September 7, 2026 12:49
a8aa5ea to
47cb4c0
Compare
aaltshuler
force-pushed
the
codex/branch-operations-refactor
branch
from
September 7, 2026 12:49
d52e668 to
f57268d
Compare
5 tasks
Review all 103 ledger rows: five read/list counters change and every write, delete, adapter and verification counter remains identical. Branch creation saves 65 GETs; deletion saves 47 GETs and one LIST. Retained projections save 20 Optimize GETs while compaction fallback adds two latest-version LISTs. The golden owner replays two identical universes before regeneration.
…ming Merging main into a branch that owned a table fork lost a child's rows (#683), and a first touch that met a leftover ref could wedge the next open. - A main-source merge into an owning branch points the table at main again; the delta write onto the owned fork is gone. While a child still pins the former fork, the branch's next write to that table is refused with `detached native lineage`. #683 is a refusal now, not a fix; unique fork names follow in a separate PR. - Merge and write first touches classify an existing target ref before arming: an orphan is dropped, a borrowed ref is refused, a claimed ref is a retryable conflict. The adopt-source fork runs under the armed operation id. - Tests: five gqt cases for these shapes, three duplicates removed, crash pins in failpoints.rs, the topology diagnostics module dropped, registry row for the new read site. Docs and release note say the pointer rule and the refusal.
azimafroozeh
force-pushed
the
codex/branch-operations-refactor
branch
from
September 8, 2026 19:16
f57268d to
e3533e9
Compare
azimafroozeh
changed the base branch from
codex/repro-untouched-table-merge
to
main
September 8, 2026 19:26
4 tasks
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
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.
What & why
Branch operations repeatedly reconstruct manifest history, and parent merges or cleanup can invalidate native table history still borrowed by a child. Preserve that history and reuse freshly verified source, target, and borrower views so repeated operations do less storage work while retaining the existing publication and recovery checks.
Stacked on #630, using
codex/repro-untouched-table-mergeas the base. That PR owns the original lagging-source-version merge fix. This PR contains the subsequent branch refactor and small-fixture diagnostics. Merge #630 first, then retarget this PR tomain.Backing issue / RFC
Related performance work: #641 and #642. This PR does not close their broader acceptance criteria. Accepted-context reuse is part of canonical RFC 0057, accepted separately in #638. The included RFC 0058 document remains a draft proposal and is not implemented or used as authorization for a format change.
Measurements
The latest reuse comparison is
edb8a87dbf31484d7261d025edf3822e4246e584→86aa508580097cf46fb4c32b55f053d41f98e412. Both saved release executables were built from verified clean sources. The reference already contains the earlier branch refactor below; these numbers isolate the later view-reuse changes.Core matrix: 65 points / 195 verified samples per side, three repetitions, fixed
rows=16, four-dimensional values, H0/H16/H64 history, cold/read-metadata-warm × compacted/uncompacted layouts. Largest fixture: 9.87 MiB, with maximum operation-process RSS about 65 MiB on each side.Cold, uncompacted operation medians, excluding separately measured initial open:
At H64, fork removes one manifest scan and is 64% faster; deletion removes one scan and is 16% faster in this local matrix. Open, prewarm, first-read, and reclamation measurements did not cross the diagnostic regression threshold; first-read request counts were unchanged. Separate release width controls contain 12 points / 36 samples per side. Separate debug topology diagnostics cover ancestry, legacy refs, and concurrent merges; their timings are not pooled with release measurements.
Five original merge cells showed +11–21% timing increases with unchanged reads/scans. All original results were retained. A matched 84-sample alternating check did not reproduce those five flags (−5.5% to +4.7%); a further 36-sample fresh control measured 19.808 → 20.050 ms (+1.2%). No merge speedup or zero-overhead claim follows. These are local directional medians, not S3 measurements or tail-latency estimates; OS cache was uncontrolled.
The earlier refactor phase, measured separately at
24f5cb3c, reduced delete acknowledgement at 32 branches / 16 tables from 79.752 → 63.855 ms and total deletion from 93.343 → 73.213 ms. Its mixed-merge manifest GETs fell 1,261 → 1,049, while splitting insert/update effects raised table GETs 81 → 105 and PUTs 8 → 12. Stronger borrower proofs also raised that fixture's setup from 1.193 → 2.035 s and delete-process RSS from 48.984 → 54.938 MiB. These earlier results are not measurements of final HEAD and are not combined with the later baseline.Reproduction and accounting controls are documented in
benchmarks/README.md,scripts/bench-branch-age.py, anddocs/dev/testing.md.Checklist
Rebase and GQT verification — 2026-09-07
Rebased onto updated #630 at
47cb4c02; this stack now includes main5f94a741and Azim's branch statements/GQT support.parent_merge_preserves_lazy_child.gqt. It fails on fix(merge): preserve state when source table versions lag target #630 at47cb4c02after the parent's next write: the child's historical published dataset version 3 was reclaimed. The case records this witnessedred_onresult.empty_merge_preserves_lazy_child_first_write.gqt. It already passed on fix(merge): preserve state when source table versions lag target #630 and guards the empty-delta path plus the child's first write from its original snapshot.f57268dc: all 52 GQT cases passed, including fix(merge): preserve state when source table versions lag target #630's two regressions. New child cases take about 0.1 s each and seed only one row.The GQT runner keeps the new branch-control and per-step branch syntax while preserving this PR's boxed execution boundary.
Original implementation verification (before this rebase)
The following checks passed on
3a879157d99e4873b0fd8f5fc40034b8d871b249; the measured86aa5085added only 23 lines of testing documentation. Subsequent RFC-numbering corrections are documentation-only; focused documentation checks passed again. Formatting ran before only the reviewed DST cost-golden and documentation changes; Rust/build/test implementation was identical. No builds or benchmarks were repeated merely to open this PR.cargo test --workspace --locked --features omnigraph-engine/failpoints,omnigraph-cluster/failpoints --no-fail-fast -- --test-threads=1— 2,963 top-level passed, 27 ignored, plus two nested helper passes. Includes GQT.crates/omnigraph-dst, with itstokio_unstableconfiguration andOMNIGRAPH_DST_SEEDS=301:cargo test -p omnigraph-dst --locked --profile dst --no-fail-fast -- --test-threads=1— 78 passed, 30 ignored. CPU/Rayon/Tokio pools 1, I/O 2. Optional concurrent and two-seed supplements also passed; the concurrent supplement reported nine scheduler escapes, so it does not establish strict replay.cargo clippy --workspace --all-targets --locked -- -D warnings -W clippy::dbg_macro— passed.cargo clippy --workspace --all-targets --locked --features omnigraph-engine/failpoints,omnigraph-cluster/failpoints -- -D warnings -W clippy::dbg_macro— passed.cargo test --workspace --locked --features omnigraph-engine/failpoints,omnigraph-cluster/failpoints,omnigraph-server/aws --lib --bins --test auth_policy --test boot_settings --test data_routes --test multi_graph --test openapi --test s3 --test schema_routes --test stored_queries --no-fail-fast -- --test-threads=1— 1,827 top-level passed, 4 ignored, plus two nested helpers; includes all eight server integration owners. Live S3/Azure environments were unconfigured, so this verifies the feature graph, not live cloud operation.cargo fmt --all --check,git diff --check,bash scripts/check-agents-md.sh,python3 scripts/check-docs.py, andpython3 scripts/check-workflow-action-pins.py— passed.Notes for reviewers
Review native-ref ownership and recovery first, then exact-view capture and projection handoff, then the bounded read paths and benchmark controls. Projection outcomes are boxed to avoid enlarging async stack frames; a small boxed GQT runner boundary fixes a separately reproduced default-stack failure.
No new persistent state, storage/recovery format, or dependency upgrade is introduced. Publisher validation still scans history; uncached create-from and cold opens remain history-dependent. Eight-sibling deletion still requires nine scans in the width control. Same-graph merges still serialize. Retained projections can consume memory proportional to historical table lifetimes. RFCs 0024/0025/0058, historical ancestry retention, and broader cold-access/concurrency work remain separate.