feat: configurable vnode count with env var #18161
Conversation
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @BugenZhao and the rest of your teammates on |
| @@ -31,44 +34,82 @@ pub struct VirtualNode(VirtualNodeInner); | |||
|
|
|||
| /// The internal representation of a virtual node id. | |||
| type VirtualNodeInner = u16; | |||
| static_assertions::const_assert!(VirtualNodeInner::BITS >= VirtualNode::BITS as u32); | |||
| // static_assertions::const_assert!(VirtualNodeInner::BITS >= VirtualNode::BITS as u32); | |||
There was a problem hiding this comment.
| // static_assertions::const_assert!(VirtualNodeInner::BITS >= VirtualNode::BITS as u32); |
| pub fn count() -> usize { | ||
| // Cache the value to avoid repeated env lookups and parsing. | ||
| static COUNT: LazyLock<usize> = LazyLock::new(|| { | ||
| if let Ok(count) = std::env::var("RW_VNODE_COUNT") { |
There was a problem hiding this comment.
What if the RW_VNODE_COUNT is different after restart?
Considering the vnode is persisted in storage, this will certainly cause some problems, I think. If so, I would question whether it's a good idea to pass it via env var, perhaps making it a MV argument would be better? Or at least it should be a system parameter, and should always use the first-time initialized value, like state_store_url
There was a problem hiding this comment.
You are right.
I had some discussion about these approaches in the original issue. Basically this PR is a quick hack if we want to unblock some users to utilize more resources for their clusters. However, if we find this niche and not that urgent, I agree that we may not merge this PR but directly implement the polished ideas, like per-job configuration or global immutable system parameter.
|
Superseded by the works of fragment-local vnode count. |
Signed-off-by: Bugen Zhao i@bugenzhao.comI hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
See background in #15900.
Allow configuring vnode count by setting env var
RW_VNODE_COUNT.Ideally it should be specified as an immutable system parameter. However, such approach will involve a considerable amount of code changes. So for the initial PR, I simply switch to use the environment variable. This enables support for more parallelism in RW with minimal changes.
As the cost, specifying different
RW_VNODE_COUNTvalues for different worker nodes or different runs will lead to unexpected behavior, which can be dangerous.There are also other limitations on the valid range of vnode counts:
In future PRs, we may make it...
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.