From 296914d5acdf90c91db778affa735dbc76bbbf61 Mon Sep 17 00:00:00 2001 From: Weizhen Wang Date: Fri, 13 Jan 2023 16:50:35 +0800 Subject: [PATCH 1/8] statistics: fix data race in the Handle.IsTableLocked Signed-off-by: Weizhen Wang --- statistics/handle/handle.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/statistics/handle/handle.go b/statistics/handle/handle.go index ab900ffc43392..0754fb9f2555a 100644 --- a/statistics/handle/handle.go +++ b/statistics/handle/handle.go @@ -355,6 +355,8 @@ func (h *Handle) RemoveLockedTables(tids []int64, pids []int64, tables []*ast.Ta // IsTableLocked check whether table is locked in handle func (h *Handle) IsTableLocked(tableID int64) bool { + h.mu.RLock() + defer h.mu.RUnlock() return isTableLocked(h.tableLocked, tableID) } From 8733d2e3328aaccc82aea1c219edcf9b86c015dc Mon Sep 17 00:00:00 2001 From: Weizhen Wang Date: Fri, 13 Jan 2023 16:51:50 +0800 Subject: [PATCH 2/8] statistics: fix data race in the Handle.IsTableLocked Signed-off-by: Weizhen Wang --- statistics/handle/BUILD.bazel | 1 + 1 file changed, 1 insertion(+) diff --git a/statistics/handle/BUILD.bazel b/statistics/handle/BUILD.bazel index 81dff92c5b143..edfd062509fda 100644 --- a/statistics/handle/BUILD.bazel +++ b/statistics/handle/BUILD.bazel @@ -72,6 +72,7 @@ go_test( ], embed = [":handle"], flaky = True, + race = "on", shard_count = 50, deps = [ "//config", From f4f3154db29fa69294c0870e1cb24647f8b93150 Mon Sep 17 00:00:00 2001 From: Weizhen Wang Date: Fri, 13 Jan 2023 16:58:28 +0800 Subject: [PATCH 3/8] statistics: fix data race in the Handle.IsTableLocked Signed-off-by: Weizhen Wang --- statistics/handle/BUILD.bazel | 1 + statistics/handle/handle.go | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/statistics/handle/BUILD.bazel b/statistics/handle/BUILD.bazel index edfd062509fda..d52847495d539 100644 --- a/statistics/handle/BUILD.bazel +++ b/statistics/handle/BUILD.bazel @@ -42,6 +42,7 @@ go_library( "//util/memory", "//util/ranger", "//util/sqlexec", + "//util/syncutil", "//util/timeutil", "@com_github_ngaut_pools//:pools", "@com_github_pingcap_errors//:errors", diff --git a/statistics/handle/handle.go b/statistics/handle/handle.go index 0754fb9f2555a..f4463f25aac87 100644 --- a/statistics/handle/handle.go +++ b/statistics/handle/handle.go @@ -48,6 +48,7 @@ import ( "github.com/pingcap/tidb/util/mathutil" "github.com/pingcap/tidb/util/memory" "github.com/pingcap/tidb/util/sqlexec" + "github.com/pingcap/tidb/util/syncutil" "github.com/prometheus/client_golang/prometheus" "github.com/tikv/client-go/v2/oracle" atomic2 "go.uber.org/atomic" @@ -66,7 +67,7 @@ const ( // Handle can update stats info periodically. type Handle struct { mu struct { - sync.RWMutex + syncutil.RWMutex ctx sessionctx.Context // rateMap contains the error rate delta from feedback. rateMap errorRateDeltaMap From 37f5b50b4e285bac9dc8ef7398e6b605fd062702 Mon Sep 17 00:00:00 2001 From: Weizhen Wang Date: Fri, 13 Jan 2023 17:37:11 +0800 Subject: [PATCH 4/8] update Signed-off-by: Weizhen Wang --- executor/analyze.go | 4 ++-- executor/show_stats.go | 6 +++--- statistics/handle/handle.go | 8 +++++--- statistics/handle/update.go | 6 +++--- 4 files changed, 13 insertions(+), 11 deletions(-) diff --git a/executor/analyze.go b/executor/analyze.go index 705e6eed6c590..8e832d4fd7d8e 100644 --- a/executor/analyze.go +++ b/executor/analyze.go @@ -102,7 +102,7 @@ func (e *AnalyzeExec) Next(ctx context.Context, _ *chunk.Chunk) error { tableID = task.idxIncrementalExec.tableID } // skip locked tables - if !statsHandle.IsTableLocked(tableID.TableID) { + if !statsHandle.IsTableLocked(tableID.TableID, true) { tasks = append(tasks, task) } // generate warning message @@ -115,7 +115,7 @@ func (e *AnalyzeExec) Next(ctx context.Context, _ *chunk.Chunk) error { } //avoid generate duplicate tables if !dup { - if statsHandle.IsTableLocked(tableID.TableID) { + if statsHandle.IsTableLocked(tableID.TableID, true) { tbl, ok := is.TableByID(tableID.TableID) if !ok { return nil diff --git a/executor/show_stats.go b/executor/show_stats.go index 6161a173e8fe4..c70c0eda1ec36 100644 --- a/executor/show_stats.go +++ b/executor/show_stats.go @@ -164,19 +164,19 @@ func (e *ShowExec) fetchShowStatsLocked() error { if pi != nil { partitionName = "global" } - if h.IsTableLocked(tbl.ID) { + if h.IsTableLocked(tbl.ID, true) { e.appendTableForStatsLocked(db.Name.O, tbl.Name.O, partitionName) } if pi != nil { for _, def := range pi.Definitions { - if h.IsTableLocked(def.ID) { + if h.IsTableLocked(def.ID, true) { e.appendTableForStatsLocked(db.Name.O, tbl.Name.O, def.Name.O) } } } } else { for _, def := range pi.Definitions { - if h.IsTableLocked(def.ID) { + if h.IsTableLocked(def.ID, true) { e.appendTableForStatsLocked(db.Name.O, tbl.Name.O, def.Name.O) } } diff --git a/statistics/handle/handle.go b/statistics/handle/handle.go index f4463f25aac87..c7229336b3123 100644 --- a/statistics/handle/handle.go +++ b/statistics/handle/handle.go @@ -355,9 +355,11 @@ func (h *Handle) RemoveLockedTables(tids []int64, pids []int64, tables []*ast.Ta } // IsTableLocked check whether table is locked in handle -func (h *Handle) IsTableLocked(tableID int64) bool { - h.mu.RLock() - defer h.mu.RUnlock() +func (h *Handle) IsTableLocked(tableID int64, lock bool) bool { + if lock { + h.mu.RLock() + defer h.mu.RUnlock() + } return isTableLocked(h.tableLocked, tableID) } diff --git a/statistics/handle/update.go b/statistics/handle/update.go index 68aa9cebbcf05..17f77bbe029bd 100644 --- a/statistics/handle/update.go +++ b/statistics/handle/update.go @@ -198,7 +198,7 @@ func (s *SessionStatsCollector) StoreQueryFeedback(feedback interface{}, h *Hand } // if table locked, skip - if h.IsTableLocked(q.PhysicalID) { + if h.IsTableLocked(q.PhysicalID, true) { return nil } @@ -549,7 +549,7 @@ func (h *Handle) dumpTableStatCountToKV(id int64, delta variable.TableDelta) (up startTS := txn.StartTS() updateStatsMeta := func(id int64) error { var err error - if h.IsTableLocked(id) { + if h.IsTableLocked(id, false) { if delta.Delta < 0 { _, err = exec.ExecuteInternal(ctx, "update mysql.stats_table_locked set version = %?, count = count - %?, modify_count = modify_count + %? where table_id = %? and count >= %?", startTS, -delta.Delta, delta.Count, id, -delta.Delta) } else { @@ -1110,7 +1110,7 @@ func (h *Handle) HandleAutoAnalyze(is infoschema.InfoSchema) (analyzed bool) { }) for _, tbl := range tbls { //if table locked, skip analyze - if h.IsTableLocked(tbl.Meta().ID) { + if h.IsTableLocked(tbl.Meta().ID, true) { continue } tblInfo := tbl.Meta() From 37351d2c908a9d7657d821d7934e8a02122ba048 Mon Sep 17 00:00:00 2001 From: Weizhen Wang Date: Mon, 16 Jan 2023 10:32:17 +0800 Subject: [PATCH 5/8] tidb: not use DEFAULT if columns have not default value Signed-off-by: Weizhen Wang --- statistics/handle/handle.go | 4 ++-- statistics/handle/update.go | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/statistics/handle/handle.go b/statistics/handle/handle.go index c7229336b3123..e567e88515b14 100644 --- a/statistics/handle/handle.go +++ b/statistics/handle/handle.go @@ -355,8 +355,8 @@ func (h *Handle) RemoveLockedTables(tids []int64, pids []int64, tables []*ast.Ta } // IsTableLocked check whether table is locked in handle -func (h *Handle) IsTableLocked(tableID int64, lock bool) bool { - if lock { +func (h *Handle) IsTableLocked(tableID int64, isLock bool) bool { + if isLock { h.mu.RLock() defer h.mu.RUnlock() } diff --git a/statistics/handle/update.go b/statistics/handle/update.go index 17f77bbe029bd..6ea489fbc042d 100644 --- a/statistics/handle/update.go +++ b/statistics/handle/update.go @@ -549,6 +549,7 @@ func (h *Handle) dumpTableStatCountToKV(id int64, delta variable.TableDelta) (up startTS := txn.StartTS() updateStatsMeta := func(id int64) error { var err error + // This lock is already locked on it so it's isLock is set to false. if h.IsTableLocked(id, false) { if delta.Delta < 0 { _, err = exec.ExecuteInternal(ctx, "update mysql.stats_table_locked set version = %?, count = count - %?, modify_count = modify_count + %? where table_id = %? and count >= %?", startTS, -delta.Delta, delta.Count, id, -delta.Delta) From 61cf7683a9380feb7b1f40903083a1a40e28866c Mon Sep 17 00:00:00 2001 From: Weizhen Wang Date: Mon, 16 Jan 2023 15:58:33 +0800 Subject: [PATCH 6/8] update Signed-off-by: Weizhen Wang --- executor/analyze.go | 4 ++-- executor/show_stats.go | 6 +++--- statistics/handle/handle.go | 13 ++++++++----- statistics/handle/update.go | 8 ++++---- 4 files changed, 17 insertions(+), 14 deletions(-) diff --git a/executor/analyze.go b/executor/analyze.go index 8e832d4fd7d8e..705e6eed6c590 100644 --- a/executor/analyze.go +++ b/executor/analyze.go @@ -102,7 +102,7 @@ func (e *AnalyzeExec) Next(ctx context.Context, _ *chunk.Chunk) error { tableID = task.idxIncrementalExec.tableID } // skip locked tables - if !statsHandle.IsTableLocked(tableID.TableID, true) { + if !statsHandle.IsTableLocked(tableID.TableID) { tasks = append(tasks, task) } // generate warning message @@ -115,7 +115,7 @@ func (e *AnalyzeExec) Next(ctx context.Context, _ *chunk.Chunk) error { } //avoid generate duplicate tables if !dup { - if statsHandle.IsTableLocked(tableID.TableID, true) { + if statsHandle.IsTableLocked(tableID.TableID) { tbl, ok := is.TableByID(tableID.TableID) if !ok { return nil diff --git a/executor/show_stats.go b/executor/show_stats.go index c70c0eda1ec36..6161a173e8fe4 100644 --- a/executor/show_stats.go +++ b/executor/show_stats.go @@ -164,19 +164,19 @@ func (e *ShowExec) fetchShowStatsLocked() error { if pi != nil { partitionName = "global" } - if h.IsTableLocked(tbl.ID, true) { + if h.IsTableLocked(tbl.ID) { e.appendTableForStatsLocked(db.Name.O, tbl.Name.O, partitionName) } if pi != nil { for _, def := range pi.Definitions { - if h.IsTableLocked(def.ID, true) { + if h.IsTableLocked(def.ID) { e.appendTableForStatsLocked(db.Name.O, tbl.Name.O, def.Name.O) } } } } else { for _, def := range pi.Definitions { - if h.IsTableLocked(def.ID, true) { + if h.IsTableLocked(def.ID) { e.appendTableForStatsLocked(db.Name.O, tbl.Name.O, def.Name.O) } } diff --git a/statistics/handle/handle.go b/statistics/handle/handle.go index e567e88515b14..0ec6d0ed1ea7f 100644 --- a/statistics/handle/handle.go +++ b/statistics/handle/handle.go @@ -355,11 +355,14 @@ func (h *Handle) RemoveLockedTables(tids []int64, pids []int64, tables []*ast.Ta } // IsTableLocked check whether table is locked in handle -func (h *Handle) IsTableLocked(tableID int64, isLock bool) bool { - if isLock { - h.mu.RLock() - defer h.mu.RUnlock() - } +func (h *Handle) IsTableLocked(tableID int64) bool { + h.mu.RLock() + defer h.mu.RUnlock() + return h.isTableLocked(tableID) +} + +// IsTableLocked check whether table is locked in handle +func (h *Handle) isTableLocked(tableID int64) bool { return isTableLocked(h.tableLocked, tableID) } diff --git a/statistics/handle/update.go b/statistics/handle/update.go index 6ea489fbc042d..e245a3ea0bca5 100644 --- a/statistics/handle/update.go +++ b/statistics/handle/update.go @@ -198,7 +198,7 @@ func (s *SessionStatsCollector) StoreQueryFeedback(feedback interface{}, h *Hand } // if table locked, skip - if h.IsTableLocked(q.PhysicalID, true) { + if h.IsTableLocked(q.PhysicalID) { return nil } @@ -549,8 +549,8 @@ func (h *Handle) dumpTableStatCountToKV(id int64, delta variable.TableDelta) (up startTS := txn.StartTS() updateStatsMeta := func(id int64) error { var err error - // This lock is already locked on it so it's isLock is set to false. - if h.IsTableLocked(id, false) { + // This lock is already locked on it so it use isTableLocked without lock. + if h.isTableLocked(id) { if delta.Delta < 0 { _, err = exec.ExecuteInternal(ctx, "update mysql.stats_table_locked set version = %?, count = count - %?, modify_count = modify_count + %? where table_id = %? and count >= %?", startTS, -delta.Delta, delta.Count, id, -delta.Delta) } else { @@ -1111,7 +1111,7 @@ func (h *Handle) HandleAutoAnalyze(is infoschema.InfoSchema) (analyzed bool) { }) for _, tbl := range tbls { //if table locked, skip analyze - if h.IsTableLocked(tbl.Meta().ID, true) { + if h.IsTableLocked(tbl.Meta().ID) { continue } tblInfo := tbl.Meta() From e5585cf11e44417975bd33b23e528c2d3e3277dd Mon Sep 17 00:00:00 2001 From: Weizhen Wang Date: Mon, 16 Jan 2023 15:59:51 +0800 Subject: [PATCH 7/8] update Signed-off-by: Weizhen Wang --- statistics/handle/handle.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/statistics/handle/handle.go b/statistics/handle/handle.go index 0ec6d0ed1ea7f..11f47953cf03a 100644 --- a/statistics/handle/handle.go +++ b/statistics/handle/handle.go @@ -361,12 +361,12 @@ func (h *Handle) IsTableLocked(tableID int64) bool { return h.isTableLocked(tableID) } -// IsTableLocked check whether table is locked in handle +// IsTableLocked check whether table is locked in handle with Handle.Mutex func (h *Handle) isTableLocked(tableID int64) bool { return isTableLocked(h.tableLocked, tableID) } -// isTableLocked check whether table is locked +// isTableLocked check whether table is locked without Handle.Mutex func isTableLocked(tableLocked []int64, tableID int64) bool { return lockTableIndexOf(tableLocked, tableID) > -1 } From 09825743f3ae93ffdf7b0273565aef6d348dd7cf Mon Sep 17 00:00:00 2001 From: Weizhen Wang Date: Mon, 16 Jan 2023 16:00:19 +0800 Subject: [PATCH 8/8] update Signed-off-by: Weizhen Wang --- statistics/handle/handle.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/statistics/handle/handle.go b/statistics/handle/handle.go index 11f47953cf03a..d40aceb44e977 100644 --- a/statistics/handle/handle.go +++ b/statistics/handle/handle.go @@ -354,19 +354,19 @@ func (h *Handle) RemoveLockedTables(tids []int64, pids []int64, tables []*ast.Ta return "", err } -// IsTableLocked check whether table is locked in handle +// IsTableLocked check whether table is locked in handle with Handle.Mutex func (h *Handle) IsTableLocked(tableID int64) bool { h.mu.RLock() defer h.mu.RUnlock() return h.isTableLocked(tableID) } -// IsTableLocked check whether table is locked in handle with Handle.Mutex +// IsTableLocked check whether table is locked in handle without Handle.Mutex func (h *Handle) isTableLocked(tableID int64) bool { return isTableLocked(h.tableLocked, tableID) } -// isTableLocked check whether table is locked without Handle.Mutex +// isTableLocked check whether table is locked func isTableLocked(tableLocked []int64, tableID int64) bool { return lockTableIndexOf(tableLocked, tableID) > -1 }