cli/doctor: address issues in the debug doctor command #104641
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fqazi
requested review from
srosenberg and
renatolabs
and removed request for
a team
|
This change is |
chengxiong-ruan
approved these changes
Jun 12, 2023
| if err != nil { | ||
| return errors.Wrapf(err, "failed to parse descriptor id %s", fields[0]) | ||
| } | ||
| if checkIfFileExists(zipDirPath, "system.descriptor.txt") { |
There was a problem hiding this comment.
hmm...this is sad. I wonder what made us missing the change of this part at the first place. Maybe the test should have been more end-to-end. It should have generated the debug zip from a test db instead of having the fixtures in the format it was expecting. But this is low priority and can be future refactoring.
There was a problem hiding this comment.
Yes, let good point let me add this in a future PR using roachtest
Previously, the CLI debug doctor command was updated to be able to read the crdb_internal.system_jobs. This table doesn't exist on older releases, so the doctor command could only be run against the latest releases. To address, this patch adds support for running doctor against different releases. Epic: none Fixes: cockroachdb#104352 Releases note (bug fix): Fixed a bug that prevented debug doctor from running against older versions of Cockroach.
Previously, when parsing the namespace entries we did not handle quoted table names properly. This patch addresses, this by unquoting entries properly. Fixes: cockroachdb#104347 Release note: None
Previously, the debug doctor command only worked on the older text format from debug zips. A recent change on master started generating JSON based formats, which are incompatible. This patch adds support for reading either the JSON or text based formats depending on the directory. Epic: none Release note: None
|
@chengxiong-ruan TFTR! |
|
bors r+ |
rafiss
reviewed
Jun 13, 2023
| @@ -313,6 +314,19 @@ FROM system.namespace` | |||
| stmt = ` | |||
| SELECT id, status, payload, progress FROM "".crdb_internal.system_jobs | |||
| ` | |||
| checkSystemJobs := `SELECT count(*) FROM "".crdb_internal.system_jobs LIMIT 1` | |||
| _, err = sqlConn.QueryRow(ctx, checkSystemJobs) | |||
| // On versions before 23.1, we have a new table called crdb_internal.system_jobs, | |||
There was a problem hiding this comment.
is this supposed to say "after 23.1"
|
Build succeeded: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, the debug doctor had the following issues on master: