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(storage): do not aware of keyspace in state table #3239

Merged
merged 6 commits into from
Jun 15, 2022

Conversation

BugenZhao
Copy link
Member

@BugenZhao BugenZhao commented Jun 15, 2022

Signed-off-by: Bugen Zhao i@bugenzhao.com

What's changed and what's your intention?

With the prefixed_range introduced in #3217, we can make the cell-based table & state table not aware of keyspace prefix with ease.

Checklist

  • I have written necessary docs and comments
  • I have added necessary unit tests and integration tests
  • All checks passed in ./risedev check (or alias, ./risedev c)

Refer to a related PR or issue link (optional)

Signed-off-by: Bugen Zhao <i@bugenzhao.com>
@BugenZhao BugenZhao marked this pull request as draft June 15, 2022 06:49
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
@BugenZhao BugenZhao marked this pull request as ready for review June 15, 2022 07:08
Copy link
Member

@xxchan xxchan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make Keyspace::prefixed_key private?

@codecov
Copy link

codecov bot commented Jun 15, 2022

Codecov Report

Merging #3239 (fd3f1de) into main (33c717c) will increase coverage by 0.03%.
The diff coverage is 78.94%.

@@            Coverage Diff             @@
##             main    #3239      +/-   ##
==========================================
+ Coverage   73.18%   73.22%   +0.03%     
==========================================
  Files         745      745              
  Lines      101845   101783      -62     
==========================================
- Hits        74540    74532       -8     
+ Misses      27305    27251      -54     
Flag Coverage Δ
rust 73.22% <78.94%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/storage/src/table/cell_based_table.rs 75.95% <33.33%> (+5.45%) ⬆️
src/storage/src/table/state_table.rs 93.70% <92.30%> (+5.11%) ⬆️
src/storage/src/keyspace.rs 85.86% <93.75%> (-3.83%) ⬇️
src/storage/hummock_sdk/src/key.rs 84.89% <100.00%> (+10.79%) ⬆️
src/common/src/types/ordered_float.rs 24.90% <0.00%> (+0.19%) ⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

Signed-off-by: Bugen Zhao <i@bugenzhao.com>
@wcy-fdu
Copy link
Contributor

wcy-fdu commented Jun 15, 2022

May conflict with #3238.

src/storage/src/keyspace.rs Outdated Show resolved Hide resolved
src/storage/src/keyspace.rs Outdated Show resolved Hide resolved
…le-prefix

Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
@BugenZhao BugenZhao requested a review from xxchan June 15, 2022 07:24
@BugenZhao BugenZhao enabled auto-merge (squash) June 15, 2022 07:24
Copy link
Contributor

@lmatz lmatz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙇‍♂️

Copy link
Member

@xxchan xxchan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

look much better!

@BugenZhao
Copy link
Member Author

make Keyspace::prefixed_key private?

We're still using it in write_batch.rs. Maybe we can make this a submodule of keyspace so that we can make it private.

@BugenZhao BugenZhao merged commit 7bbd84b into main Jun 15, 2022
@BugenZhao BugenZhao deleted the bz/refactor-state-table-prefix branch June 15, 2022 07:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

storage: make state table not aware of the keyspace prefix
5 participants