Skip to content

Commit 90ceff4

Browse files
committed
sql/inspect: skip INSPECT checks for non-applicable spans earlier
INSPECT jobs generate a set of checks along with a separate list of spans, which only apply to some of the checks. Previously, filtering to ensure a span applied to a check happened late in the process. This commit moves that filtering earlier, so checks immediately skip spans that are not relevant. This improves efficiency and prepares the system for upcoming progress tracking logic, which will be added in a separate PR. Informs: #148297 Epic: CRDB-30356 Release note: none
1 parent ffca318 commit 90ceff4

File tree

6 files changed

+92
-12
lines changed

6 files changed

+92
-12
lines changed

pkg/sql/inspect/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ go_library(
1818
deps = [
1919
"//pkg/jobs",
2020
"//pkg/jobs/jobspb",
21+
"//pkg/keys",
2122
"//pkg/kv/kvserver/protectedts/ptpb",
2223
"//pkg/roachpb",
2324
"//pkg/security/username",

pkg/sql/inspect/index_consistency_check.go

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"fmt"
1212
"strings"
1313

14+
"github.com/cockroachdb/cockroach/pkg/keys"
1415
"github.com/cockroachdb/cockroach/pkg/roachpb"
1516
"github.com/cockroachdb/cockroach/pkg/security/username"
1617
"github.com/cockroachdb/cockroach/pkg/sql/catalog"
@@ -30,13 +31,28 @@ import (
3031
"github.com/cockroachdb/redact"
3132
)
3233

33-
// indexConsistencyCheck verifies consistency between a table’s primary index
34+
// indexConsistencyCheckApplicability is a lightweight version that only implements applicability logic.
35+
type indexConsistencyCheckApplicability struct {
36+
tableID descpb.ID
37+
}
38+
39+
var _ inspectCheckApplicability = (*indexConsistencyCheckApplicability)(nil)
40+
41+
// AppliesTo implements the inspectCheckApplicability interface.
42+
func (c *indexConsistencyCheckApplicability) AppliesTo(
43+
codec keys.SQLCodec, span roachpb.Span,
44+
) (bool, error) {
45+
return spanContainsTable(c.tableID, codec, span)
46+
}
47+
48+
// indexConsistencyCheck verifies consistency between a table's primary index
3449
// and a specified secondary index by streaming rows from both sides of a
3550
// query. It reports an issue if a key exists in the primary but not the
3651
// secondary, or vice versa.
3752
type indexConsistencyCheck struct {
53+
indexConsistencyCheckApplicability
54+
3855
flowCtx *execinfra.FlowCtx
39-
tableID descpb.ID
4056
indexID descpb.IndexID
4157
asOf hlc.Timestamp
4258

@@ -57,6 +73,7 @@ type indexConsistencyCheck struct {
5773
}
5874

5975
var _ inspectCheck = (*indexConsistencyCheck)(nil)
76+
var _ inspectCheckApplicability = (*indexConsistencyCheck)(nil)
6077

6178
// Started implements the inspectCheck interface.
6279
func (c *indexConsistencyCheck) Started() bool {
@@ -67,14 +84,9 @@ func (c *indexConsistencyCheck) Started() bool {
6784
func (c *indexConsistencyCheck) Start(
6885
ctx context.Context, cfg *execinfra.ServerConfig, span roachpb.Span, workerIndex int,
6986
) error {
70-
// Early out for spans that don't apply to the table we are running the check on.
71-
_, tableID, err := cfg.Codec.DecodeTablePrefix(span.Key)
72-
if err != nil {
87+
if err := assertCheckApplies(c, cfg.Codec, span); err != nil {
7388
return err
7489
}
75-
if descpb.ID(tableID) != c.tableID {
76-
return nil
77-
}
7890

7991
// Load up the index and table descriptors.
8092
if err := c.loadCatalogInfo(ctx); err != nil {

pkg/sql/inspect/inspect_processor.go

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -190,10 +190,24 @@ func getInspectLogger(flowCtx *execinfra.FlowCtx, jobID jobspb.JobID) inspectLog
190190
func (p *inspectProcessor) processSpan(
191191
ctx context.Context, span roachpb.Span, workerIndex int,
192192
) (err error) {
193-
checks := make([]inspectCheck, len(p.checkFactories))
194-
for i, factory := range p.checkFactories {
195-
checks[i] = factory()
193+
// Only create checks that apply to this span
194+
var checks []inspectCheck
195+
for _, factory := range p.checkFactories {
196+
check := factory()
197+
applies, err := check.AppliesTo(p.cfg.Codec, span)
198+
if err != nil {
199+
return err
200+
}
201+
if applies {
202+
checks = append(checks, check)
203+
}
196204
}
205+
206+
// If no checks apply to this span, there's nothing to do
207+
if len(checks) == 0 {
208+
return nil
209+
}
210+
197211
runner := inspectRunner{
198212
checks: checks,
199213
logger: p.logger,
@@ -263,8 +277,10 @@ func buildInspectCheckFactories(
263277
case jobspb.InspectCheckIndexConsistency:
264278
checkFactories = append(checkFactories, func() inspectCheck {
265279
return &indexConsistencyCheck{
280+
indexConsistencyCheckApplicability: indexConsistencyCheckApplicability{
281+
tableID: tableID,
282+
},
266283
flowCtx: flowCtx,
267-
tableID: tableID,
268284
indexID: indexID,
269285
asOf: spec.InspectDetails.AsOf,
270286
}

pkg/sql/inspect/inspect_processor_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"testing"
1313
"time"
1414

15+
"github.com/cockroachdb/cockroach/pkg/keys"
1516
"github.com/cockroachdb/cockroach/pkg/roachpb"
1617
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
1718
"github.com/cockroachdb/cockroach/pkg/sql/execinfra"
@@ -112,6 +113,13 @@ type testingInspectCheck struct {
112113
}
113114

114115
var _ inspectCheck = (*testingInspectCheck)(nil)
116+
var _ inspectCheckApplicability = (*testingInspectCheck)(nil)
117+
118+
// AppliesTo implements the inspectCheckApplicability interface.
119+
func (t *testingInspectCheck) AppliesTo(codec keys.SQLCodec, span roachpb.Span) (bool, error) {
120+
// For testing, assume all checks apply to all spans
121+
return true, nil
122+
}
115123

116124
// Started implements the inspectCheck interface.
117125
func (t *testingInspectCheck) Started() bool {

pkg/sql/inspect/runner.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,35 @@ package inspect
88
import (
99
"context"
1010

11+
"github.com/cockroachdb/cockroach/pkg/keys"
1112
"github.com/cockroachdb/cockroach/pkg/roachpb"
13+
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
1214
"github.com/cockroachdb/cockroach/pkg/sql/execinfra"
1315
"github.com/cockroachdb/errors"
1416
)
1517

18+
// inspectCheckApplicability defines the interface for determining if a check
19+
// applies to a span.
20+
type inspectCheckApplicability interface {
21+
// AppliesTo reports whether this check applies to the given span.
22+
AppliesTo(codec keys.SQLCodec, span roachpb.Span) (bool, error)
23+
}
24+
25+
// assertCheckApplies is a helper that calls AppliesTo and asserts the check applies.
26+
func assertCheckApplies(
27+
check inspectCheckApplicability, codec keys.SQLCodec, span roachpb.Span,
28+
) error {
29+
applies, err := check.AppliesTo(codec, span)
30+
if err != nil {
31+
return err
32+
}
33+
if !applies {
34+
return errors.AssertionFailedf(
35+
"check does not apply to this span: span=%s", span.String())
36+
}
37+
return nil
38+
}
39+
1640
// inspectCheck defines a single validation operation used by the INSPECT system.
1741
// Each check represents a specific type of data validation, such as index consistency.
1842
//
@@ -25,6 +49,8 @@ import (
2549
// under the hood to detect inconsistencies. All results are surfaced through the
2650
// inspectIssue type.
2751
type inspectCheck interface {
52+
inspectCheckApplicability
53+
2854
// Started reports whether the check has been initialized.
2955
Started() bool
3056

@@ -117,3 +143,12 @@ func (c *inspectRunner) Close(ctx context.Context) error {
117143
}
118144
return retErr
119145
}
146+
147+
// spanContainsTable checks if the given span contains data for the specified table.
148+
func spanContainsTable(tableID descpb.ID, codec keys.SQLCodec, span roachpb.Span) (bool, error) {
149+
_, spanTableID, err := codec.DecodeTablePrefix(span.Key)
150+
if err != nil {
151+
return false, err
152+
}
153+
return descpb.ID(spanTableID) == tableID, nil
154+
}

pkg/sql/inspect/runner_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"fmt"
1111
"testing"
1212

13+
"github.com/cockroachdb/cockroach/pkg/keys"
1314
"github.com/cockroachdb/cockroach/pkg/roachpb"
1415
"github.com/cockroachdb/cockroach/pkg/sql/execinfra"
1516
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
@@ -28,6 +29,13 @@ type mockInspectCheck struct {
2829
}
2930

3031
var _ inspectCheck = &mockInspectCheck{}
32+
var _ inspectCheckApplicability = &mockInspectCheck{}
33+
34+
// AppliesTo implements the inspectCheckApplicability interface.
35+
func (m *mockInspectCheck) AppliesTo(codec keys.SQLCodec, span roachpb.Span) (bool, error) {
36+
// For testing, assume all checks apply to all spans
37+
return true, nil
38+
}
3139

3240
func (m *mockInspectCheck) Started() bool {
3341
return m.started

0 commit comments

Comments
 (0)