-
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
release-23.2: cli/doctor, doctor: use right jobs table, skip dropped descs #124365
Conversation
Thanks for opening a backport. Please check the backport criteria before merging:
If your backport adds new functionality, please ensure that the following additional criteria are satisfied:
Also, please add a brief release justification to the body of your PR to justify this |
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 @annrpom, @nameisbhaskar, and @vidit-bhat)
pkg/sql/doctor/doctor_test.go
line 200 at r2 (raw file):
}, valid: true, expected: "Examining 1 descriptors and 0 namespace entries...\n",
nit: just noticed this. why are we removing the expected error check?
pkg/sql/doctor/doctor_test.go
line 419 at r2 (raw file):
}, valid: true, expected: "Examining 2 descriptors and 1 namespace entries...\n",
nit: just noticed this. why are we removing the expected error check?
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.
resolving nits since they were addressed #124302 (comment)
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @nameisbhaskar and @vidit-bhat)
081b471
to
91664e0
Compare
In cockroachdb#97762, we started writing a job's payload (and progress) information to the `system.jobs_info` table. As a result, we had to change the parts of our code that relied on the `system.jobs` table to use `crdb_internal.system_jobs` instead (since that table would inaccurately report that some `payload`s were `NULL`). This change did not occur for the in-memory representation of the jobs table created by debug doctor -- which can result in missing job false-positives. This patch updates debug doctor's representation of the jobs table by referring to `crdb_internal.system_jobs` instead. Epic: none Fixes: cockroachdb#122675 Release note: None
In some cases, dropped descriptors appear in our `system.descriptors` table with dangling job mutations without an associated job. This patch teaches debug doctor examine to skip validation on such dropped descriptors. Epic: none Fixes: cockroachdb#123477 Fixes: cockroachdb#122956 Release note: none
91664e0
to
4c50a21
Compare
Backport 2/2 commits from #123298.
/cc @cockroachdb/release
Release justification: non-production change to debugging tool
cli/doctor: doctor should read from the right jobs table
In #97762, we started writing a job's payload (and progress)
information to the
system.jobs_info
table. As a result, wehad to change the parts of our code that relied on the
system.jobs
table to use
crdb_internal.system_jobs
instead (since that tablewould inaccurately report that some
payload
s wereNULL
).This change did not occur for the in-memory representation of the jobs
table created by debug doctor -- which can result in missing job
false-positives. This patch updates debug doctor's representation of
the jobs table by referring to
crdb_internal.system_jobs
instead.Epic: none
Fixes: #122675
Release note: None
doctor: skip validation for dropped descriptors
In some cases, dropped descriptors appear in our
system.descriptors
table with dangling job mutations without an associated job.
This patch teaches debug doctor examine to skip validation
on such dropped descriptors.
Epic: none
Fixes: #123477
Fixes: #122956
Release note: none