-
Notifications
You must be signed in to change notification settings - Fork 587
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
feat: support per-fragment vnode count #18444
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @BugenZhao and the rest of your teammates on Graphite |
32d0692
to
afe9e79
Compare
3292eed
to
c4772c9
Compare
afe9e79
to
f168f35
Compare
c4772c9
to
166d631
Compare
f168f35
to
be13c79
Compare
166d631
to
31e2d77
Compare
be13c79
to
18e0165
Compare
a5afcdf
to
fce41b8
Compare
23435d7
to
01ee86d
Compare
fa288c5
to
e4ef75f
Compare
640e7d8
to
fe25c89
Compare
34bdd9b
to
8b31007
Compare
fe25c89
to
4ae30fd
Compare
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]>
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]>
Signed-off-by: Bugen Zhao <[email protected]>
Signed-off-by: Bugen Zhao <[email protected]>
Signed-off-by: Bugen Zhao <[email protected]>
0d6830b
to
db14e46
Compare
Signed-off-by: Bugen Zhao <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
.add_column( | ||
ColumnDef::new(Table::VnodeCount) | ||
.integer() | ||
.default(VNODE_COUNT), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.default(VNODE_COUNT), | |
.default(VNODE_COUNT) | |
.not_null(), |
Nits, better to keep consistent with model definition since the column type is not Option<i32>
.
This column always has a value during initialization and updates, so it doesn't matter whether set it not null or not. Feel free to change it or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Will address in following PRs.
.add_column( | ||
ColumnDef::new(Fragment::VnodeCount) | ||
.integer() | ||
.default(VNODE_COUNT), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto.
Signed-off-by: Bugen Zhao <[email protected]>
Signed-off-by: Bugen Zhao <[email protected]>
Signed-off-by: Bugen Zhao <[email protected]>
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
This is a progress towards #15900.
In previous PRs, we mainly focused on refactoring codes from different modules to avoid directly referencing the global constant of
VirtualNode::COUNT
, but infer it from the context or directly accept it as a parameter.In this PR, we introduce the
vnode_count
field forTable
andStreamFragmentGraph
, update the logic for its maintenance. Many TODOs of directly usingVirtualNode::COUNT
are fixed by reading the values fromFragment
orTableDesc
.Note that there's still no user-facing changes in this PR, but we are now quite close to that. However, there are still some unaddressed TODOs that may require changes in behavior. We leave this in future PRs.
Checklist
./risedev check
(or alias,./risedev c
)Documentation
Release note
If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.