-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
jobs: Implement job control for schedules. #52038
Conversation
f70c430
to
1e80ac6
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.
looks good to me but I am not expert on the grammar and parsing. You'd want to include Rapahel on the PR for this.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @dt and @spaskob)
@rohany can you pls take a look at this pr wrt to sql stuff? |
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.
I'm hesitant on this approach to shove the parsed query in an IN
clause in the delegate. It's going to give extremely cryptic error messages when something has gone wrong, and will be a hassle if we want to allow non-root users to control certain jobs.
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.
I hear you, @rohany . I've tried different approaches around this; alas, without much success.
Going to discuss this w/ you offline.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @dt)
I discussed with yevgeniy offline, and while I don't like the delegate approach i think that it's fine to do this for now. There is a good amount of work that can be done after this to move to utilizing the optimizer + planNode implementations to do this work and add features (like |
Add a `FOR SCHEDULES` clause to job control statements to enable control over jobs created by the scheduled jobs. ``` PAUSE JOBS FOR SCHEDULE 123 RESUME JOBS FOR SCHEDULES (SELECT schedule_id ....) CANCEL JOBS FOR SCHEDULE 321 ``` Release Notes (enterprise change): Add `FOR SCHEDULES` clause to the job control statements to enable management of the jobs created by schedules.
1e80ac6
to
e9ea87c
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.
TODO comments added to the delegate implementation. @rohany, ptal.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @dt)
bors r+ |
Build succeeded: |
Informs #51600
Add a
FOR SCHEDULES
clause to job control statementsto enable control over jobs created by the scheduled jobs.
Release Notes (enterprise change): Add
FOR SCHEDULES
clause tothe job control statements to enable management of the jobs created
by schedules.