feat(storage): support vnode hint in storage table #3628
Conversation
Signed-off-by: Bugen Zhao <[email protected]>
Signed-off-by: Bugen Zhao <[email protected]>
Signed-off-by: Bugen Zhao <[email protected]>
Signed-off-by: Bugen Zhao <[email protected]>
Signed-off-by: Bugen Zhao <[email protected]>
Signed-off-by: Bugen Zhao <[email protected]>
Signed-off-by: Bugen Zhao <[email protected]>
| @@ -603,7 +569,7 @@ impl<S: StateStore, E: Encoding, const T: AccessType> StorageTableBase<S, E, T> | |||
| } else { | |||
| // Should use excluded next key for end bound. | |||
| // Otherwise keys starting with the bound is not included. | |||
| Excluded(next_key(&serialized_key)) | |||
| end_bound_of_prefix(&serialized_key) | |||
There was a problem hiding this comment.
Here we also fix that, if serialized_key is \xff, we should return Unbounded here instead of Excluded(b""). cc @xxchan
| pub(super) async fn batch_iter_with_encoded_key_range<R, B>( | ||
| /// Iterates on the table with the given prefix of the pk in `pk_prefix` and the range bounds of | ||
| /// the next primary key column in `next_col_bounds`. | ||
| // TODO: support multiple datums or `Row` for `next_col_bounds`. |
There was a problem hiding this comment.
Should we support multiple datums in next_col_bounds here? So that all cases can reuse this function? 🤣 cc @xxchan
There was a problem hiding this comment.
I think other cases are simple. Reusing this function does no benefit to them 🥸
There was a problem hiding this comment.
But you already removed iter_with_pk_prefix... 🤔
There was a problem hiding this comment.
So when will next_col_bounds for multiple columns needed?
There was a problem hiding this comment.
If next_col_bounds can be multiple columns, then all of the scan patterns can be formalized by this method, including arbitrary pk bound scan.
Codecov Report
@@ Coverage Diff @@
## main #3628 +/- ##
==========================================
- Coverage 74.30% 74.30% -0.01%
==========================================
Files 788 788
Lines 111653 111369 -284
==========================================
- Hits 82967 82750 -217
+ Misses 28686 28619 -67
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
Signed-off-by: Bugen Zhao <[email protected]>
Signed-off-by: Bugen Zhao <[email protected]>
* use row for cell based table iter interfaces Signed-off-by: Bugen Zhao <[email protected]> * compute vnode hint Signed-off-by: Bugen Zhao <[email protected]> * refine docs Signed-off-by: Bugen Zhao <[email protected]> * remove tests for state table pk bounds iter Signed-off-by: Bugen Zhao <[email protected]> * fix typo Signed-off-by: Bugen Zhao <[email protected]> * Update src/storage/src/table/storage_table.rs * trigger ci Signed-off-by: Bugen Zhao <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
I hereby agree to the terms of the Singularity Data, Inc. Contributor License Agreement.
What's changed and what's your intention?
This PR adds support for vnode hint in the storage table. We'll always try computing vnode based on the pk prefix before iteration to avoid scanning all vnodes.
Checklist
./risedev check(or alias,./risedev c)Refer to a related PR or issue link (optional)