Skip to content

Commit 6193ddf

Browse files
committed
sql: add unit test for SCRUB job execution semantics
This change adds a unit test (TestInspectJobImplicitTxnSemantics) to verify the behavior of inspect jobs started via EXPERIMENTAL SCRUB TABLE in implicit transactions. Includes test hooks via InspectTestingKnobs to simulate and control job execution behavior. Informs: #148289 Epic: CRDB-30356 Release note: None
1 parent e42353d commit 6193ddf

File tree

8 files changed

+201
-1
lines changed

8 files changed

+201
-1
lines changed

pkg/BUILD.bazel

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -496,6 +496,7 @@ ALL_TESTS = [
496496
"//pkg/sql/idxrecommendations:idxrecommendations_test",
497497
"//pkg/sql/idxusage:idxusage_test",
498498
"//pkg/sql/importer:importer_test",
499+
"//pkg/sql/inspect:inspect_test",
499500
"//pkg/sql/inverted:inverted_disallowed_imports_test",
500501
"//pkg/sql/inverted:inverted_test",
501502
"//pkg/sql/lex:lex_disallowed_imports_test",
@@ -2021,6 +2022,7 @@ GO_TARGETS = [
20212022
"//pkg/sql/importer:importer",
20222023
"//pkg/sql/importer:importer_test",
20232024
"//pkg/sql/inspect:inspect",
2025+
"//pkg/sql/inspect:inspect_test",
20242026
"//pkg/sql/inverted:inverted",
20252027
"//pkg/sql/inverted:inverted_test",
20262028
"//pkg/sql/isql:isql",

pkg/base/testing_knobs.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ type TestingKnobs struct {
3232
JobsTestingKnobs ModuleTestingKnobs
3333
BackupRestore ModuleTestingKnobs
3434
TTL ModuleTestingKnobs
35+
Inspect ModuleTestingKnobs
3536
SchemaTelemetry ModuleTestingKnobs
3637
Streaming ModuleTestingKnobs
3738
UpgradeManager ModuleTestingKnobs

pkg/server/server_sql.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1110,6 +1110,9 @@ func newSQLServer(ctx context.Context, cfg sqlServerArgs) (*SQLServer, error) {
11101110
if ttlKnobs := cfg.TestingKnobs.TTL; ttlKnobs != nil {
11111111
execCfg.TTLTestingKnobs = ttlKnobs.(*sql.TTLTestingKnobs)
11121112
}
1113+
if inspectKnobs := cfg.TestingKnobs.Inspect; inspectKnobs != nil {
1114+
execCfg.InspectTestingKnobs = inspectKnobs.(*sql.InspectTestingKnobs)
1115+
}
11131116
if schemaTelemetryKnobs := cfg.TestingKnobs.SchemaTelemetry; schemaTelemetryKnobs != nil {
11141117
execCfg.SchemaTelemetryTestingKnobs = schemaTelemetryKnobs.(*sql.SchemaTelemetryTestingKnobs)
11151118
}

pkg/sql/exec_util.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1513,6 +1513,7 @@ type ExecutorConfig struct {
15131513
EvalContextTestingKnobs eval.TestingKnobs
15141514
TenantTestingKnobs *TenantTestingKnobs
15151515
TTLTestingKnobs *TTLTestingKnobs
1516+
InspectTestingKnobs *InspectTestingKnobs
15161517
SchemaTelemetryTestingKnobs *SchemaTelemetryTestingKnobs
15171518
BackupRestoreTestingKnobs *BackupRestoreTestingKnobs
15181519
StreamingTestingKnobs *StreamingTestingKnobs
@@ -1967,6 +1968,16 @@ type TTLTestingKnobs struct {
19671968
// ModuleTestingKnobs implements the base.ModuleTestingKnobs interface.
19681969
func (*TTLTestingKnobs) ModuleTestingKnobs() {}
19691970

1971+
// InspectTestingKnobs contains testing knobs for the INSPECT command.
1972+
type InspectTestingKnobs struct {
1973+
// OnInspectJobStart is called just before the inspect job begins execution.
1974+
// If it returns an error, the job fails immediately.
1975+
OnInspectJobStart func() error
1976+
}
1977+
1978+
// ModuleTestingKnobs implements the base.ModuleTestingKnobs interface.
1979+
func (*InspectTestingKnobs) ModuleTestingKnobs() {}
1980+
19701981
// SchemaTelemetryTestingKnobs contains testing knobs for schema telemetry.
19711982
type SchemaTelemetryTestingKnobs struct {
19721983
// AOSTDuration changes the AOST timestamp duration to add to the

pkg/sql/inspect/BUILD.bazel

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
load("@io_bazel_rules_go//go:def.bzl", "go_library")
1+
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
22

33
go_library(
44
name = "inspect",
@@ -9,7 +9,30 @@ go_library(
99
"//pkg/jobs",
1010
"//pkg/jobs/jobspb",
1111
"//pkg/settings/cluster",
12+
"//pkg/sql",
1213
"//pkg/sql/isql",
1314
"//pkg/util/log",
1415
],
1516
)
17+
18+
go_test(
19+
name = "inspect_test",
20+
srcs = [
21+
"inspect_job_test.go",
22+
"main_test.go",
23+
],
24+
deps = [
25+
"//pkg/base",
26+
"//pkg/security/securityassets",
27+
"//pkg/security/securitytest",
28+
"//pkg/server",
29+
"//pkg/sql",
30+
"//pkg/testutils/serverutils",
31+
"//pkg/testutils/sqlutils",
32+
"//pkg/testutils/testcluster",
33+
"//pkg/util/leaktest",
34+
"//pkg/util/log",
35+
"@com_github_cockroachdb_errors//:errors",
36+
"@com_github_stretchr_testify//require",
37+
],
38+
)

pkg/sql/inspect/inspect_job.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"github.com/cockroachdb/cockroach/pkg/jobs"
1212
"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
1313
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
14+
"github.com/cockroachdb/cockroach/pkg/sql"
1415
"github.com/cockroachdb/cockroach/pkg/sql/isql"
1516
"github.com/cockroachdb/cockroach/pkg/util/log"
1617
)
@@ -25,6 +26,20 @@ var _ jobs.Resumer = &inspectResumer{}
2526
func (c *inspectResumer) Resume(ctx context.Context, execCtx interface{}) error {
2627
log.Infof(ctx, "starting INSPECT job")
2728

29+
jobExecCtx := execCtx.(sql.JobExecContext)
30+
execCfg := jobExecCtx.ExecCfg()
31+
32+
var knobs sql.InspectTestingKnobs
33+
if inspectKnobs := execCfg.InspectTestingKnobs; inspectKnobs != nil {
34+
knobs = *inspectKnobs
35+
}
36+
37+
if knobs.OnInspectJobStart != nil {
38+
if err := knobs.OnInspectJobStart(); err != nil {
39+
return err
40+
}
41+
}
42+
2843
if err := c.job.NoTxn().Update(ctx,
2944
func(_ isql.Txn, md jobs.JobMetadata, ju *jobs.JobUpdater) error {
3045
progress := md.Progress
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
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_test
7+
8+
import (
9+
"context"
10+
"regexp"
11+
"sync/atomic"
12+
"testing"
13+
"time"
14+
15+
"github.com/cockroachdb/cockroach/pkg/base"
16+
"github.com/cockroachdb/cockroach/pkg/sql"
17+
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
18+
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
19+
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
20+
"github.com/cockroachdb/cockroach/pkg/util/log"
21+
"github.com/cockroachdb/errors"
22+
"github.com/stretchr/testify/require"
23+
)
24+
25+
// TestInspectJobImplicitTxnSemantics validates how inspect jobs behave when
26+
// triggered by statements that run in implicit transactions. It verifies that the job
27+
// starts correctly, that errors or timeouts propagate to the user, and that
28+
// client-visible semantics (like statement timeout or job failure) behave as expected.
29+
//
30+
// Note: This test currently uses SCRUB to trigger a job, but is not testing SCRUB
31+
// itself. The goal is to verify general execution semantics for async job statements.
32+
func TestInspectJobImplicitTxnSemantics(t *testing.T) {
33+
defer leaktest.AfterTest(t)()
34+
defer log.Scope(t).Close(t)
35+
36+
var onInspectErrorToReturn atomic.Pointer[error]
37+
var pauseJobStart atomic.Bool
38+
s, db, _ := serverutils.StartServer(t, base.TestServerArgs{
39+
Knobs: base.TestingKnobs{
40+
Inspect: &sql.InspectTestingKnobs{
41+
OnInspectJobStart: func() error {
42+
// Use a timeout so we aren't stuck in here forever in case something bad happens.
43+
const maxPause = 30 * time.Second
44+
deadline := time.After(maxPause)
45+
for {
46+
if !pauseJobStart.Load() {
47+
break
48+
}
49+
select {
50+
case <-deadline:
51+
return errors.Newf("test timed out after %s while waiting for pause to clear", maxPause)
52+
default:
53+
time.Sleep(10 * time.Millisecond)
54+
}
55+
}
56+
if errPtr := onInspectErrorToReturn.Load(); errPtr != nil {
57+
return *errPtr
58+
}
59+
return nil
60+
},
61+
},
62+
},
63+
})
64+
defer s.Stopper().Stop(context.Background())
65+
runner := sqlutils.MakeSQLRunner(db)
66+
67+
runner.Exec(t, `
68+
CREATE DATABASE db;
69+
SET enable_scrub_job = true;
70+
CREATE TABLE db.t (
71+
id INT PRIMARY KEY,
72+
val INT
73+
);
74+
CREATE INDEX i1 on db.t (val);
75+
INSERT INTO db.t VALUES (1, 2), (2,3);
76+
`)
77+
78+
for _, tc := range []struct {
79+
desc string
80+
setupSQL string
81+
tearDownSQL string
82+
pauseAtStart bool
83+
onStartError error
84+
expectedErrRegex string
85+
}{
86+
{desc: "inspect success"},
87+
{desc: "inspect failure", onStartError: errors.Newf("inspect validation error"), expectedErrRegex: "inspect validation error"},
88+
// Note: avoiding small statement timeouts, as this can impact the ability to reset.
89+
{desc: "statement timeout", setupSQL: "SET statement_timeout = '1s'", tearDownSQL: "RESET statement_timeout", pauseAtStart: true, expectedErrRegex: "canceled"},
90+
} {
91+
t.Run(tc.desc, func(t *testing.T) {
92+
if tc.setupSQL != "" {
93+
runner.Exec(t, tc.setupSQL)
94+
}
95+
if tc.tearDownSQL != "" {
96+
defer func() { runner.Exec(t, tc.tearDownSQL) }()
97+
}
98+
if tc.pauseAtStart {
99+
pauseJobStart.Store(true)
100+
}
101+
if tc.onStartError != nil {
102+
onInspectErrorToReturn.Store(&tc.onStartError)
103+
defer func() { onInspectErrorToReturn.Store(nil) }()
104+
}
105+
_, err := db.Exec("EXPERIMENTAL SCRUB TABLE db.t")
106+
pauseJobStart.Store(false)
107+
if tc.expectedErrRegex == "" {
108+
require.NoError(t, err)
109+
return
110+
}
111+
112+
require.Error(t, err)
113+
re := regexp.MustCompile(tc.expectedErrRegex)
114+
match := re.MatchString(err.Error())
115+
require.True(t, match, "Error text %q doesn't match the expected regexp of %q",
116+
err.Error(), tc.expectedErrRegex)
117+
})
118+
}
119+
}

pkg/sql/inspect/main_test.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
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_test
7+
8+
import (
9+
"os"
10+
"testing"
11+
12+
"github.com/cockroachdb/cockroach/pkg/security/securityassets"
13+
"github.com/cockroachdb/cockroach/pkg/security/securitytest"
14+
"github.com/cockroachdb/cockroach/pkg/server"
15+
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
16+
"github.com/cockroachdb/cockroach/pkg/testutils/testcluster"
17+
)
18+
19+
func TestMain(m *testing.M) {
20+
securityassets.SetLoader(securitytest.EmbeddedAssets)
21+
serverutils.InitTestServerFactory(server.TestServerFactory)
22+
serverutils.InitTestClusterFactory(testcluster.TestClusterFactory)
23+
os.Exit(m.Run())
24+
}
25+
26+
//go:generate ../../util/leaktest/add-leaktest.sh *_test.go

0 commit comments

Comments
 (0)