Skip to content

Commit 6b0321a

Browse files
committed
sql/inspect: introduce interface for INSPECT check infrastructure
Add a new interface to support INSPECT operations that perform checks emitting rows for each issue found. This interface follows a row iterator pattern, enabling future implementation of specific checks. Informs: #146863 Epic: CRDB-30356 Release note: none
1 parent 9ddf582 commit 6b0321a

File tree

6 files changed

+479
-0
lines changed

6 files changed

+479
-0
lines changed

pkg/sql/inspect/BUILD.bazel

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@ go_library(
55
srcs = [
66
"inspect_job.go",
77
"inspect_processor.go",
8+
"issue.go",
9+
"log_sink.go",
10+
"runner.go",
811
],
912
importpath = "github.com/cockroachdb/cockroach/pkg/sql/inspect",
1013
visibility = ["//visibility:public"],
@@ -14,6 +17,7 @@ go_library(
1417
"//pkg/roachpb",
1518
"//pkg/settings/cluster",
1619
"//pkg/sql",
20+
"//pkg/sql/catalog/descpb",
1721
"//pkg/sql/catalog/descs",
1822
"//pkg/sql/execinfra",
1923
"//pkg/sql/execinfrapb",
@@ -25,28 +29,35 @@ go_library(
2529
"//pkg/util/log",
2630
"//pkg/util/tracing",
2731
"@com_github_cockroachdb_errors//:errors",
32+
"@com_github_cockroachdb_redact//:redact",
2833
],
2934
)
3035

3136
go_test(
3237
name = "inspect_test",
3338
srcs = [
3439
"inspect_job_test.go",
40+
"issue_test.go",
3541
"main_test.go",
42+
"runner_test.go",
3643
],
44+
embed = [":inspect"],
3745
deps = [
3846
"//pkg/base",
47+
"//pkg/roachpb",
3948
"//pkg/security/securityassets",
4049
"//pkg/security/securitytest",
4150
"//pkg/server",
4251
"//pkg/sql",
52+
"//pkg/sql/execinfra",
4353
"//pkg/testutils",
4454
"//pkg/testutils/serverutils",
4555
"//pkg/testutils/sqlutils",
4656
"//pkg/testutils/testcluster",
4757
"//pkg/util/leaktest",
4858
"//pkg/util/log",
4959
"@com_github_cockroachdb_errors//:errors",
60+
"@com_github_cockroachdb_redact//:redact",
5061
"@com_github_stretchr_testify//require",
5162
],
5263
)

pkg/sql/inspect/issue.go

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
// Copyright 2025 The Cockroach Authors.
2+
//
3+
// Use of this software is governed by the CockroachDB Software License
4+
// included in the /LICENSE file.
5+
6+
package inspect
7+
8+
import (
9+
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
10+
"github.com/cockroachdb/redact"
11+
)
12+
13+
// inspectIssue represents a single validation failure detected by an inspectCheck.
14+
// These issues correspond to rows written into the system.inspect_errors table.
15+
//
16+
// Each issue identifies the object where the inconsistency was found, the specific
17+
// problem type, and a JSON-serializable details map with check-specific context.
18+
type inspectIssue struct {
19+
// ErrorType is a machine-readable string describing the type of validation failure
20+
ErrorType redact.RedactableString
21+
22+
// DatabaseID is the descriptor ID of the database containing the object in error.
23+
// May be 0 if not applicable.
24+
DatabaseID descpb.ID
25+
26+
// SchemaID is the descriptor ID of the schema containing the object in error.
27+
// May be 0 if not applicable.
28+
SchemaID descpb.ID
29+
30+
// ObjectID is the descriptor ID of the thing where the error occurred.
31+
// Usually this is the ID of the table.
32+
ObjectID descpb.ID
33+
34+
// PrimaryKey is the primary key of the row involved in the issue, rendered as a string.
35+
// If the error does not relate to a specific row, this may be empty.
36+
PrimaryKey string
37+
38+
// Details contains additional structured metadata describing the issue.
39+
// The contents vary by check type.
40+
Details map[redact.RedactableString]interface{}
41+
}
42+
43+
var _ redact.SafeFormatter = (*inspectIssue)(nil)
44+
45+
// SafeFormat implements the redact.SafeFormatter interface.
46+
func (i inspectIssue) SafeFormat(w redact.SafePrinter, _ rune) {
47+
var buf redact.StringBuilder
48+
buf.Printf("{type=")
49+
buf.Print(i.ErrorType)
50+
if i.DatabaseID != 0 {
51+
buf.Printf(" db=%d", i.DatabaseID)
52+
}
53+
if i.SchemaID != 0 {
54+
buf.Printf(" schema=%d", i.SchemaID)
55+
}
56+
if i.ObjectID != 0 {
57+
buf.Printf(" obj=%d", i.ObjectID)
58+
}
59+
buf.Printf(" pk=%q", i.PrimaryKey)
60+
if i.Details != nil {
61+
buf.Printf(" details=%+v", i.Details)
62+
}
63+
buf.Printf("}")
64+
w.Print(buf)
65+
}
66+
67+
func (i inspectIssue) String() string { return redact.StringWithoutMarkers(i) }

pkg/sql/inspect/issue_test.go

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
// Copyright 2025 The Cockroach Authors.
2+
//
3+
// Use of this software is governed by the CockroachDB Software License
4+
// included in the /LICENSE file.
5+
6+
package inspect
7+
8+
import (
9+
"testing"
10+
11+
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
12+
"github.com/cockroachdb/cockroach/pkg/util/log"
13+
"github.com/cockroachdb/redact"
14+
"github.com/stretchr/testify/require"
15+
)
16+
17+
func TestIssueRedaction(t *testing.T) {
18+
defer leaktest.AfterTest(t)()
19+
defer log.Scope(t).Close(t)
20+
21+
testCases := []struct {
22+
desc string
23+
issue *inspectIssue
24+
redactable string
25+
redacted string
26+
}{
27+
{
28+
desc: "no details",
29+
issue: &inspectIssue{
30+
ErrorType: "testing_error",
31+
DatabaseID: 10,
32+
ObjectID: 101,
33+
PrimaryKey: "/Table/101/c=10",
34+
},
35+
redactable: "{type=testing_error db=10 obj=101 pk=‹\"/Table/101/c=10\"›}",
36+
redacted: "{type=testing_error db=10 obj=101 pk=‹×›}",
37+
},
38+
{
39+
desc: "with details",
40+
issue: &inspectIssue{
41+
ErrorType: "testing_system_error",
42+
SchemaID: 100,
43+
PrimaryKey: "/System/100/",
44+
Details: map[redact.RedactableString]interface{}{
45+
"field1": 10,
46+
"field2": "strval",
47+
},
48+
},
49+
redactable: "{type=testing_system_error schema=100 pk=‹\"/System/100/\"› details=map[field1:10 field2:‹strval›]}",
50+
redacted: "{type=testing_system_error schema=100 pk=‹×› details=map[field1:10 field2:‹×›]}",
51+
},
52+
{
53+
desc: "details that has a redactable string value",
54+
issue: &inspectIssue{
55+
ErrorType: "testing_system_error",
56+
Details: map[redact.RedactableString]interface{}{
57+
"field1": redact.RedactableString("redactable string"),
58+
},
59+
},
60+
redactable: "{type=testing_system_error pk=‹\"\"› details=map[field1:redactable string]}",
61+
redacted: "{type=testing_system_error pk=‹×› details=map[field1:redactable string]}",
62+
},
63+
}
64+
for _, c := range testCases {
65+
t.Run(c.desc, func(t *testing.T) {
66+
require.EqualValues(t, c.redactable, redact.Sprint(c.issue))
67+
require.EqualValues(t, c.redacted, redact.Sprint(c.issue).Redact())
68+
})
69+
}
70+
}

pkg/sql/inspect/log_sink.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// Copyright 2025 The Cockroach Authors.
2+
//
3+
// Use of this software is governed by the CockroachDB Software License
4+
// included in the /LICENSE file.
5+
6+
package inspect
7+
8+
import (
9+
"context"
10+
11+
"github.com/cockroachdb/cockroach/pkg/util/log"
12+
)
13+
14+
// logSink will report any inspect errors directly to cockroach.log.
15+
type logSink struct{}
16+
17+
var _ inspectLogger = &logSink{}
18+
19+
// logIssue implements the inspectLogger interface.
20+
func (c *logSink) logIssue(ctx context.Context, issue *inspectIssue) error {
21+
log.Errorf(ctx, "inspect issue: %+v", issue)
22+
return nil
23+
}

pkg/sql/inspect/runner.go

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
// Copyright 2025 The Cockroach Authors.
2+
//
3+
// Use of this software is governed by the CockroachDB Software License
4+
// included in the /LICENSE file.
5+
6+
package inspect
7+
8+
import (
9+
"context"
10+
11+
"github.com/cockroachdb/cockroach/pkg/roachpb"
12+
"github.com/cockroachdb/cockroach/pkg/sql/execinfra"
13+
"github.com/cockroachdb/errors"
14+
)
15+
16+
// inspectCheck defines a single validation operation used by the INSPECT system.
17+
// Each check represents a specific type of data validation, such as index consistency.
18+
//
19+
// A check is stateful. It must be initialized with Start(), which prepares it to
20+
// produce results. Once started, repeated calls to Next() yield zero or more
21+
// inspectIssue results until Done() returns true. After completion, Close() releases
22+
// any associated resources.
23+
//
24+
// Checks are expected to run on a single node and may execute SQL queries or scans
25+
// under the hood to detect inconsistencies. All results are surfaced through the
26+
// inspectIssue type.
27+
type inspectCheck interface {
28+
// Started reports whether the check has been initialized.
29+
Started() bool
30+
31+
// Start prepares the check to begin returning results.
32+
Start(ctx context.Context, cfg *execinfra.ServerConfig, span roachpb.Span, workerIndex int) error
33+
34+
// Next returns the next inspect error, if any.
35+
// Returns (nil, nil) when there are no errors for the current row.
36+
Next(ctx context.Context, cfg *execinfra.ServerConfig) (*inspectIssue, error)
37+
38+
// Done reports whether the check has produced all results.
39+
Done(ctx context.Context) bool
40+
41+
// Close cleans up resources for the check.
42+
Close(ctx context.Context) error
43+
}
44+
45+
// inspectLogger records issues found by inspect checks. Implementations of this
46+
// interface define how inspectIssue results are handled.
47+
type inspectLogger interface {
48+
logIssue(ctx context.Context, issue *inspectIssue) error
49+
}
50+
51+
// inspectRunner coordinates the execution of a set of inspectChecks.
52+
//
53+
// It manages the lifecycle of each check, including initialization,
54+
// iteration, and cleanup. Each call to Step processes one unit of
55+
// work: either advancing a check by one result or moving on to the next
56+
// check if the current one is finished.
57+
//
58+
// When a validation issue is found, the runner calls the provided
59+
// inspectLogger to record it.
60+
type inspectRunner struct {
61+
// checks holds the list of checks to run. Each check is run to completion
62+
// before moving on to the next.
63+
checks []inspectCheck
64+
65+
// logger records issues reported by the checks.
66+
logger inspectLogger
67+
}
68+
69+
// Step advances execution by processing one result from the current inspectCheck.
70+
//
71+
// If the current check is not yet started, it is initialized. If it has more results,
72+
// Step retrieves the next result and logs it if an issue is found. If the check is done,
73+
// it is closed and removed from the queue.
74+
//
75+
// Returns true if a check was advanced or an issue was found. Returns false when all
76+
// checks are complete. If an error occurs at any stage, it is returned immediately.
77+
func (c *inspectRunner) Step(
78+
ctx context.Context, cfg *execinfra.ServerConfig, span roachpb.Span, workerIndex int,
79+
) (bool, error) {
80+
for len(c.checks) > 0 {
81+
check := c.checks[0]
82+
if !check.Started() {
83+
if err := check.Start(ctx, cfg, span, workerIndex); err != nil {
84+
return false, err
85+
}
86+
}
87+
88+
if !check.Done(ctx) {
89+
issue, err := check.Next(ctx, cfg)
90+
if err != nil {
91+
return false, err
92+
}
93+
if issue != nil {
94+
err = c.logger.logIssue(ctx, issue)
95+
if err != nil {
96+
return false, errors.Wrapf(err, "error logging inspect issue")
97+
}
98+
}
99+
return true, nil
100+
}
101+
102+
if err := check.Close(ctx); err != nil {
103+
return false, err
104+
}
105+
c.checks = c.checks[1:]
106+
}
107+
return false, nil
108+
}

0 commit comments

Comments
 (0)