Skip to content
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 deferred mode for alter parallelism #14826

Merged
merged 13 commits into from
Jan 30, 2024
Merged

Conversation

shanicky
Copy link
Contributor

@shanicky shanicky commented Jan 26, 2024

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

When using the DEFERRED parameter with commands like ALTER TABLE, it only modifies the parallelism field in the meta information of the streaming job and doesn't perform an actual scaling. The modification is left to be handled by the subsequent auto scale loop.

Checklist

  • I have written necessary rustdoc comments
  • I have added necessary unit tests and integration tests
  • All checks passed in ./risedev check (or alias, ./risedev c)

@BugenZhao
Copy link
Member

Could you please elaborate more on the motivation?

@shanicky shanicky force-pushed the peng/deferred-alter branch from 4821634 to 8575cc9 Compare January 29, 2024 07:55
@shanicky
Copy link
Contributor Author

Could you please elaborate more on the motivation?

Normally, an ALTER statement requires the scaling to succeed before updating the parallelism of the streaming job. However, in some cases, this might not be suitable. For instance, if there's a desire to forcefully change a custom job to automatic, or in emergency situations, such as when the current cluster's parallelism is insufficient for fixed requirements, you might need to forcefully reduce the degree of parallelism. 'deferred' is a parameter thought up by AI, signifying a delayed scaling.

Base automatically changed from peng/auto-scale-down to main January 29, 2024 08:05
@shanicky shanicky force-pushed the peng/deferred-alter branch from 8575cc9 to 1886acf Compare January 29, 2024 08:09
@shanicky shanicky force-pushed the peng/deferred-alter branch from 1886acf to c17766e Compare January 30, 2024 11:25
@shanicky shanicky marked this pull request as ready for review January 30, 2024 11:31
@shanicky shanicky requested a review from yezizp2012 January 30, 2024 11:31
.as_ref()
.unwrap()
.post_apply_reschedule(&HashMap::new(), &table_parallelism_assignment)
.await?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is acceptable to keep such a quick path to modify the streaming job parallelism, but it is better not to have any public documents. It should only be used in urgent situations when the user cluster load is too high and the response time is not timely. Because this can easily cause inconsistencies, after supporting trigger recovery manually we'd better call it here rather than rebooting the meta.

@shanicky shanicky enabled auto-merge January 30, 2024 13:57
@shanicky shanicky added this pull request to the merge queue Jan 30, 2024
Merged via the queue into main with commit e05015a Jan 30, 2024
35 checks passed
@shanicky shanicky deleted the peng/deferred-alter branch January 30, 2024 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants