-
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(frontend): support alter source pause/resume #19636
Conversation
src/sqlparser/src/parser.rs
Outdated
AlterSourceOperation::SetSourceRateLimit { | ||
rate_limit: SOURCE_RATE_LIMIT_PAUSED, | ||
} | ||
} else if self.parse_keyword(Keyword::UNPAUSE) { |
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.
We should use ALTER SOURCE RESUME
instead of "Unpause"
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.
yep fixed it everywhere
df3418c
to
4203597
Compare
Hi @lyang24 , thanks for your contribution.
I have a question about the semantics here, please consider the following use case.
Setting rate_limit here may abuse the pause/resume semantic. cc @fuyufjh, what do you think of the proposed syntax? |
We can ban |
The challenge is - The sql parser doesn't know the current rate limit nor can it distinguish from pause or set source_rate_limit to 0. To achieve this we probably need to modify |
I am up for this idea. |
42b594f
to
15cbc6b
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.
Rest LGTM, thanks for your work!
15cbc6b
to
9a99b62
Compare
6f2d0c3
|| (prev_limit != 0 && rate_limit == SOURCE_RATE_LIMIT_RESUMED) | ||
{ | ||
return Err(InvalidInputSyntax( | ||
"PAUSE or RESUME is invalid when the stream has pre configured ratelimit." |
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.
"PAUSE or RESUME is invalid when the stream has pre configured ratelimit. Just use SET source_rate_limit = 0
"
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
Please do not leave this empty!
Please explain IN DETAIL what the changes are in this PR and why they are needed:
Support alter source pause or resume syntax sugar by setting the rate limit to 0 on pause or to cancel rate limit on resume. Please not PAUSE/ RESUME only works when the source haven't configure the rate limit.
feat: alter source pause/resume #19181
Checklist
./risedev check
(or alias,./risedev c
)Documentation
Release note
Public Preview: Do note that the structure and format of ALTER SOURCE PAUSE/ RESUME is considered unstable, and may change overtime.
After this PR, two new alter source syntax will be supported.
ALTER SOURCE <name> PAUSE;
to pause processing stream from source.ALTER SOURCE <name> RESUME;
to continue processing stream from source.