Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: follow-up to leaf updater uses binary search #565

Merged
merged 1 commit into from
Nov 22, 2024

Conversation

gabriele-0201
Copy link
Contributor

The following pr contains all the follow-ups to #529.

One specific thing I would like to ask is an opinion to the introduction of LeafNode::extract_key. It is a small abstraction that still requires some conceptual thoughts. The point is: the layout of the leaf node is mostly encapsulated within its module, and inside keep_up_to, I was extracting the key from the cell_pointers. However, I thought that a modification in the layout of the leaf node could silently break the leaf updater. That's why I added this helper function. In case a layout change happens, the entire leaf/node.rs code would be updated, including the way a key is extracted from the cell pointer.

Copy link
Contributor Author

gabriele-0201 commented Nov 18, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

@rphmeier
Copy link
Contributor

One specific thing I would like to ask is an opinion to the introduction of LeafNode::extract_key. It is a small abstraction that still requires some conceptual thoughts.

This seems reasonable. We should avoid silent breakage (and ideally have a test which prevents those kinds of regressions). Helper functions are usually good, just not worth hiding the format of the leaf from the updater. as we will eventually have greater optimizations by making use of it.

Base automatically changed from gm_leaf_fix_base_leaf_in_keep_up_to to master November 22, 2024 03:16
@gabriele-0201 gabriele-0201 force-pushed the gm_leaf_stage_binary_search_follow_up branch from c76d252 to bbb2bad Compare November 22, 2024 03:49
@gabriele-0201 gabriele-0201 merged commit b45c2d9 into master Nov 22, 2024
8 checks passed
@gabriele-0201 gabriele-0201 deleted the gm_leaf_stage_binary_search_follow_up branch November 22, 2024 04:06
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.

2 participants