-
Notifications
You must be signed in to change notification settings - Fork 590
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: user-facing part of variable vnode count #18515
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @BugenZhao and the rest of your teammates on Graphite |
e4ef75f
to
640e7d8
Compare
968ee5d
to
dd4b4f7
Compare
640e7d8
to
fe25c89
Compare
dd4b4f7
to
8b0a5df
Compare
4ae30fd
to
9cdeee0
Compare
0802213
to
61f8b0d
Compare
17dbbeb
to
3d4949f
Compare
632ae87
to
f32cb85
Compare
3d4949f
to
4f4f9ce
Compare
f32cb85
to
669a70e
Compare
4f4f9ce
to
c20dae1
Compare
669a70e
to
43f4a88
Compare
c20dae1
to
787e661
Compare
43f4a88
to
bf7205a
Compare
787e661
to
ec5bca3
Compare
bf7205a
to
4a20c1c
Compare
ec5bca3
to
8c36626
Compare
4a20c1c
to
f39bc7d
Compare
3c19e8b
to
cf797a8
Compare
f531007
to
8414be3
Compare
e29b738
to
4e93810
Compare
This reverts commit 683746c.
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]>
4e93810
to
a3a5472
Compare
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.
Generally LGTM!
// TODO(var-vnode): after `ServingVnodeMapping::upsert` is made vnode-count-aware, | ||
// we may return 1 for singleton. | ||
/// | ||
/// For backwards compatibility, [`VirtualNode::COUNT_FOR_COMPAT`] is used for singleton. |
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.
I'm wondering the initial reason that we used 256 as Singleton's vnode_count
here... What if we change it to 1? i.e. Which part of code rely on Singleton's vnode count being 256?
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.
I don't remember exactly, but I previously encountered some problems and found that using 256 for the singleton simplified things.
However, it's true that using 1
is much clearer and more consistent. Given that this PR has passed the tests, I'll make another attempt in future PRs.
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.
Significant changes in this PR:
streaming_max_parallelism
to decide the max parallelism used in future streaming jobs. Note that the default value of the session variable can be changed globally withALTER SYSTEM
.rw_vnode()
, used internally for 2-phase aggregation.VirtualNode::COUNT
toVirtualNode::COUNT_FOR_COMPAT
, indicating that it should not be used in most of the code. Also update the Java binding function.The variable vnode count feature is now generally available after this PR, with just a few individual TODOs that still need to be fixed.
Checklist
./risedev check
(or alias,./risedev c
)Documentation
Release note
Introduce a new session config of
streaming_max_parallelism
. Update the doc ofstreaming_parallelism
. Please refer to the rustdoc for documentation.