-
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
bulkio: Fix transaction semantics in job scheduler. #53049
Conversation
@@ -252,8 +254,20 @@ func (s *jobScheduler) executeSchedules( | |||
continue | |||
} | |||
|
|||
sp, err := txn.CreateSavepoint(ctx) |
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.
@knz or @nvanbenschoten can you just double-check we're using the Savepoint API correctly?
4f77599
to
3192e5c
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.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @nvanbenschoten)
pkg/jobs/job_scheduler.go, line 257 at r1 (raw file):
Previously, dt (David Taylor) wrote…
@knz or @nvanbenschoten can you just double-check we're using the Savepoint API correctly?
looks good
c051cdd
to
73f7d62
Compare
bors r+ |
Fix show jobs delegation code to correclty handle `FOR SCHEDUL(s)` clause. Release Notes: None
Use transaction when querying for the schedules to run. In addition, ensure that a single bad schedule does not cause all of the previous work to be wasted by using transaction savepoints. Release Notes: None
73f7d62
to
aab9608
Compare
Canceled. |
bors r+ |
Build succeeded: |
Fixes #53033
Fixes #52959
Use transaction when querying for the schedules to run.
In addition, ensure that a single bad schedule does not cause
all of the previous work to be wasted by using transaction savepoints.
Release Notes: None