From 85ce24db3680468c8ef0073fddaed39b35e5b567 Mon Sep 17 00:00:00 2001 From: Andrew Werner Date: Sat, 30 Jul 2022 13:14:10 -0400 Subject: [PATCH] sql,logictest: use deterministic descriptor ID generation in logctests This commit adds a testing knob which results in descriptor ID generation being handled transactionally. The logictests now use this knob. Fixes #37751 Fixes #69226 Release note: None --- pkg/sql/BUILD.bazel | 1 + pkg/sql/conn_executor.go | 11 ++++++++++- pkg/sql/exec_util.go | 5 +++++ pkg/sql/logictest/logic.go | 3 ++- pkg/sql/mvcc_backfiller_test.go | 1 + 5 files changed, 19 insertions(+), 2 deletions(-) diff --git a/pkg/sql/BUILD.bazel b/pkg/sql/BUILD.bazel index 749b2c3ef1ab..fe2dfbcc783b 100644 --- a/pkg/sql/BUILD.bazel +++ b/pkg/sql/BUILD.bazel @@ -312,6 +312,7 @@ go_library( "//pkg/sql/catalog/colinfo", "//pkg/sql/catalog/dbdesc", "//pkg/sql/catalog/descbuilder", + "//pkg/sql/catalog/descidgen", "//pkg/sql/catalog/descpb", "//pkg/sql/catalog/descs", "//pkg/sql/catalog/funcdesc", diff --git a/pkg/sql/conn_executor.go b/pkg/sql/conn_executor.go index 0dfe2203b572..b6c2e27a4871 100644 --- a/pkg/sql/conn_executor.go +++ b/pkg/sql/conn_executor.go @@ -31,6 +31,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/server/telemetry" "github.com/cockroachdb/cockroach/pkg/sql/catalog" "github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/descidgen" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descs" "github.com/cockroachdb/cockroach/pkg/sql/clusterunique" @@ -2715,7 +2716,7 @@ func (ex *connExecutor) initEvalCtx(ctx context.Context, evalCtx *extendedEvalCo StmtDiagnosticsRequestInserter: ex.server.cfg.StmtDiagnosticsRecorder.InsertRequest, CatalogBuiltins: &p.evalCatalogBuiltins, QueryCancelKey: ex.queryCancelKey, - DescIDGenerator: ex.server.cfg.DescIDGenerator, + DescIDGenerator: ex.getDescIDGenerator(), }, Tracing: &ex.sessionTracing, MemMetrics: &ex.memMetrics, @@ -3273,6 +3274,14 @@ func (ex *connExecutor) runPreCommitStages(ctx context.Context) error { return nil } +func (ex *connExecutor) getDescIDGenerator() eval.DescIDGenerator { + if ex.server.cfg.TestingKnobs.UseTransactionalDescIDGenerator && + ex.state.mu.txn != nil { + return descidgen.NewTransactionalGenerator(ex.server.cfg.Codec, ex.state.mu.txn) + } + return ex.server.cfg.DescIDGenerator +} + // StatementCounters groups metrics for counting different types of // statements. type StatementCounters struct { diff --git a/pkg/sql/exec_util.go b/pkg/sql/exec_util.go index 6caf5b04a607..46b961dca2ce 100644 --- a/pkg/sql/exec_util.go +++ b/pkg/sql/exec_util.go @@ -1470,6 +1470,11 @@ type ExecutorTestingKnobs struct { // OnRecordTxnFinish, if set, will be called as we record a transaction // finishing. OnRecordTxnFinish func(isInternal bool, phaseTimes *sessionphase.Times, stmt string) + + // UseTransactionDescIDGenerator is used to force descriptor ID generation + // to use a transaction, and, in doing so, more deterministically allocate + // descriptor IDs at the cost of decreased parallelism. + UseTransactionalDescIDGenerator bool } // PGWireTestingKnobs contains knobs for the pgwire module. diff --git a/pkg/sql/logictest/logic.go b/pkg/sql/logictest/logic.go index 7a41b3e66753..38b2d051047a 100644 --- a/pkg/sql/logictest/logic.go +++ b/pkg/sql/logictest/logic.go @@ -1171,7 +1171,8 @@ func (t *logicTest) newCluster( ForceProductionValues: serverArgs.ForceProductionValues, }, SQLExecutor: &sql.ExecutorTestingKnobs{ - DeterministicExplain: true, + DeterministicExplain: true, + UseTransactionalDescIDGenerator: true, }, SQLStatsKnobs: &sqlstats.TestingKnobs{ AOSTClause: "AS OF SYSTEM TIME '-1us'", diff --git a/pkg/sql/mvcc_backfiller_test.go b/pkg/sql/mvcc_backfiller_test.go index 0a425b569b69..1187c081f4b6 100644 --- a/pkg/sql/mvcc_backfiller_test.go +++ b/pkg/sql/mvcc_backfiller_test.go @@ -57,6 +57,7 @@ func TestIndexBackfillMergeRetry(t *testing.T) { defer log.Scope(t).Close(t) skip.UnderStressRace(t, "TODO(ssd) test times outs under race") + skip.UnderRace(t, "TODO(ssd) test times outs under race") params, _ := tests.CreateTestServerParams()