Skip to content

feat(durable-storage): support persisting node without its key value#954

Merged
vapourismo merged 1 commit intomainfrom
ole/push-wzvrzrvmuupp
Mar 30, 2026
Merged

feat(durable-storage): support persisting node without its key value#954
vapourismo merged 1 commit intomainfrom
ole/push-wzvrzrvmuupp

Conversation

@vapourismo
Copy link
Copy Markdown
Collaborator

@vapourismo vapourismo commented Mar 26, 2026

What

This PR changes the stored representation for MAVL nodes. The key's value from the node is no longer eagerly stored in the KV store.

That means we access these things using KeyValueStore::blob_get/KeyValueStore::blob_set:

  • key
  • balance factor
  • left tree hash
  • right tree hash

The key's value is optionally stored using KeyValueStore::set, and will always be loaded via KeyValueStore::get.

Why

We would rather not store the value twice in the KV store.

How

This change includes a sizeable refactor to ensure tests continue to work. This is necessary because not all commit operations are equal. In the presence of a Database, the commit operation of the MerkleLayer would not need to write the node key's value. However, when testing the MerkleLayer in isolation, we need to flush those node values as no other component would do it otherwise.

To make this work cleanly, I have added two traits Storable and Loadable which abstract over things that can be accessed in a KV store.

Let's first look at notable Loadable implementations:

  • Node<TreeId, Normal>: This instance loads the key, value, and balance factor. The left and right tree are loaded via Loadable for TreeId.
  • Tree<NodeId>: Loads the optional hash from the KV store and then delegates further loading of the inner node to Loadable for NodeId.
  • LazyNodeId/LazyTreeId: Accepts the requested hash and then does nothing. The value will be loaded lazily.
  • ArcNodeId/ArcTreeId: Eagerly loads the inner value using the corresponding Loadable for Node<...> or Loadable for Tree<...> implementation.

Notable implementations of Storable are:

  • Node<TreeId, Normal>: Always stores key, balance factor, and the hashes of the left and right tree. The left and right tree are optionally persisted. Same for the node's key value.
  • Tree<NodeId>: If the tree isn't empty, it stores the optional hash. Optionally, the nested node will be stored as well via Storable for NodeId.
  • LazyNodeId/LazyTreeId: When either of these contain a value (not blinded) then they delegate to the Storable implementation of that value.
  • ArcNodeId/ArcTreeId: The storing is delegated to the inner Node or Tree.

Manually Testing

make all

Tasks for the Author

  • Link all Linear issues related to this MR using magic words (e.g. part of, relates to, closes).
  • Eliminate dead code and other spurious artefacts introduced in your changes.
  • Document new public functions, methods and types.
  • Make sure the documentation for updated functions, methods, and types is correct.
  • Add tests for bugs that have been fixed.
  • Explain changes to regression test captures when applicable.
  • Write commit messages in agreement with our guidelines.
  • Self-review your changes to ensure they are high-quality.
  • Complete all of the above before assigning this MR to reviewers.

@vapourismo vapourismo force-pushed the ole/push-wzvrzrvmuupp branch 6 times, most recently from f44c0b8 to 128dfe0 Compare March 26, 2026 15:58
@vapourismo vapourismo marked this pull request as ready for review March 26, 2026 16:20
@vapourismo vapourismo enabled auto-merge March 26, 2026 16:20
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 26, 2026

Benchmark results for revision 7732c06:

Metric Duration TPS
Mean 1.518218084s 26.347
Worst 1.533626238s 26.082
Best 1.508798287s 26.511
Standard Deviation ±6.370241ms ±0.110
Full results
Run Transfers Duration TPS
1 40 1.520188551s 26.313
2 40 1.517214527s 26.364
3 40 1.520948982s 26.299
4 40 1.533626238s 26.082
5 40 1.514645065s 26.409
6 40 1.511735141s 26.460
7 40 1.513485671s 26.429
8 40 1.519278398s 26.328
9 40 1.520807815s 26.302
10 40 1.510478883s 26.482
11 40 1.515730128s 26.390
12 40 1.523516011s 26.255
13 40 1.516237807s 26.381
14 40 1.508798287s 26.511
15 40 1.530819322s 26.130
16 40 1.514866241s 26.405
17 40 1.515674472s 26.391
18 40 1.511181934s 26.469
19 40 1.526185737s 26.209
20 40 1.518942464s 26.334

Compare the results above with those for the default branch.

@vapourismo vapourismo force-pushed the ole/push-wzvrzrvmuupp branch from 128dfe0 to 6768542 Compare March 26, 2026 17:07
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 26, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.69%. Comparing base (6b8df3c) to head (4c31640).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #954      +/-   ##
==========================================
- Coverage   89.70%   89.69%   -0.02%     
==========================================
  Files         110      110              
  Lines       22136    22136              
  Branches    22136    22136              
==========================================
- Hits        19857    19854       -3     
- Misses       1884     1887       +3     
  Partials      395      395              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@vapourismo vapourismo force-pushed the ole/push-wzvrzrvmuupp branch from 6768542 to 488d4cd Compare March 26, 2026 21:17
@vapourismo vapourismo requested a review from NSant215 March 27, 2026 17:22
@vapourismo vapourismo force-pushed the ole/push-wzvrzrvmuupp branch 3 times, most recently from d077284 to 8aee2fb Compare March 30, 2026 09:19
@vapourismo vapourismo force-pushed the ole/push-wzvrzrvmuupp branch from 8aee2fb to 6eb9a7a Compare March 30, 2026 12:19
@vapourismo vapourismo force-pushed the ole/push-wzvrzrvmuupp branch from 6eb9a7a to 4c31640 Compare March 30, 2026 13:18
@vapourismo vapourismo added this pull request to the merge queue Mar 30, 2026
Merged via the queue into main with commit 4c07fba Mar 30, 2026
9 checks passed
@vapourismo vapourismo deleted the ole/push-wzvrzrvmuupp branch March 30, 2026 15:02
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.

4 participants