Merge #89540

89540: sql: avoid starvation of validation after backfill r=fqazi a=fqazi Fixes: #84911 Previously, after a backfill queries inside validateIndexes could be starved by GC jobs due to the lack of a protected time stamp. Hence it was possible that these queries could run into retry errors. To avoid these errors we can setup a protected timestamp, so that the GC job doesn't interfere with these queries. Release note (bug fix): Index validation could be starved if it took longer then GC jobs for a given table. Protected timestamps are now created during index validation. Co-authored-by: Faizan Qazi <[email protected]>
cockroachdb · Oct 28, 2022 · d39558f · d39558f
2 parents 6fba541 + ccf9205
commit d39558f
Show file tree

Hide file tree

Showing 12 changed files with 440 additions and 43 deletions.
diff --git a/pkg/ccl/backupccl/restore_job.go b/pkg/ccl/backupccl/restore_job.go
@@ -1774,12 +1774,12 @@ func revalidateIndexes(
 
 	// We don't actually need the 'historical' read the way the schema change does
 	// since our table is offline.
-	var runner descs.HistoricalInternalExecTxnRunner = func(ctx context.Context, fn descs.InternalExecFn) error {
+	runner := descs.NewHistoricalInternalExecTxnRunner(hlc.Timestamp{}, func(ctx context.Context, fn descs.InternalExecFn) error {
 		return execCfg.DB.Txn(ctx, func(ctx context.Context, txn *kv.Txn) error {
 			ie := job.MakeSessionBoundInternalExecutor(sql.NewFakeSessionData(execCfg.SV())).(*sql.InternalExecutor)
 			return fn(ctx, txn, ie, nil /* descriptors */)
 		})
-	}
+	})
 
 	invalidIndexes := make(map[descpb.ID][]descpb.IndexID)
 
@@ -1804,12 +1804,17 @@ func revalidateIndexes(
 		if len(forward) > 0 {
 			if err := sql.ValidateForwardIndexes(
 				ctx,
+				job.ID(),
+				execCfg.Codec,
+				execCfg.DB,
 				tableDesc.MakePublic(),
 				forward,
 				runner,
 				false, /* withFirstMutationPublic */
 				true,  /* gatherAllInvalid */
 				sessiondata.InternalExecutorOverride{},
+				execCfg.ProtectedTimestampProvider,
+				execCfg.SystemConfig,
 			); err != nil {
 				if invalid := (sql.InvalidIndexesError{}); errors.As(err, &invalid) {
 					invalidIndexes[tableDesc.ID] = invalid.Indexes
@@ -1821,13 +1826,17 @@ func revalidateIndexes(
 		if len(inverted) > 0 {
 			if err := sql.ValidateInvertedIndexes(
 				ctx,
+				job.ID(),
 				execCfg.Codec,
+				execCfg.DB,
 				tableDesc.MakePublic(),
 				inverted,
 				runner,
 				false, /* withFirstMutationPublic */
 				true,  /* gatherAllInvalid */
 				sessiondata.InternalExecutorOverride{},
+				execCfg.ProtectedTimestampProvider,
+				execCfg.SystemConfig,
 			); err != nil {
 				if invalid := (sql.InvalidIndexesError{}); errors.As(err, &invalid) {
 					invalidIndexes[tableDesc.ID] = append(invalidIndexes[tableDesc.ID], invalid.Indexes...)

diff --git a/pkg/server/server_sql.go b/pkg/server/server_sql.go
@@ -1025,6 +1025,8 @@ func newSQLServer(ctx context.Context, cfg sqlServerArgs) (*SQLServer, error) {
 		execCfg.Codec,
 		execCfg.Settings,
 		ieFactory,
+		execCfg.ProtectedTimestampProvider,
+		execCfg.SystemConfig,
 		sql.ValidateForwardIndexes,
 		sql.ValidateInvertedIndexes,
 		sql.ValidateCheckConstraint,

diff --git a/pkg/sql/BUILD.bazel b/pkg/sql/BUILD.bazel
@@ -282,6 +282,7 @@ go_library(
         "//pkg/gossip",
         "//pkg/jobs",
         "//pkg/jobs/jobspb",
+        "//pkg/jobs/jobsprotectedts",
         "//pkg/keys",
         "//pkg/kv",
         "//pkg/kv/kvclient",
@@ -293,6 +294,7 @@ go_library(
         "//pkg/kv/kvserver/kvserverbase",
         "//pkg/kv/kvserver/liveness/livenesspb",
         "//pkg/kv/kvserver/protectedts",
+        "//pkg/kv/kvserver/protectedts/ptpb",
         "//pkg/multitenant",
         "//pkg/obs",
         "//pkg/obsservice/obspb",
@@ -528,6 +530,7 @@ go_test(
         "as_of_test.go",
         "authorization_test.go",
         "backfill_num_ranges_in_span_test.go",
+        "backfill_protected_timestamp_test.go",
         "backfill_test.go",
         "builtin_mem_usage_test.go",
         "builtin_test.go",
@@ -665,6 +668,7 @@ go_test(
         "//pkg/kv/kvclient/rangefeed",
         "//pkg/kv/kvserver",
         "//pkg/kv/kvserver/kvserverbase",
+        "//pkg/kv/kvserver/protectedts",
         "//pkg/roachpb",
         "//pkg/rpc",
         "//pkg/rpc/nodedialer",
@@ -677,6 +681,7 @@ go_test(
         "//pkg/server/status/statuspb",
         "//pkg/server/telemetry",
         "//pkg/settings/cluster",
+        "//pkg/spanconfig/spanconfigptsreader",
         "//pkg/sql/backfill",
         "//pkg/sql/catalog",
         "//pkg/sql/catalog/bootstrap",