Skip to content
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.

Sign up for GitHub

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

Jump to bottom

cli/doctor: address issues in the debug doctor command #104641

Merged
merged 3 commits into from
Jun 13, 2023
Merged

cli/doctor: address issues in the debug doctor command #104641

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4975ede
cli/doctor: make backwards compatible with older jobs tables
fqazi Jun 6, 2023
0933849
cli/doctor: handle quoted namespace entries
fqazi Jun 6, 2023
4ecbeb8
cli/doctor: parse new json froamt debug zips
fqazi Jun 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
303 changes: 227 additions & 76 deletions pkg/cli/doctor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"context"
"database/sql/driver"
hx "encoding/hex"
"encoding/json"
"fmt"
"io"
"os"
Expand Down Expand Up @@ -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,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this supposed to say "after 23.1"

// if its not available fall back to system.jobs
if pgErr := (*pgconn.PgError)(nil); errors.As(err, &pgErr) {
if pgcode.MakeCode(pgErr.Code) == pgcode.UndefinedTable {
stmt = `
SELECT id, status, payload, progress FROM system.jobs
`
}
} else if err != nil {
return nil, nil, nil, err
}
jobsTable = make(doctor.JobsTable, 0)

if err := selectRowsMap(sqlConn, stmt, make([]driver.Value, 4), func(vals []driver.Value) error {
Expand All @@ -325,12 +339,10 @@ SELECT id, status, payload, progress FROM "".crdb_internal.system_jobs
}
md.Progress = &jobspb.Progress{}
// Progress is a nullable column, so have to check for nil here.
progressBytes, ok := vals[3].([]byte)
if !ok {
return errors.Errorf("unexpected NULL progress on job row: %v", md)
}
if err := protoutil.Unmarshal(progressBytes, md.Progress); err != nil {
return err
if progressBytes, ok := vals[3].([]byte); ok {
if err := protoutil.Unmarshal(progressBytes, md.Progress); err != nil {
return err
}
}
jobsTable = append(jobsTable, md)
return nil
Expand All @@ -354,23 +366,50 @@ func fromZipDir(
_ = builtins.AllBuiltinNames()

descTable = make(doctor.DescriptorTable, 0)
if err := slurp(zipDirPath, "system.descriptor.txt", func(row string) error {
fields := strings.Fields(row)
last := len(fields) - 1
i, err := strconv.Atoi(fields[0])
if err != nil {
return errors.Wrapf(err, "failed to parse descriptor id %s", fields[0])
}
if checkIfFileExists(zipDirPath, "system.descriptor.txt") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, let good point let me add this in a future PR using roachtest

if err := slurp(zipDirPath, "system.descriptor.txt", func(row string) error {
fields := strings.Fields(row)
last := len(fields) - 1
i, err := strconv.Atoi(fields[0])
if err != nil {
return errors.Wrapf(err, "failed to parse descriptor id %s", fields[0])
}

descBytes, err := hx.DecodeString(fields[last])
if err != nil {
return errors.Wrapf(err, "failed to decode hex descriptor %d", i)
descBytes, err := hx.DecodeString(fields[last])
if err != nil {
return errors.Wrapf(err, "failed to decode hex descriptor %d", i)
}
ts := hlc.Timestamp{WallTime: timeutil.Now().UnixNano()}
descTable = append(descTable, doctor.DescriptorTableRow{ID: int64(i), DescBytes: descBytes, ModTime: ts})
return nil
}); err != nil {
return nil, nil, nil, err
}
} else {
descTableJSON := make([]struct {
ID string `json:"id"`
Descriptor string `json:"hex_descriptor"`
}, 0)
if err := parseJSONFile(zipDirPath, "system.descriptor.json", &descTableJSON); err != nil {
return nil, nil, nil, errors.Wrapf(err, "failed to parse system.descriptor.json")
}
ts := hlc.Timestamp{WallTime: timeutil.Now().UnixNano()}
descTable = append(descTable, doctor.DescriptorTableRow{ID: int64(i), DescBytes: descBytes, ModTime: ts})
return nil
}); err != nil {
return nil, nil, nil, err
for _, desc := range descTableJSON {
var err error
row := doctor.DescriptorTableRow{
ModTime: ts,
}
row.ID, err = strconv.ParseInt(desc.ID, 10, 64)
if err != nil {
return nil, nil, nil, errors.Wrapf(err, "failed to decode hex descriptor: (%s)", desc.ID)
}

row.DescBytes = make([]byte, hx.DecodedLen(len(desc.Descriptor)))
if _, err := hx.Decode(row.DescBytes, []byte(desc.Descriptor)); err != nil {
return nil, nil, nil, errors.Wrapf(err, "failed to decode hex descriptor: (%s)", desc.Descriptor)
}
descTable = append(descTable, row)
}
}

// Handle old debug zips where the namespace table dump is from namespace2.
Expand All @@ -382,80 +421,192 @@ func fromZipDir(
}
namespaceFileName = "system.namespace.txt"
}

namespaceTable = make(doctor.NamespaceTable, 0)
if err := slurp(zipDirPath, namespaceFileName, func(row string) error {
fields := strings.Fields(row)
parID, err := strconv.Atoi(fields[0])
if err != nil {
return errors.Wrapf(err, "failed to parse parent id %s", fields[0])
if checkIfFileExists(zipDirPath, namespaceFileName) {
if err := slurp(zipDirPath, namespaceFileName, func(row string) error {
fields := strings.Fields(row)
parID, err := strconv.Atoi(fields[0])
if err != nil {
return errors.Wrapf(err, "failed to parse parent id %s", fields[0])
}
parSchemaID, err := strconv.Atoi(fields[1])
if err != nil {
return errors.Wrapf(err, "failed to parse parent schema id %s", fields[1])
}
id, err := strconv.Atoi(fields[3])
if err != nil {
if fields[3] == "NULL" {
id = int(descpb.InvalidID)
} else {
return errors.Wrapf(err, "failed to parse id %s", fields[3])
}
}
// Attempt to unquote any strings, if they aren't quoted,
// we will use the original raw string.
unquotedName := fields[2]
if updatedName := strings.TrimSuffix(strings.TrimPrefix(unquotedName, "\""), "\""); updatedName != unquotedName {
unquotedName = strings.Replace(updatedName, "\"\"", "\"", -1)
}
namespaceTable = append(namespaceTable, doctor.NamespaceTableRow{
NameInfo: descpb.NameInfo{
ParentID: descpb.ID(parID), ParentSchemaID: descpb.ID(parSchemaID), Name: unquotedName,
},
ID: int64(id),
})
return nil
}); err != nil {
return nil, nil, nil, err
}
parSchemaID, err := strconv.Atoi(fields[1])
if err != nil {
return errors.Wrapf(err, "failed to parse parent schema id %s", fields[1])
} else {
namespaceTableJSON := make([]struct {
ID string `json:"id"`
ParentID string `json:"parentID"`
ParentSchemaID string `json:"parentSchemaID"`
Name string `json:"name"`
}, 0)
if err := parseJSONFile(zipDirPath, "system.namespace.json", &namespaceTableJSON); err != nil {
return nil, nil, nil, errors.Wrapf(err, "failed to parse system.descriptor.json")
}
id, err := strconv.Atoi(fields[3])
if err != nil {
if fields[3] == "NULL" {
id = int(descpb.InvalidID)
} else {
return errors.Wrapf(err, "failed to parse id %s", fields[3])
for _, namespace := range namespaceTableJSON {
row := doctor.NamespaceTableRow{
NameInfo: descpb.NameInfo{
Name: namespace.Name,
},
}
}

namespaceTable = append(namespaceTable, doctor.NamespaceTableRow{
NameInfo: descpb.NameInfo{
ParentID: descpb.ID(parID), ParentSchemaID: descpb.ID(parSchemaID), Name: fields[2],
},
ID: int64(id),
})
return nil
}); err != nil {
return nil, nil, nil, err
id, err := strconv.ParseInt(namespace.ParentID, 10, 64)
if len(namespace.ParentID) > 0 && err != nil {
return nil, nil, nil, errors.Wrapf(err, "failed to decode namespace ParentID %v", namespace)
}
row.ParentID = descpb.ID(id)
id, err = strconv.ParseInt(namespace.ParentSchemaID, 10, 64)
if len(namespace.ParentSchemaID) > 0 && err != nil {
return nil, nil, nil, errors.Wrapf(err, "failed to decode namespace ParentSchemaID %v", namespace)
}
row.ParentSchemaID = descpb.ID(id)
id, err = strconv.ParseInt(namespace.ID, 10, 64)
if len(namespace.ID) > 0 && err != nil {
return nil, nil, nil, errors.Wrapf(err, "failed to decode namespace ID %v", namespace)
}
row.ID = id
namespaceTable = append(namespaceTable,
row)
}
}

jobsTable = make(doctor.JobsTable, 0)
if err := slurp(zipDirPath, "system.jobs.txt", func(row string) error {
fields := strings.Fields(row)
md := jobs.JobMetadata{}
md.Status = jobs.Status(fields[1])
if checkIfFileExists(zipDirPath, "system.jobs.txt") {
if err := slurp(zipDirPath, "system.jobs.txt", func(row string) error {
fields := strings.Fields(row)
md := jobs.JobMetadata{}
md.Status = jobs.Status(fields[1])

id, err := strconv.Atoi(fields[0])
if err != nil {
return errors.Wrapf(err, "failed to parse job id %s", fields[0])
}
md.ID = jobspb.JobID(id)
id, err := strconv.Atoi(fields[0])
if err != nil {
return errors.Wrapf(err, "failed to parse job id %s", fields[0])
}
md.ID = jobspb.JobID(id)

last := len(fields) - 1
payloadBytes, err := hx.DecodeString(fields[last-1])
if err != nil {
// TODO(postamar): remove this check once payload redaction is improved
if fields[last-1] == "NULL" {
return nil
last := len(fields) - 1
payloadBytes, err := hx.DecodeString(fields[last-1])
if err != nil {
// TODO(postamar): remove this check once payload redaction is improved
if fields[last-1] == "NULL" {
return nil
}
return errors.Wrapf(err, "job %d: failed to decode hex payload", id)
}
md.Payload = &jobspb.Payload{}
if err := protoutil.Unmarshal(payloadBytes, md.Payload); err != nil {
return errors.Wrap(err, "failed unmarshalling job payload")
}

progressBytes, err := hx.DecodeString(fields[last])
if err != nil {
return errors.Wrapf(err, "job %d: failed to decode hex progress", id)
}
md.Progress = &jobspb.Progress{}
if err := protoutil.Unmarshal(progressBytes, md.Progress); err != nil {
return errors.Wrap(err, "failed unmarshalling job progress")
}
return errors.Wrapf(err, "job %d: failed to decode hex payload", id)

jobsTable = append(jobsTable, md)
return nil
}); err != nil {
return nil, nil, nil, err
}
md.Payload = &jobspb.Payload{}
if err := protoutil.Unmarshal(payloadBytes, md.Payload); err != nil {
return errors.Wrap(err, "failed unmarshalling job payload")
} else {
jobsTableJSON := make([]struct {
ID string `json:"id"`
Status string `json:"status"`
Payload string `json:"hex_payload"`
Progress string `json:"hex_progress"`
}, 0)
if err := parseJSONFile(zipDirPath, "system.jobs.json", &jobsTableJSON); err != nil {
return nil, nil, nil, errors.Wrapf(err, "failed to parse system.descriptor.json")
}
for _, job := range jobsTableJSON {
row := jobs.JobMetadata{
Status: jobs.Status(job.Status),
}
id, err := strconv.ParseInt(job.ID, 10, 64)
if len(job.ID) > 0 && err != nil {
return nil, nil, nil, errors.Wrapf(err, "failed to decode job ID %v", job)
}
row.ID = jobspb.JobID(id)
// Skip NULL job payloads.
if job.Payload == "NULL" {
continue
}
payloadBytes, err := hx.DecodeString(job.Payload)
if err != nil {
return nil, nil, nil, errors.Wrapf(err, "job %d: failed to decode hex payload", id)
}
row.Payload = &jobspb.Payload{}
if err := protoutil.Unmarshal(payloadBytes, row.Payload); err != nil {
return nil, nil, nil, errors.Wrap(err, "failed unmarshalling job payload")
}

progressBytes, err := hx.DecodeString(fields[last])
if err != nil {
return errors.Wrapf(err, "job %d: failed to decode hex progress", id)
}
md.Progress = &jobspb.Progress{}
if err := protoutil.Unmarshal(progressBytes, md.Progress); err != nil {
return errors.Wrap(err, "failed unmarshalling job progress")
progressBytes, err := hx.DecodeString(job.Progress)
if err != nil && job.Progress != "NULL" {
return nil, nil, nil, errors.Wrapf(err, "job %d: failed to decode hex progress", id)
}
row.Progress = &jobspb.Progress{}
if err := protoutil.Unmarshal(progressBytes, row.Progress); err != nil {
return nil, nil, nil, errors.Wrap(err, "failed unmarshalling job progress")
}
jobsTable = append(jobsTable,
row)
}
}
return descTable, namespaceTable, jobsTable, nil
}

jobsTable = append(jobsTable, md)
return nil
}); err != nil {
return nil, nil, nil, err
func checkIfFileExists(zipDirPath string, fileName string) bool {
_, err := os.Stat(path.Join(zipDirPath, fileName))
return err == nil
}

// readJsonArray reads the given file as an array of JSON structs.
func parseJSONFile(zipDirPath string, fileName string, result interface{}) error {
filePath := path.Join(zipDirPath, fileName)
// Check for existence of companion .err.txt file.
_, err := os.Stat(filePath + ".err.txt")
if err == nil {
// A .err.txt file exists.
fmt.Printf("WARNING: errors occurred during the production of %s, contents may be missing or incomplete.\n", fileName)
} else if !errors.Is(err, os.ErrNotExist) {
// Handle unexpected errors.
return err
}

return descTable, namespaceTable, jobsTable, nil
f, err := os.Open(filePath)
if err != nil {
return err
}
defer f.Close()
decoder := json.NewDecoder(f)
return decoder.Decode(result)
}

// slurp reads a file in zipDirPath and processes its contents.
Expand Down
24 changes: 24 additions & 0 deletions pkg/cli/doctor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,18 @@ func TestDoctorZipDir(t *testing.T) {
})
})

t.Run("examine", func(t *testing.T) {
out, err := c.RunWithCapture("debug doctor examine zipdir testdata/doctor/debugzip-with-quotes")
if err != nil {
t.Fatal(err)
}

// Using datadriven allows TESTFLAGS=-rewrite.
datadriven.RunTest(t, datapathutils.TestDataPath(t, "doctor", "test_examine_zipdir_with_quotes"), func(t *testing.T, td *datadriven.TestData) string {
return out
})
})

t.Run("recreate", func(t *testing.T) {
out, err := c.RunWithCapture("debug doctor recreate zipdir testdata/doctor/debugzip")
if err != nil {
Expand All @@ -80,6 +92,18 @@ func TestDoctorZipDir(t *testing.T) {
})
})

t.Run("recreate-json", func(t *testing.T) {
out, err := c.RunWithCapture("debug doctor recreate zipdir testdata/doctor/debugzip-json")
if err != nil {
t.Fatal(err)
}

// Using datadriven allows TESTFLAGS=-rewrite.
datadriven.RunTest(t, datapathutils.TestDataPath(t, "doctor", "test_recreate_zipdir-json"), func(t *testing.T, td *datadriven.TestData) string {
return out
})
})

t.Run("deprecated doctor zipdir with verbose", func(t *testing.T) {
out, err := c.RunWithCapture("debug doctor zipdir testdata/doctor/debugzip 21.11-52 --verbose")
if err != nil {
Expand Down
Loading