Replace keep-while cond reverse index with a prefix tree #298
Conversation
This file contains 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
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #298 +/- ##
==========================================
+ Coverage 89.34% 89.69% +0.34%
==========================================
Files 21 22 +1
Lines 3191 3259 +68
==========================================
+ Hits 2851 2923 +72
+ Misses 340 336 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Dialyzer looks to get stuck in recursion on the Ubuntu + Erlang/OTP 27 runner, I can reproduce locally |
dumbbell
requested changes
Sep 25, 2024
Refactoring `convert_state/3` this way allows us to write `convert_state1/3` so that it always acts on a single version upgrade.
the-mikedavis
force-pushed
the
md/keep-while-revidx-prefix-tree
branch
from
3426952 to
1c32d4c
Compare
the-mikedavis
commented
Sep 26, 2024
This type is otherwise completely private to the `khepri_tree` module and type. We'll take advantage of its private nature in the child commits to change its representation. This value was only exposed for a single test.
This will be used in the child commit to replace the keep-while conditions reverse index which currently uses a map of paths to watchers. The design is very similar to- but simpler than the existing trees in `khepri_tree` and `khepri_pattern_tree`.
the-mikedavis
force-pushed
the
md/keep-while-revidx-prefix-tree
branch
from
1c32d4c to
1e8b3b9
Compare
dumbbell
reviewed
Oct 1, 2024
When deleting tree nodes in the `khepri_tree` we lookup in the reverse index to find any conditions that are associated to paths which are prefixes of the deleted path. Prior to this commit we folded over the keep-while conditions reverse index - a map - and used `lists:prefix/2` to find prefixing paths. In a store with many nodes and tracking many keep-while conditions this can become very expensive while deleting many nodes at once. The parent commit introduced a prefix tree type which allows quick lookup, given a path, of any tree nodes associated with a path which is a prefix of the given path. We set a version 2 for khepri_machine which upgrades the reverse index use to this new type. Because the reverse index is private to the khepri_tree type I have avoided introducing versioning for the `khepri_tree` module. Instead the reverse index can either be a map - as in prior versions - or a prefix tree. We act on the reverse index using the appropriate functions for the type, which we detect at runtime.
the-mikedavis
force-pushed
the
md/keep-while-revidx-prefix-tree
branch
from
1e8b3b9 to
1d74ff8
Compare
dumbbell
approved these changes
Oct 1, 2024
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.
When handling a deletion,
khepri_tree:eval_keep_while_conditions/2folded over the reverse index of keep while conditions and evaluated any for which the removed path was a prefix. This meant folding over each non-leaf node in the tree. We can eliminate nodes very quickly though to limit the impact of this search by using a prefix tree instead.This change switches out the
keep_while_conds_revidxfield in#khepri_tree{}to a newkhepri_prefix_treetype when the Khepri cluster reaches the new effective machine version of 2.