Commit d48f129
authored
Fix history independence with Dolt-style streaming chunker (#185)
* Enforce history independence via canonical rebuild after mutations
ProllyTree mutations (insert/insert_batch/delete/delete_batch) now call
canonicalize(), which rebuilds the tree from its leaf-level (key, value)
pairs so the resulting shape and root hash depend only on the final set
and the TreeConfig, never on operation history. The rebuild chunks the
sorted sequence with the existing rolling-hash chunker, emits leaves,
then iteratively chunks each level's pivots until a single root remains.
Add a history-independence test suite at three layers:
* ProllyNode unit tests in src/node.rs - the primitive layer that
bypasses canonicalize still has order-dependent balance behavior;
these tests are #[ignore]d as the acceptance suite for a future
incremental rebalance.
* ProllyTree unit tests in src/tree.rs - 4 always-on tests covering
root hash equality across orders, plus update and delete paths.
* tests/history_independence.rs - 6-test integration matrix covering
(config x N x key-pattern x order x op-mix) at the public API.
Trade-off: canonicalize is O(N) per mutation, so an N-key insert is
O(N^2) overall. Acceptable as a correctness-first stop-gap; the
incremental fast path can return once the node-level tests validate it.
* Extend history-independence tests to versioned and namespaced stores
The canonicalize fix in ProllyTree::insert/delete is inherited by the
git-backed store layers because both ultimately call through to the
ProllyTree mutation API. Add tests at those layers to verify:
- GitVersionedKvStore: insert/update/delete sequences in different
orders produce identical committed tree root hashes (3 tests in
src/git/versioned_store/tests.rs).
- GitNamespacedKvStore: per-namespace root hashes are order-
independent, including under cross-namespace interleaved writes
with multiple intermediate commits (3 tests in
src/git/versioned_store/namespaced_tests.rs).
All 6 new tests pass under the default, git-only, and minimal feature
flag combinations.
* Bench: document canonicalize stop-gap perf cost (3x-26x slower N=100-10k)
* Verify Tree::merge_trees / apply_merge_results inherit canonicalize
The merge path was already using ProllyTree::insert / delete to apply
adds, modifications, and removes to a destination tree, so it inherits
the canonicalize step for free. Add three tests that pin this:
* apply_merge_results_is_canonical - merging two divergent trees
produces a result whose root hash matches a fresh canonical build
of the same final key/value set.
* apply_merge_results_independent_of_result_order - shuffling the
merge results before apply produces the same root hash.
* merge_trees_ignore_conflicts_is_canonical - the higher-level
convenience wrapper also produces a canonical result.
All three pass without further code changes.
* Batch commit-time mutations through one canonicalize per commit
VersionedKvStore::commit and NamespacedKvStore::commit_impl both drained
their staging area by calling ProllyTree::insert / delete for each
item - and since each ProllyTree mutation triggers a full canonicalize
(O(tree size)), a commit with K items cost O(K * tree_size).
Add ProllyTree::apply_changes(impl IntoIterator<Item = (Vec<u8>,
Option<Vec<u8>>)>) which materializes the current state into a BTreeMap,
applies all changes, then rebuilds the tree canonically once. Wire the
two commit paths through it; namespaced commit collects the externalize
threshold transformation into the batch before applying.
Result: commit cost drops from O(K * N) to O(N + K log K), a meaningful
win for any commit that stages more than a handful of changes.
* Step 3 follow-up: document the incremental-rebalance redesign needed
Attempted a focused fix to lift the canonicalize stop-gap (a `removed`
flag on ProllyNode set when a delete empties a node, with parent
dropping it). The change compiled but broke node::tests::test_delete
because the existing `merged` / `split` two-bit signaling protocol
isn't expressive enough to share state with the new flag.
Reverted. Documented why a small fix isn't tractable and what the
correct redesign looks like: a richer BalanceOutcome signal carrying
"remove range [l..r] and insert these new children", with the rebalance
loading both left and right neighbors and recursing up to the root.
The canonicalize stop-gap remains in place. The #[ignore]d node-level
tests in src/node.rs are the acceptance suite for the redesign work.
* docs: Dolt streaming-chunker analysis and ProllyTree port plan
* Replace canonicalize backend with streaming chunker (Dolt port, Phase 1)
Add src/streaming_chunker.rs - a Rust port of Dolt's streaming chunker
pipeline (chunker.go + node_splitter.go + node_builder.go). The chunker
streams sorted (key, value) pairs through a Splitter that resets at
every chunk boundary; each chunk's split decision depends only on the
items inside that chunk, so the same sorted input always produces the
same root hash by construction.
Phase 1 scope: no cursor support yet. The chunker is wired in as the
backend for ProllyTree::canonicalize and apply_changes, replacing
ProllyNode::build_canonical_from_pairs. Per-mutation cost is still
O(N) until Phase 2 adds NodeCursor + advance_to for fast-forward over
unchanged subtrees.
Tests: 5 new unit tests in src/streaming_chunker.rs verify empty
inputs, single-item, sub-min sequences, order-independence, and parity
with build_canonical_from_pairs at N=1024. Existing suite of 154 lib
tests + integration matrix all pass.
Perf: cargo bench --bench tree -- insert_single_10000 drops from 30.1s
(canonicalize stop-gap) to 13.3s - 2.3x speedup. Still 11.7x slower
than the pre-fix baseline (1.14s), which is what Phase 2 closes.
* Cursor-driven apply_mutations replaces the legacy insert/delete path
Add NodeCursor (Rust port of Dolt's tree.cursor): a stateful linked
list of cursors, one per tree level, with `at_key`, `at_start`,
`advance`, etc. Add `apply_mutations(root, sorted_mutations, config,
storage)` which walks the old tree with the cursor and streams
unchanged + mutated items through a fresh chunker, producing a
canonical new tree.
ProllyTree::{insert, insert_batch, delete, delete_batch} now route
through `apply_changes -> apply_mutations`. The old in-place
`self.root.insert/delete` calls (which couldn't maintain history
independence) are gone from the public mutation path.
Tests: 5 new apply_mutations cases (empty tree, no-op pass-through,
insert middle, deletes, updates) all pass; 159 lib tests + integration
matrix + all other suites green.
Perf: bench at N=10k went from 30.1s (canonicalize stop-gap) -> 13.3s
(Phase 1 streaming) -> 16.6s (Phase 2 cursor). The Phase 2 number is
slightly worse than Phase 1 because the cursor adds overhead without
yet enabling the fast-forward skip. Adding the cursor fast-forward
(matching Dolt's chunker.advanceTo) is the next step and is what
brings the per-op cost back to O(log N + edits).
* docs: update history-independence + perf docs after Phases 1+2
Refresh docs/bench-canonicalize-impact.md and
docs/dolt-streaming-chunker.md with the bench numbers and
status after Phase 1 (streaming chunker) and Phase 2 (cursor-driven
apply_mutations).
State after these phases:
* Public API at every consumed layer (ProllyTree, GitVersionedKvStore,
GitNamespacedKvStore) has history-independence by construction -
no more post-hoc canonicalize rebuild.
* Bench at N=10k: 16.6s (14.5x slower than pre-fix baseline of 1.14s)
* The cursor fast-forward (chunker.advanceTo) is the next step and
is what closes the perf gap to ~2x of baseline.
* tree: drop dead canonicalize/collect_pairs - replaced by streaming chunker
* streaming_chunker: Phase 3 fast paths (pure-append + alignment fast-forward)
Add two complementary fast paths to apply_mutations that recover most
of the per-op cost lost in Phase 2 (cursor-driven walk):
1. **Pure-append** (`try_pure_append`): triggered when every mutation
is an insert past the tree's max key (common in append-only and
monotonic-key workloads). Walks the tree's leaves once to collect
(firstKey, leaf_hash), feeds all but the last leaf directly to the
level-1 chunker via `Chunker::append_subtree_at_parent_level`, then
re-processes only the last leaf's items + new keys through level-0.
Skips reading the items of N-1 leaves.
2. **Alignment-aware fast-forward** (`fast_forward_to_end`): after each
chunker boundary emit, the chunker exposes its `last_emit_hash`.
When the cursor was at the end of an old leaf and the emitted hash
matches that leaf's hash, the chunker has re-synced with the old
tree. If no more mutations are pending, we copy the remaining old
leaves directly to the level-1 chunker without reading their items.
Both paths preserve canonicality: they feed the same sequence of
(firstKey, hash) entries to the level-1 chunker that a full canonical
rebuild would, just without re-walking unchanged leaf items. Tests:
the 6 history-independence tests at the ProllyTree API layer + the
integration matrix + the 10 streaming-chunker unit tests all pass.
Bench: insert_single at N=10k drops from 16.6s (Phase 2) to 11.2s
(Phase 3), a 32% speedup. vs the canonicalize stop-gap (30.1s) the
total speedup is 63%. The remaining gap to the pre-fix baseline (1.14s)
is the inherent O(N²) of one-mutation-per-call and the cost of writing
new tree nodes to storage on every commit.
* docs: scrub stale ProllyTree::canonicalize references
The canonicalize method was removed in 2c019bf when the streaming
chunker took over. Update all the comment/doc references that still
mentioned it across the codebase: src/streaming_chunker.rs,
src/node.rs test-module header, src/tree.rs merge tests,
src/git/versioned_store/{tests,namespaced_tests,namespaced,core}.rs,
and tests/history_independence.rs.
Also annotate ProllyNode::build_canonical_from_pairs as the alternate
canonical builder kept as an independent oracle in the
matches_node_build_canonical_from_pairs unit test - it is no longer
in the production path, the streaming chunker handles every mutation.
* Phase 4 cleanup: remove obsolete node-level ignored tests and stale roadmap
Two cleanups now that the streaming-chunker port is the only mutation
path and the public-API + integration tests cover history independence
end-to-end:
1. Remove the 6 #[ignore]d tests in src/node.rs and their dedicated
`build_node` helper. These exercised the legacy in-place
`Balanced::balance` path and documented bugs (chunk-boundary drift
under non-default configs, empty trailing leaves after delete,
internal-node structure divergence) that no longer matter for the
public API. Coverage of history independence now lives in:
* src/streaming_chunker.rs (10 unit tests)
* src/tree.rs::history_independence_tests (4 tests)
* src/tree.rs::merge_canonicality_tests (3 tests)
* src/git/versioned_store/tests.rs::history_independence_tests (3)
* src/git/versioned_store/namespaced_tests.rs (3 tests)
* tests/history_independence.rs (6 integration tests)
The narrow `test_history_independence` and the lifted-to-u64
`test_history_independence_default_config_traversal` remain as
ProllyNode-level smoke checks against the legacy path. Updated the
module header to reflect the new state.
2. Remove docs/incremental-rebalance-roadmap.md. It described the
earlier "rewrite the in-place balance" plan that was abandoned in
favor of the Dolt streaming-chunker port. Fully superseded by
docs/dolt-streaming-chunker.md, which now documents Phase 4 as done
and lists the remaining optional follow-ups (recursive fast-forward,
multi-mutation batch advance, legacy balance removal).
Suite: 159 lib tests pass, 0 ignored, 0 failed.
* streaming_chunker: don't emit boundary on internal-level chunks with <2 entries
The Python CI hit a segfault (release) / u8 overflow panic (debug) in
basic_usage.py at the file-backed `insert` step. Root cause: Python's
`TreeConfig.__init__` has `pattern=0` as its default, and the example
also sets `min_chunk_size=1`. With pattern=0 the splitter's
`(hash & pattern) == pattern` check is true on every item, so a
boundary fires on every append. The chunker's `handle_boundary` then
recursively created a new parent chunker one level up, whose splitter
saw the just-emitted `(firstKey, hash)` entry, fired a boundary, and
cascaded - climbing the `u8` level counter until it overflowed.
This is Dolt's "constraint (3): internal nodes must contain at least
2 key-value pairs" rule from `chunker.go::append`, which we didn't
port. Adding it: at level > 0, defer boundary emission until the
in-progress chunk has at least 2 entries. Leaf-level chunks (level 0)
are still allowed to be single-entry so we can store a 1-key dataset.
The old in-place `Balanced::balance` didn't have this bug because it
emitted upward only when the chunker reported >1 chunks (its
`chunks.len() <= 1` early-return). The streaming chunker emits at
every boundary, so we need the explicit constraint here.
Regression test in `streaming_chunker::tests` reproduces the
degenerate config and asserts the level stays bounded.
Verified by re-running basic_usage.py against a debug + release build,
both succeed end-to-end. Rust suite: 160 lib tests pass, 0 failed.
* remove unused design doc
* clean up comments
* add new document
* fix diagram
* add more tests
* address copilot's comments1 parent 02e1cfa commit d48f129
13 files changed
Lines changed: 3345 additions & 40 deletions
File tree
- docs/theory
- src
- git/versioned_store
- proximity
- tests
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
13 | 13 | | |
14 | 14 | | |
15 | 15 | | |
16 | | - | |
17 | | - | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
18 | 19 | | |
19 | 20 | | |
20 | 21 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
95 | 95 | | |
96 | 96 | | |
97 | 97 | | |
| 98 | + | |
98 | 99 | | |
99 | 100 | | |
100 | 101 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
278 | 278 | | |
279 | 279 | | |
280 | 280 | | |
281 | | - | |
282 | | - | |
283 | | - | |
284 | | - | |
285 | | - | |
286 | | - | |
287 | | - | |
288 | | - | |
289 | | - | |
290 | | - | |
291 | | - | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
292 | 285 | | |
293 | 286 | | |
294 | 287 | | |
| |||
0 commit comments