refactor(meta): persist job-level max parallelism & check when ALTER .. SET PARALLELISM
#18740
Conversation
BugenZhao
force-pushed
the
bz/persist-max-parallelism
branch
from
8cfaf3f
to
bad6ee3
Compare
BugenZhao
force-pushed
the
bz/persist-max-parallelism
branch
from
bad6ee3
to
2587bb0
Compare
BugenZhao
force-pushed
the
bz/persist-max-parallelism
branch
from
2587bb0
to
612a489
Compare
BugenZhao
changed the title
ALTER .. SET PARALLELISM
| // TODO(var-vnode): get correct max parallelism from catalogs. | ||
| let max_parallelism = VirtualNode::COUNT_FOR_COMPAT; | ||
| let max_parallelism = self | ||
| .metadata_manager | ||
| .get_table_max_parallelism(table_id) | ||
| .await?; |
There was a problem hiding this comment.
This is what's addressed.
BugenZhao
force-pushed
the
bz/persist-max-parallelism
branch
from
ecf3a25
to
8a4481c
Compare
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>
BugenZhao
force-pushed
the
bz/persist-max-parallelism
branch
from
8a4481c
to
9736d32
Compare
| // The scheduler on the meta service will use this as a hint to decide the vnode count | ||
| // for each fragment. | ||
| // | ||
| // Note that the actual vnode count may be different from this value. | ||
| // For example, a no-shuffle exchange between current fragment graph and an existing | ||
| // upstream fragment graph requires two fragments to be in the same distribution, | ||
| // thus the same vnode count. | ||
| uint32 expected_vnode_count = 7; | ||
| uint32 max_parallelism = 7; |
There was a problem hiding this comment.
Personally, I'd like to use vnode_count for all internal occurrences, and only use "max_parallelism" for the user-facing part. Because we (RW developers) are familiar with vnode, so vnode_count sounds like the most straight-forward naming.
There was a problem hiding this comment.
My major concern is that, vnode_count of a streaming job may not be a physical concept at all. But it's true that prefixing with expected makes it doesn't sounds that confusing. Will change.
There was a problem hiding this comment.
Upon further consideration, I still think max_parallelism is more understandable because it aligns better with existing job attributes like parallelism (or assigned_parallelism). When the scope goes to fragment, it'll still be named as vnode_count.
src/meta/src/manager/metadata.rs
Outdated
| @@ -892,6 +892,24 @@ impl MetadataManager { | |||
| } | |||
| } | |||
|
|
|||
| pub async fn get_table_max_parallelism(&self, table_id: TableId) -> MetaResult<usize> { | |||
There was a problem hiding this comment.
nit.
| pub async fn get_table_max_parallelism(&self, table_id: TableId) -> MetaResult<usize> { | |
| pub async fn get_job_max_parallelism(&self, table_id: TableId) -> MetaResult<usize> { |
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
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.
Persist the max parallelism (vnode count) specified through session variable when a streaming job was created. Use that to check if a future parallelism change (with
ALTER .. SET PARALLELISM) is valid, ensuring the behavior is consistent with setting session variablestreaming_parallelismwhen the job was created.Note that in some cases when there are no-shuffle exchanges between subgraphs, the
vnode_countadopted by some fragment may not follow the job'smax_parallelismat all. The subtle difference will be addressed when generating the resizing (rescheduling) plans, but it does not affect whether a parallelism change issued by the user should be accepted or rejected, just like how we handle it when creating a new job.Added e2e tests to reflect the behavior.
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.