-
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: clean running jobs with no mutations left #41172
Conversation
2fed7a7
to
359bdeb
Compare
pkg/jobs/registry.go
Outdated
return false, nil | ||
} | ||
for _, id := range payload.DescriptorIDs { | ||
txn := r.db.NewTxn(ctx, "get_table_and_db_descrs") |
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 usually use r.db.Txn()
and pass a closure, which also means you don't need to do your own retries.
c099cd2
to
20a766c
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 @dt)
pkg/jobs/registry.go, line 375 at r1 (raw file):
Previously, dt (David Taylor) wrote…
we usually use
r.db.Txn()
and pass a closure, which also means you don't need to do your own retries.
Done.
pkg/jobs/registry.go
Outdated
if err != nil { | ||
return err | ||
} | ||
if len(td.GetMutations()) != 0 { |
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.
can just assign the bool to the if condition instead of using an if, right?
Also, this seems like the name vs definition is inverted, no?
Failures can leave jobs in orphaned state where no work needs to be done but they are left in `system.jobs` table. We detect and delete such jobs if they are too old. Touches cockroachdb#40563. Release justification: This commit is safe for 19.2 because this is a fairly low risk change since it only affects jobs that have no mutations left and that are >24h old. Release note: None.
bors r+ |
41172: jobs: clean running jobs with no mutations left r=spaskob a=spaskob Failures can leave jobs in orphaned state where no work needs to be done but they are left in `system.jobs` table. We detect and delete such jobs if they are too old. Touches #40563. Release justification: This commit is safe for 19.2 because this is a fairly low risk change since it only affects jobs that have no mutations left and that are >24h old. Release note: None. Co-authored-by: spaskob <spas@cockroachlabs.com>
Build succeeded |
Failures can leave jobs in orphaned state where no work needs
to be done but they are left in
system.jobs
table. We detectand delete such jobs if they are too old.
Touches #40563.
Release justification: This commit is safe for 19.2 because
this is a fairly low risk change since it only affects jobs that
have no mutations left and that are >24h old.
Release note: None.