Merge #156585 #156637

156585: sql: move inspect_job.go to pkg/sql/inspect r=spilchen a=spilchen

This change moves all inspect related code out of `pkg/sql`, consolidating it under the `pkg/sql/inspect` package:
- Moves `inspect_job.go` to `inspect/inspect_job_entry.go`, fully relocating inspect job logic into the inspect package.
- Renames `inspect_job.go` to `inspect_resumer.go` to match naming conventions used by other job resumers.
- Cleans up naming: removes redundant Inspect prefixes (e.g., TriggerInspectJob → TriggerJob) and unexports functions that are internal-only.

Closes #155473
Epic: CRDB-55075
Release note: none

156637: rpc: skip clock offset tests under duress r=stevendanna a=tbg

Closes #154839.

Reviewers: please bors on LGTM.

Epic: none

Co-authored-by: Matt Spilchen <matt.spilchen@cockroachlabs.com>
Co-authored-by: Tobias Grieger <tobias.b.grieger@gmail.com>

Author: 3 people

pkg/rpc/context_test.go

@@ -193,6 +193,7 @@ func TestClockOffsetInClientPingRequest(t *testing.T) {
 }
 
 func testClockOffsetInPingRequestInternal(t *testing.T, clientOnly bool) {
+	skip.UnderDuress(t, "https://github.com/cockroachdb/cockroach/issues/154839")
 	ctx := context.Background()
 	stopper := stop.NewStopper()
 	defer stopper.Stop(ctx)

pkg/sql/BUILD.bazel

@@ -141,7 +141,6 @@ go_library(
         "information_schema.go",
         "insert.go",
         "insert_fast_path.go",
-        "inspect_job.go",
         "instrumentation.go",
         "internal.go",
         "internal_result_channel.go",

pkg/sql/importer/BUILD.bazel

@@ -61,6 +61,7 @@ go_library(
         "//pkg/sql/faketreeeval",
         "//pkg/sql/flowinfra",
         "//pkg/sql/gcjob",
+        "//pkg/sql/inspect",
         "//pkg/sql/isql",
         "//pkg/sql/lexbase",
         "//pkg/sql/pgwire/pgcode",

pkg/sql/importer/import_job.go

@@ -31,6 +31,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/execinfrapb"
 	"github.com/cockroachdb/cockroach/pkg/sql/flowinfra"
 	"github.com/cockroachdb/cockroach/pkg/sql/gcjob"
+	"github.com/cockroachdb/cockroach/pkg/sql/inspect"
 	"github.com/cockroachdb/cockroach/pkg/sql/isql"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/catid"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
@@ -328,12 +329,12 @@ func (r *importResumer) Resume(ctx context.Context, execCtx interface{}) error {
 		}
 		tblDesc := tabledesc.NewBuilder(table.Desc).BuildImmutableTable()
 		if len(tblDesc.PublicNonPrimaryIndexes()) > 0 {
-			checks, err := sql.InspectChecksForTable(ctx, nil /* p */, tblDesc)
+			checks, err := inspect.ChecksForTable(ctx, nil /* p */, tblDesc)
 			if err != nil {
 				return err
 			}
 
-			job, err := sql.TriggerInspectJob(
+			job, err := inspect.TriggerJob(
 				ctx,
 				fmt.Sprintf("import-validation-%s", tblDesc.GetName()),
 				p.ExecCfg(),

pkg/sql/inspect/BUILD.bazel

@@ -4,11 +4,12 @@ go_library(
     name = "inspect",
     srcs = [
         "index_consistency_check.go",
-        "inspect_job.go",
+        "inspect_job_entry.go",
         "inspect_logger.go",
         "inspect_metrics.go",
         "inspect_plan.go",
         "inspect_processor.go",
+        "inspect_resumer.go",
         "issue.go",
         "log_sink.go",
         "progress.go",
@@ -71,9 +72,9 @@ go_test(
     size = "large",
     srcs = [
         "index_consistency_check_test.go",
-        "inspect_job_test.go",
         "inspect_metrics_test.go",
         "inspect_processor_test.go",
+        "inspect_resumer_test.go",
         "issue_test.go",
         "main_test.go",
         "progress_test.go",

pkg/sql/inspect_job.go renamed to pkg/sql/inspect/inspect_job_entry.go

@@ -3,17 +3,15 @@
 // Use of this software is governed by the CockroachDB Software License
 // included in the /LICENSE file.
 
-// TODO(148365): Move this file to the `pkg/sql/inspect` package. The scrub command
-// will have to forgo its dependency on inspect.
-
-package sql
+package inspect
 
 import (
 	"context"
 
 	"github.com/cockroachdb/cockroach/pkg/jobs"
 	"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
 	"github.com/cockroachdb/cockroach/pkg/security/username"
+	"github.com/cockroachdb/cockroach/pkg/sql"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/isql"
@@ -27,13 +25,11 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 )
 
-// TriggerInspectJob starts an inspect job for the snapshot.
-// TODO(148365): Stop exporting this function when it's moved to the inspect
-// package.
-func TriggerInspectJob(
+// TriggerJob starts an inspect job for the snapshot.
+func TriggerJob(
 	ctx context.Context,
 	jobRecordDescription string,
-	execCfg *ExecutorConfig,
+	execCfg *sql.ExecutorConfig,
 	txn isql.Txn,
 	checks []*jobspb.InspectDetails_Check,
 	asOf hlc.Timestamp,
@@ -98,11 +94,10 @@ func TriggerInspectJob(
 	return job, nil
 }
 
-// InspectChecksForDatabase generates checks on every supported index on every
+// checksForDatabase generates checks on every supported index on every
 // table in the given database.
-// TODO(148365): Stop exporting this function after moving to pkg/sql/inspect.
-func InspectChecksForDatabase(
-	ctx context.Context, p PlanHookState, db catalog.DatabaseDescriptor,
+func checksForDatabase(
+	ctx context.Context, p sql.PlanHookState, db catalog.DatabaseDescriptor,
 ) ([]*jobspb.InspectDetails_Check, error) {
 	avoidLeased := false
 	if aost := p.ExtendedEvalContext().AsOfSystemTime; aost != nil {
@@ -120,7 +115,7 @@ func InspectChecksForDatabase(
 	checks := []*jobspb.InspectDetails_Check{}
 
 	if err := tables.ForEachDescriptor(func(desc catalog.Descriptor) error {
-		tableChecks, err := InspectChecksForTable(ctx, p, desc.(catalog.TableDescriptor))
+		tableChecks, err := ChecksForTable(ctx, p, desc.(catalog.TableDescriptor))
 		if err != nil {
 			return err
 		}
@@ -133,10 +128,10 @@ func InspectChecksForDatabase(
 	return checks, nil
 }
 
-// InspectChecksForTable generates checks on every supported index on the given
+// ChecksForTable generates checks on every supported index on the given
 // table.
-func InspectChecksForTable(
-	ctx context.Context, p PlanHookState, table catalog.TableDescriptor,
+func ChecksForTable(
+	ctx context.Context, p sql.PlanHookState, table catalog.TableDescriptor,
 ) ([]*jobspb.InspectDetails_Check, error) {
 	checks := []*jobspb.InspectDetails_Check{}
 
@@ -165,11 +160,11 @@ type indexKey struct {
 	descpb.IndexID
 }
 
-// InspectChecksByIndexNames generates checks for the specified index names.
+// checksByIndexNames generates checks for the specified index names.
 // If index names are not found or are not supported for inspection, an error is returned.
 // Index names are deduplicated.
-func InspectChecksByIndexNames(
-	ctx context.Context, p PlanHookState, names tree.TableIndexNames,
+func checksByIndexNames(
+	ctx context.Context, p sql.PlanHookState, names tree.TableIndexNames,
 ) ([]*jobspb.InspectDetails_Check, error) {
 	checks := []*jobspb.InspectDetails_Check{}
 

pkg/sql/inspect/inspect_plan.go

@@ -109,13 +109,13 @@ func newInspectRun(
 		// No INDEX options or INDEX ALL specified - inspect all indexes.
 		switch stmt.Typ {
 		case tree.InspectTable:
-			checks, err := sql.InspectChecksForTable(ctx, p, run.table)
+			checks, err := ChecksForTable(ctx, p, run.table)
 			if err != nil {
 				return inspectRun{}, err
 			}
 			run.checks = checks
 		case tree.InspectDatabase:
-			if checks, err := sql.InspectChecksForDatabase(ctx, p, run.db); err != nil {
+			if checks, err := checksForDatabase(ctx, p, run.db); err != nil {
 				return inspectRun{}, err
 			} else {
 				run.checks = checks
@@ -152,7 +152,7 @@ func newInspectRun(
 			}
 		}
 
-		if checks, err := sql.InspectChecksByIndexNames(ctx, p, run.namedIndexes); err != nil {
+		if checks, err := checksByIndexNames(ctx, p, run.namedIndexes); err != nil {
 			return inspectRun{}, err
 		} else {
 			run.checks = checks
@@ -236,7 +236,7 @@ func inspectPlanHook(
 		// the job record creation happens transactionally.
 		plannerTxn := p.InternalSQLTxn()
 
-		sj, err := sql.TriggerInspectJob(ctx, tree.AsString(stmt), p.ExecCfg(), plannerTxn, run.checks, run.asOfTimestamp)
+		sj, err := TriggerJob(ctx, tree.AsString(stmt), p.ExecCfg(), plannerTxn, run.checks, run.asOfTimestamp)
 		if err != nil {
 			return err
 		}