sql: add support for span checks and cluster checks

Previously the checks were independent and could only process individual
spans.
The row count check will be dependent on other checks that perform the
scan of each span and will need to aggregate the row counts after all
the spans have been processed.
This change sets up interfaces for span-level (dependent on other
checks running on the spans) and cluster-level checks (run after all spans have
been processed).

Part of: #155472
Epic: CRDB-55075

Release note: None

Author: bghal

pkg/jobs/jobspb/jobs.go

@@ -176,3 +176,9 @@ func (r *RowLevelTTLProcessorProgress) SafeFormat(p redact.SafePrinter, _ rune)
 func (c CreateStatsDetails) IsAuto() bool {
 	return c.Name == AutoStatsName || c.Name == AutoPartialStatsName
 }
+
+// UpdateWithSpanProgress updates the InspectProgress with data from the
+// check-specific fields in the span progress update.
+func (p *InspectProgress) UpdateWithSpanProgress(prog *InspectProcessorProgress) {
+	return
+}

pkg/sql/inspect/index_consistency_check.go

@@ -58,21 +58,6 @@ func (c *indexConsistencyCheckApplicability) AppliesTo(
 	return spanContainsTable(c.tableID, codec, span)
 }
 
-// checkState represents the state of an index consistency check.
-type checkState int
-
-const (
-	// checkNotStarted indicates Start() has not been called yet.
-	checkNotStarted checkState = iota
-	// checkHashMatched indicates the hash precheck passed - no corruption detected,
-	// so the full check can be skipped.
-	checkHashMatched
-	// checkRunning indicates the full check is actively running and may produce more results.
-	checkRunning
-	// checkDone indicates the check has finished (iterator exhausted or error occurred).
-	checkDone
-)
-
 // indexConsistencyCheck verifies consistency between a table's primary index
 // and a specified secondary index by streaming rows from both sides of a
 // query. It reports an issue if a key exists in the primary but not the

pkg/sql/inspect/inspect_processor.go

