jobs: missing descIDs in job payload should not prevent jobs from GC #45353
Conversation
|
This change is |
pkg/jobs/registry_test.go
Outdated
| DescriptorIDs: []sqlbase.ID{descriptorID}, | ||
| DescriptorIDs: []sqlbase.ID{ | ||
| descriptorID, | ||
| sqlbase.ID(0), // invalid id to test handling of missing descriptors. |
There was a problem hiding this comment.
I think this can be replaced with sqlbase.InvalidID
pkg/jobs/registry.go
Outdated
| if err == sqlbase.ErrDescriptorNotFound { | ||
| // Treat missing table descriptors as no longer relevant for the | ||
| // job payload. See | ||
| // https://github.com/cockroachlabs/support/issues/365. |
There was a problem hiding this comment.
May be beneficial to make a public issue for this and link that.
pkg/jobs/registry.go
Outdated
| @@ -476,6 +476,12 @@ func (r *Registry) isOrphaned(ctx context.Context, payload *jobspb.Payload) (boo | |||
| pendingMutations = hasAnyMutations || hasDropJob | |||
| return nil | |||
| }); err != nil { | |||
| if err == sqlbase.ErrDescriptorNotFound { | |||
There was a problem hiding this comment.
errors.Is(err, sqlbase.ErrDescriptorNotFound) may be more resilient in case the error gets wrapped in the future.
pkg/jobs/registry.go
Outdated
| @@ -476,6 +476,12 @@ func (r *Registry) isOrphaned(ctx context.Context, payload *jobspb.Payload) (boo | |||
| pendingMutations = hasAnyMutations || hasDropJob | |||
| return nil | |||
| }); err != nil { | |||
| if err == sqlbase.ErrDescriptorNotFound { | |||
There was a problem hiding this comment.
errors.Is(err, sqlbase.ErrDescriptorNotFound) may be more resilient in case the error gets wrapped in the future.
spaskob
force-pushed
the
job-gc
branch
2 times, most recently
from
71ed912
to
b362c5b
Compare
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @pbardea)
pkg/jobs/registry.go, line 479 at r1 (raw file):
Previously, pbardea (Paul Bardea) wrote…
errors.Is(err, sqlbase.ErrDescriptorNotFound) may be more resilient in case the error gets wrapped in the future.
Done.
pkg/jobs/registry.go, line 482 at r1 (raw file):
Previously, pbardea (Paul Bardea) wrote…
May be beneficial to make a public issue for this and link that.
Done.
pkg/jobs/registry_test.go, line 270 at r1 (raw file):
Previously, pbardea (Paul Bardea) wrote…
I think this can be replaced with
sqlbase.InvalidID
Done.
…ng GCed Touches https://github.com/cockroachlabs/support/issues/365. Touches cockroachdb#45399. Release note (bug fix): When considering if a job should be orphaned we use to take the conservative approach when a descriptor ID pointing to non-existent descriptor was found. This causes jobs to hang forever and be garbage collected. We now disregard these IDs when considering if a job has still work to do.
|
bors r+ |
Build succeeded |
Touches https://github.com/cockroachlabs/support/issues/365.
Release note (bug fix): When considering if a job should be orphaned we
use to take the conservative approach when a descriptor ID pointing to
non-existent descriptor was found. This causes jobs to hang forever and
be garbage collected. We now disregard these IDs when considering if a
job has still work to do.