-
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: introduce node label #19153
feat: introduce node label #19153
Conversation
101327d
to
4d22b04
Compare
5a7017a
to
d90fae6
Compare
d1f87cc
to
981e97b
Compare
981e97b
to
0a25900
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.
LGTM!
src/cmd_all/src/standalone.rs
Outdated
@@ -482,6 +482,7 @@ mod test { | |||
total_memory_bytes: 34359738368, | |||
reserved_memory_bytes: None, | |||
parallelism: 10, | |||
node_label: "default", |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Signed-off-by: Shanicky Chen <[email protected]>
… for consistency Signed-off-by: Shanicky Chen <[email protected]>
…odule config settings Signed-off-by: Shanicky Chen <[email protected]>
…eferences Signed-off-by: Shanicky Chen <[email protected]>
…alization Signed-off-by: Shanicky Chen <[email protected]>
Signed-off-by: Shanicky Chen <[email protected]>
…st initializations Signed-off-by: Shanicky Chen <[email protected]>
…AddWorkerNodeRequest Signed-off-by: Shanicky Chen <[email protected]>
Signed-off-by: Shanicky Chen <[email protected]>
…info function Signed-off-by: Shanicky Chen <[email protected]>
0a25900
to
b61ed85
Compare
…tion Signed-off-by: Shanicky Chen <[email protected]>
a218fb3
to
9fe686f
Compare
pub fn parallelism(&self) -> usize { | ||
self.parallelism as usize | ||
assert_eq!(self.r#type(), WorkerType::ComputeNode); | ||
self.property | ||
.as_ref() | ||
.expect("property should be exist") | ||
.parallelism as usize |
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.
This will panic in some places:
- diagnose in dashboard
show cluster
Can you examine the usages of worker.parallelism()
? (Or you may change list_all_nodes()
calls to list_cn()
)
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
This PR attempts to add support for node label
Currently, our label is designed as a single string to serve as an option, but the compute node has a default label that is "default"
Regarding Property, since there is currently no good way to define its meaning, this PR first attempts to unify the Property in WorkerNode with the Property in AddWorkerNode.
Checklist
./risedev check
(or alias,./risedev c
)