@@ -267,7 +267,7 @@ func (p *inspectProcessor) processSpan(
 	asOfToUse := p.getTimestampForSpan()
 
 	// Only create checks that apply to this span.
-	var checks []inspectCheck
+	var checks inspectChecks
 	for _, factory := range p.checkFactories {
 		check := factory(asOfToUse)
 		applies, err := check.AppliesTo(p.cfg.Codec, span)
@@ -278,6 +278,16 @@ func (p *inspectProcessor) processSpan(
 			checks = append(checks, check)
 		}
 	}
+	checks.sortStable()
+
+	// Provide the span-level checks with a reference to all the other checks.
+	for _, check := range checks {
+		if check, ok := check.(inspectSpanCheck); ok {
+			if err := check.RegisterChecksForSpan(checks); err != nil {
+				return err
+			}
+		}
+	}
 
 	runner := inspectRunner{
 		checks: checks,
@@ -315,8 +325,22 @@ func (p *inspectProcessor) processSpan(
 		}
 	}
 
+	progressMsg := &jobspb.InspectProcessorProgress{
+		ChecksCompleted: 0, // No additional checks completed, just marking span done
+		Finished:        false,
+	}
+
+	for _, check := range checks {
+		if check, ok := check.(inspectSpanCheck); ok {
+			err := check.CheckSpan(ctx, p.loggerBundle, progressMsg)
+			if err != nil {
+				return err
+			}
+		}
+	}
+
 	// Report span completion for checkpointing.
-	if err := p.sendSpanCompletionProgress(ctx, output, span, false /* finished */); err != nil {
+	if err := p.sendSpanCompletionProgress(ctx, output, span, false /* finished */, progressMsg); err != nil {
 		return err
 	}
 
@@ -364,15 +388,18 @@ func (p *inspectProcessor) getTimestampForSpan() hlc.Timestamp {
 // sendSpanCompletionProgress sends progress indicating a span has been completed.
 // This is used for checkpointing to track which spans are done.
 func (p *inspectProcessor) sendSpanCompletionProgress(
-	ctx context.Context, output execinfra.RowReceiver, completedSpan roachpb.Span, finished bool,
+	ctx context.Context,
+	output execinfra.RowReceiver,
+	completedSpan roachpb.Span,
+	finished bool,
+	progressMsg *jobspb.InspectProcessorProgress,
 ) error {
 	if p.spansProcessedCtr != nil {
 		p.spansProcessedCtr.Inc(1)
 	}
-	progressMsg := &jobspb.InspectProcessorProgress{
-		ChecksCompleted: 0, // No additional checks completed, just marking span done
-		Finished:        finished,
-	}
+
+	progressMsg.ChecksCompleted = 0 // No additional checks completed, just marking span done
+	progressMsg.Finished = finished
 
 	progressAny, err := pbtypes.MarshalAny(progressMsg)
 	if err != nil {
@@ -416,7 +443,7 @@ func newInspectProcessor(
 ) (execinfra.Processor, error) {
 	execCfg := flowCtx.Cfg.ExecutorConfig.(*sql.ExecutorConfig)
 
-	checkFactories, err := buildInspectCheckFactories(execCfg, spec)
+	checkFactories, err := buildInspectCheckFactories(execCfg, spec.InspectDetails)
 	if err != nil {
 		return nil, err
 	}
@@ -430,7 +457,7 @@ func newInspectProcessor(
 		checkFactories:    checkFactories,
 		cfg:               flowCtx.Cfg,
 		spanSrc:           newSliceSpanSource(spec.Spans),
-		loggerBundle:      getInspectLogger(execCfg, spec.JobID),
+		loggerBundle:      getInspectLogger(flowCtx.Cfg.ExecutorConfig.(*sql.ExecutorConfig), spec.JobID),
 		concurrency:       getProcessorConcurrency(flowCtx),
 		clock:             flowCtx.Cfg.DB.KV().Clock(),
 		spansProcessedCtr: spansProcessedCtr,
@@ -444,10 +471,10 @@ func newInspectProcessor(
 // This indirection ensures that each check instance is freshly created per span,
 // avoiding shared state across concurrent workers.
 func buildInspectCheckFactories(
-	execCfg *sql.ExecutorConfig, spec execinfrapb.InspectSpec,
+	execCfg *sql.ExecutorConfig, details jobspb.InspectDetails,
 ) ([]inspectCheckFactory, error) {
-	checkFactories := make([]inspectCheckFactory, 0, len(spec.InspectDetails.Checks))
-	for _, specCheck := range spec.InspectDetails.Checks {
+	checkFactories := make([]inspectCheckFactory, 0, len(details.Checks))
+	for _, specCheck := range details.Checks {
 		tableID := specCheck.TableID
 		indexID := specCheck.IndexID
 		tableVersion := specCheck.TableVersion
@@ -464,7 +491,6 @@ func buildInspectCheckFactories(
 					asOf:         asOf,
 				}
 			})
-
 		default:
 			return nil, errors.AssertionFailedf("unsupported inspect check type: %v", specCheck.Type)
 		}

pkg/sql/inspect/inspect_processor_test.go

@@ -7,7 +7,6 @@ package inspect
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"testing"
 	"time"
@@ -24,6 +23,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/uuid"
+	"github.com/cockroachdb/errors"
 	"github.com/stretchr/testify/require"
 )
 
@@ -205,6 +205,69 @@ func (t *testingInspectCheck) Close(context.Context) error {
 	return nil
 }
 
+// testingInspectCheckSpan is a test implementation of inspectCheckSpan.
+type testingInspectCheckSpan struct {
+	started bool
+
+	spanIssues int64
+}
+
+var _ inspectCheckApplicability = (*testingInspectCheckSpan)(nil)
+
+var _ inspectSpanCheck = (*testingInspectCheckSpan)(nil)
+
+func (t *testingInspectCheckSpan) AppliesTo(codec keys.SQLCodec, span roachpb.Span) (bool, error) {
+	// For testing, assume all checks apply to all spans
+	return true, nil
+}
+
+func (t *testingInspectCheckSpan) AppliesToCluster() (bool, error) {
+	// For testing it runs.
+	return true, nil
+}
+
+func (t *testingInspectCheckSpan) Started() bool {
+	return t.started
+}
+
+func (t *testingInspectCheckSpan) Start(
+	ctx context.Context, cfg *execinfra.ServerConfig, span roachpb.Span, workerIndex int,
+) error {
+	t.started = true
+	return nil
+}
+
+func (*testingInspectCheckSpan) Next(
+	ctx context.Context, cfg *execinfra.ServerConfig,
+) (*inspectIssue, error) {
+	return nil, nil
+}
+
+func (*testingInspectCheckSpan) Done(ctx context.Context) bool {
+	return true
+}
+
+func (*testingInspectCheckSpan) Close(ctx context.Context) error {
+	return nil
+}
+
+func (t *testingInspectCheckSpan) RegisterChecksForSpan(checks inspectChecks) error {
+	return nil
+}
+
+func (t *testingInspectCheckSpan) CheckSpan(
+	ctx context.Context, logger *inspectLoggerBundle, msg *jobspb.InspectProcessorProgress,
+) error {
+	for i := t.spanIssues; i > 0; i-- {
+		err := logger.logIssue(ctx, &inspectIssue{})
+		if err != nil {
+			return errors.Wrapf(err, "error logging inspect issue")
+		}
+	}
+
+	return nil
+}
+
 // discardRowReceiver is a minimal RowReceiver that discards all data.
 type discardRowReceiver struct{}
 
@@ -249,7 +312,7 @@ func runProcessorAndWait(t *testing.T, proc *inspectProcessor, expectErr bool) {
 // makeProcessor will create an inspect processor for test.
 func makeProcessor(
 	t *testing.T,
-	checkFactory inspectCheckFactory,
+	checkFactories []inspectCheckFactory,
 	src spanSource,
 	concurrency int,
 	asOf hlc.Timestamp,
@@ -274,7 +337,7 @@ func makeProcessor(
 				AsOf: asOf,
 			},
 		},
-		checkFactories: []inspectCheckFactory{checkFactory},
+		checkFactories: checkFactories,
 		cfg:            flowCtx.Cfg,
 		flowCtx:        flowCtx,
 		spanSrc:        src,
@@ -358,12 +421,14 @@ func TestInspectProcessor_ControlFlow(t *testing.T) {
 
 	for _, tc := range tests {
 		t.Run(tc.desc, func(t *testing.T) {
-			factory := func(asOf hlc.Timestamp) inspectCheck {
-				return &testingInspectCheck{
-					configs: tc.configs,
-				}
+			factories := []inspectCheckFactory{
+				func(asOf hlc.Timestamp) inspectCheck {
+					return &testingInspectCheck{
+						configs: tc.configs,
+					}
+				},
 			}
-			proc, _ := makeProcessor(t, factory, tc.spanSrc, len(tc.configs), hlc.Timestamp{})
+			proc, _ := makeProcessor(t, factories, tc.spanSrc, len(tc.configs), hlc.Timestamp{})
 			runProcessorAndWait(t, proc, tc.expectErr)
 		})
 	}
@@ -377,20 +442,22 @@ func TestInspectProcessor_EmitIssues(t *testing.T) {
 		mode:     spanModeNormal,
 		maxSpans: 1,
 	}
-	factory := func(asOf hlc.Timestamp) inspectCheck {
-		return &testingInspectCheck{
-			configs: []testingCheckConfig{
-				{
-					mode: checkModeNone,
-					issues: []*inspectIssue{
-						{ErrorType: "test_error", PrimaryKey: "pk1"},
-						{ErrorType: "test_error", PrimaryKey: "pk2"},
+	factories := []inspectCheckFactory{
+		func(asOf hlc.Timestamp) inspectCheck {
+			return &testingInspectCheck{
+				configs: []testingCheckConfig{
+					{
+						mode: checkModeNone,
+						issues: []*inspectIssue{
+							{ErrorType: "test_error", PrimaryKey: "pk1"},
+							{ErrorType: "test_error", PrimaryKey: "pk2"},
+						},
 					},
 				},
-			},
-		}
+			}
+		},
 	}
-	proc, logger := makeProcessor(t, factory, spanSrc, 1, hlc.Timestamp{})
+	proc, logger := makeProcessor(t, factories, spanSrc, 1, hlc.Timestamp{})
 
 	runProcessorAndWait(t, proc, true /* expectErr */)
 
@@ -439,21 +506,23 @@ func TestInspectProcessor_AsOfTime(t *testing.T) {
 			testStartTime := time.Now()
 
 			var capturedTimestamp hlc.Timestamp
-			factory := func(asOf hlc.Timestamp) inspectCheck {
-				capturedTimestamp = asOf
-				return &testingInspectCheck{
-					configs: []testingCheckConfig{
-						{
-							mode: checkModeNone,
-							issues: []*inspectIssue{
-								{ErrorType: "test_error", PrimaryKey: "pk1", AOST: asOf.GoTime()},
+			factories := []inspectCheckFactory{
+				func(asOf hlc.Timestamp) inspectCheck {
+					capturedTimestamp = asOf
+					return &testingInspectCheck{
+						configs: []testingCheckConfig{
+							{
+								mode: checkModeNone,
+								issues: []*inspectIssue{
+									{ErrorType: "test_error", PrimaryKey: "pk1", AOST: asOf.GoTime()},
+								},
 							},
 						},
-					},
-				}
+					}
+				},
 			}
 
-			proc, logger := makeProcessor(t, factory, spanSrc, 1, tc.asOf)
+			proc, logger := makeProcessor(t, factories, spanSrc, 1, tc.asOf)
 
 			runProcessorAndWait(t, proc, true /* expectErr */)
 
@@ -465,3 +534,25 @@ func TestInspectProcessor_AsOfTime(t *testing.T) {
 		})
 	}
 }
+
+func TestInspectProcessor_SpanCheck(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
+	spanSrc := &testingSpanSource{
+		mode:     spanModeNormal,
+		maxSpans: 1,
+	}
+	factories := []inspectCheckFactory{
+		func(asOf hlc.Timestamp) inspectCheck {
+			return &testingInspectCheckSpan{
+				spanIssues: 2,
+			}
+		},
+	}
+	proc, logger := makeProcessor(t, factories, spanSrc, 1, hlc.Timestamp{})
+
+	runProcessorAndWait(t, proc, true /* expectErr */)
+
+	require.Equal(t, 2, logger.numIssuesFound())
+}