From 1f86fe5ead7ec6740dcba360b0864b8729df9726 Mon Sep 17 00:00:00 2001 From: you06 Date: Tue, 14 Feb 2023 21:00:16 +0800 Subject: [PATCH 1/3] fix race when collecting telemetry info of batch copr Signed-off-by: you06 --- executor/adapter.go | 2 +- executor/builder.go | 6 +++--- session/session.go | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/executor/adapter.go b/executor/adapter.go index 7702f2e05875a..7a4649ed3f8b5 100644 --- a/executor/adapter.go +++ b/executor/adapter.go @@ -219,7 +219,7 @@ type TelemetryInfo struct { PartitionTelemetry *PartitionTelemetryInfo AccountLockTelemetry *AccountLockTelemetryInfo UseIndexMerge bool - UseTableLookUp bool + UseTableLookUp atomic.Bool } // PartitionTelemetryInfo records table partition telemetry information during execution. diff --git a/executor/builder.go b/executor/builder.go index 132ae66272c69..12077153ac5da 100644 --- a/executor/builder.go +++ b/executor/builder.go @@ -3883,7 +3883,7 @@ func buildNoRangeIndexLookUpReader(b *executorBuilder, v *plannercore.PhysicalIn func (b *executorBuilder) buildIndexLookUpReader(v *plannercore.PhysicalIndexLookUpReader) Executor { if b.Ti != nil { - b.Ti.UseTableLookUp = true + b.Ti.UseTableLookUp.Store(true) } is := v.IndexPlans[0].(*plannercore.PhysicalIndexScan) if err := b.validCanReadTemporaryOrCacheTable(is.Table); err != nil { @@ -4022,7 +4022,7 @@ func buildNoRangeIndexMergeReader(b *executorBuilder, v *plannercore.PhysicalInd func (b *executorBuilder) buildIndexMergeReader(v *plannercore.PhysicalIndexMergeReader) Executor { if b.Ti != nil { b.Ti.UseIndexMerge = true - b.Ti.UseTableLookUp = true + b.Ti.UseTableLookUp.Store(true) } ts := v.TablePlans[0].(*plannercore.PhysicalTableScan) if err := b.validCanReadTemporaryOrCacheTable(ts.Table); err != nil { @@ -4469,7 +4469,7 @@ func (builder *dataReaderBuilder) buildIndexReaderForIndexJoin(ctx context.Conte func (builder *dataReaderBuilder) buildIndexLookUpReaderForIndexJoin(ctx context.Context, v *plannercore.PhysicalIndexLookUpReader, lookUpContents []*indexJoinLookUpContent, indexRanges []*ranger.Range, keyOff2IdxOff []int, cwc *plannercore.ColWithCmpFuncManager, memTracker *memory.Tracker, interruptSignal *atomic.Value) (Executor, error) { if builder.Ti != nil { - builder.Ti.UseTableLookUp = true + builder.Ti.UseTableLookUp.Store(true) } e, err := buildNoRangeIndexLookUpReader(builder.executorBuilder, v) if err != nil { diff --git a/session/session.go b/session/session.go index 119e50622c33f..197648842dfed 100644 --- a/session/session.go +++ b/session/session.go @@ -4094,7 +4094,7 @@ func (s *session) updateTelemetryMetric(es *executor.ExecStmt) { telemetryCreateOrAlterUserUsage.Add(float64(ti.AccountLockTelemetry.CreateOrAlterUser)) } - if ti.UseTableLookUp && s.sessionVars.StoreBatchSize > 0 { + if ti.UseTableLookUp.Load() && s.sessionVars.StoreBatchSize > 0 { telemetryStoreBatchedUsage.Inc() } } From 7889b398a3318f0ae7a71d6395c76e82a1d1e970 Mon Sep 17 00:00:00 2001 From: you06 Date: Wed, 15 Feb 2023 11:26:48 +0800 Subject: [PATCH 2/3] add regression test Signed-off-by: you06 --- executor/index_lookup_merge_join_test.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/executor/index_lookup_merge_join_test.go b/executor/index_lookup_merge_join_test.go index 826786d9f196b..8a52b799dc291 100644 --- a/executor/index_lookup_merge_join_test.go +++ b/executor/index_lookup_merge_join_test.go @@ -226,3 +226,16 @@ func TestIssue24473AndIssue25669(t *testing.T) { tk.MustQuery("SELECT /*+ inl_merge_join (t2,t3) */ t2.a,t3.a FROM x t2 inner join x t3 on t2.a = t3.a;").Sort().Check( testkit.Rows("b b", "x x", "x x", "x x", "x x", "y y")) } + +func TestIssue41412(t *testing.T) { + store := testkit.CreateMockStore(t) + tk := testkit.NewTestKit(t, store) + tk.MustExec("use test") + tk.MustExec("drop table if exists t") + tk.MustExec("create table t(a int, b int, c int, index idx_a(a), index idx_b(b))") + for i := 0; i < 2000; i++ { + v := i * 100 + tk.MustExec("insert into t values(?, ?, ?)", v, v, v) + } + tk.MustQuery("select /*+ inl_merge_join(t1, t2) */ * from t t1 right join t t2 on t1.a = t2.b and t1.c = t2.c") +} From 94aed09788820a58ce8499c185e4700a7c2b4052 Mon Sep 17 00:00:00 2001 From: you06 Date: Wed, 15 Feb 2023 12:18:25 +0800 Subject: [PATCH 3/3] move test into executor/issuetest Signed-off-by: you06 --- executor/index_lookup_merge_join_test.go | 13 ------------- executor/issuetest/executor_issue_test.go | 14 ++++++++++++++ 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/executor/index_lookup_merge_join_test.go b/executor/index_lookup_merge_join_test.go index 8a52b799dc291..826786d9f196b 100644 --- a/executor/index_lookup_merge_join_test.go +++ b/executor/index_lookup_merge_join_test.go @@ -226,16 +226,3 @@ func TestIssue24473AndIssue25669(t *testing.T) { tk.MustQuery("SELECT /*+ inl_merge_join (t2,t3) */ t2.a,t3.a FROM x t2 inner join x t3 on t2.a = t3.a;").Sort().Check( testkit.Rows("b b", "x x", "x x", "x x", "x x", "y y")) } - -func TestIssue41412(t *testing.T) { - store := testkit.CreateMockStore(t) - tk := testkit.NewTestKit(t, store) - tk.MustExec("use test") - tk.MustExec("drop table if exists t") - tk.MustExec("create table t(a int, b int, c int, index idx_a(a), index idx_b(b))") - for i := 0; i < 2000; i++ { - v := i * 100 - tk.MustExec("insert into t values(?, ?, ?)", v, v, v) - } - tk.MustQuery("select /*+ inl_merge_join(t1, t2) */ * from t t1 right join t t2 on t1.a = t2.b and t1.c = t2.c") -} diff --git a/executor/issuetest/executor_issue_test.go b/executor/issuetest/executor_issue_test.go index b97df5a5c11c1..8c467bba57073 100644 --- a/executor/issuetest/executor_issue_test.go +++ b/executor/issuetest/executor_issue_test.go @@ -1387,3 +1387,17 @@ PARTITION BY HASH (c5) PARTITIONS 4;`) // Again, a simpler reproduce. tk.MustQuery("select /*+ inl_join (t1, t2) */ t2.c5 from t1 right join t2 on t1.c2 = t2.c5 where not( t1.c2 between '4s7ht' and 'mj' );").Check(testkit.Rows()) } + +func TestIssueRaceWhenBuildingExecutorConcurrently(t *testing.T) { + // issue: https://github.com/pingcap/tidb/issues/41412 + store := testkit.CreateMockStore(t) + tk := testkit.NewTestKit(t, store) + tk.MustExec("use test") + tk.MustExec("drop table if exists t") + tk.MustExec("create table t(a int, b int, c int, index idx_a(a), index idx_b(b))") + for i := 0; i < 2000; i++ { + v := i * 100 + tk.MustExec("insert into t values(?, ?, ?)", v, v, v) + } + tk.MustQuery("select /*+ inl_merge_join(t1, t2) */ * from t t1 right join t t2 on t1.a = t2.b and t1.c = t2.c") +}