diff --git a/domain/global_vars_cache.go b/domain/global_vars_cache.go index 93218b7546ce9..68a8863a745ee 100644 --- a/domain/global_vars_cache.go +++ b/domain/global_vars_cache.go @@ -83,7 +83,7 @@ func checkEnableStmtSummary(rows []chunk.Row, fields []*ast.ResultField) { } } - stmtsummary.StmtSummaryByDigestMap.SetEnabled(sVal, false) + stmtsummary.OnEnableStmtSummaryModified(sVal) break } } diff --git a/domain/global_vars_cache_test.go b/domain/global_vars_cache_test.go index f3e3d7d654bae..455cb30fcc28d 100644 --- a/domain/global_vars_cache_test.go +++ b/domain/global_vars_cache_test.go @@ -14,6 +14,7 @@ package domain import ( + "sync/atomic" "time" . "github.com/pingcap/check" @@ -25,7 +26,6 @@ import ( "github.com/pingcap/tidb/store/mockstore" "github.com/pingcap/tidb/types" "github.com/pingcap/tidb/util/chunk" - "github.com/pingcap/tidb/util/stmtsummary" "github.com/pingcap/tidb/util/testleak" ) @@ -127,18 +127,18 @@ func (gvcSuite *testGVCSuite) TestCheckEnableStmtSummary(c *C) { Collate: charset.CollationBin, } - stmtsummary.StmtSummaryByDigestMap.SetEnabled("0", false) + atomic.StoreInt32(&variable.EnableStmtSummary, 0) ck := chunk.NewChunkWithCapacity([]*types.FieldType{ft, ft1}, 1024) ck.AppendString(0, variable.TiDBEnableStmtSummary) ck.AppendString(1, "1") row := ck.GetRow(0) gvc.Update([]chunk.Row{row}, []*ast.ResultField{rf, rf1}) - c.Assert(stmtsummary.StmtSummaryByDigestMap.Enabled(), Equals, true) + c.Assert(atomic.LoadInt32(&variable.EnableStmtSummary), Equals, int32(1)) ck = chunk.NewChunkWithCapacity([]*types.FieldType{ft, ft1}, 1024) ck.AppendString(0, variable.TiDBEnableStmtSummary) ck.AppendString(1, "0") row = ck.GetRow(0) gvc.Update([]chunk.Row{row}, []*ast.ResultField{rf, rf1}) - c.Assert(stmtsummary.StmtSummaryByDigestMap.Enabled(), Equals, false) + c.Assert(atomic.LoadInt32(&variable.EnableStmtSummary), Equals, int32(0)) } diff --git a/executor/adapter.go b/executor/adapter.go index 758ea8d0ef3a7..f071c7c99f821 100644 --- a/executor/adapter.go +++ b/executor/adapter.go @@ -725,7 +725,7 @@ func (a *ExecStmt) LogSlowQuery(txnTS uint64, succ bool, hasMoreResults bool) { // SummaryStmt collects statements for performance_schema.events_statements_summary_by_digest func (a *ExecStmt) SummaryStmt() { sessVars := a.Ctx.GetSessionVars() - if sessVars.InRestrictedSQL || !stmtsummary.StmtSummaryByDigestMap.Enabled() { + if sessVars.InRestrictedSQL || atomic.LoadInt32(&variable.EnableStmtSummary) == 0 { return } stmtCtx := sessVars.StmtCtx diff --git a/executor/set.go b/executor/set.go index cabfc6e3fd1a2..a80b8dcba42e5 100644 --- a/executor/set.go +++ b/executor/set.go @@ -30,7 +30,6 @@ import ( "github.com/pingcap/tidb/util/chunk" "github.com/pingcap/tidb/util/gcutil" "github.com/pingcap/tidb/util/logutil" - "github.com/pingcap/tidb/util/stmtsummary" "go.uber.org/zap" ) @@ -120,7 +119,6 @@ func (e *SetExecutor) setSysVariable(name string, v *expression.VarAssignment) e if sysVar.Scope == variable.ScopeNone { return errors.Errorf("Variable '%s' is a read only variable", name) } - var valStr string if v.IsGlobal { // Set global scope system variable. if sysVar.Scope&variable.ScopeGlobal == 0 { @@ -133,18 +131,18 @@ func (e *SetExecutor) setSysVariable(name string, v *expression.VarAssignment) e if value.IsNull() { value.SetString("") } - valStr, err = value.ToString() + svalue, err := value.ToString() if err != nil { return err } - err = sessionVars.GlobalVarsAccessor.SetGlobalSysVar(name, valStr) + err = sessionVars.GlobalVarsAccessor.SetGlobalSysVar(name, svalue) if err != nil { return err } err = plugin.ForeachPlugin(plugin.Audit, func(p *plugin.Plugin) error { auditPlugin := plugin.DeclareAuditManifest(p.Manifest) if auditPlugin.OnGlobalVariableEvent != nil { - auditPlugin.OnGlobalVariableEvent(context.Background(), e.ctx.GetSessionVars(), name, valStr) + auditPlugin.OnGlobalVariableEvent(context.Background(), e.ctx.GetSessionVars(), name, svalue) } return nil }) @@ -181,6 +179,7 @@ func (e *SetExecutor) setSysVariable(name string, v *expression.VarAssignment) e sessionVars.SnapshotTS = oldSnapshotTS return err } + var valStr string if value.IsNull() { valStr = "NULL" } else { @@ -191,10 +190,6 @@ func (e *SetExecutor) setSysVariable(name string, v *expression.VarAssignment) e logutil.Logger(context.Background()).Info("set session var", zap.Uint64("conn", sessionVars.ConnectionID), zap.String("name", name), zap.String("val", valStr)) } - if name == variable.TiDBEnableStmtSummary { - stmtsummary.StmtSummaryByDigestMap.SetEnabled(valStr, !v.IsGlobal) - } - return nil } diff --git a/infoschema/perfschema/tables_test.go b/infoschema/perfschema/tables_test.go index b344a1fad9ec3..5f7d044431570 100644 --- a/infoschema/perfschema/tables_test.go +++ b/infoschema/perfschema/tables_test.go @@ -109,7 +109,7 @@ func (s *testTableSuite) TestStmtSummaryTable(c *C) { ).Check(testkit.Rows("test 4 4 insert into t values(1, 'a')")) // Disable it again - tk.MustExec("set global tidb_enable_stmt_summary = false") + tk.MustExec("set global tidb_enable_stmt_summary = 0") tk.MustQuery("select @@global.tidb_enable_stmt_summary").Check(testkit.Rows("0")) // Create a new session to test @@ -122,36 +122,4 @@ func (s *testTableSuite) TestStmtSummaryTable(c *C) { tk.MustQuery(`select schema_name, exec_count, sum_rows_affected, query_sample_text from performance_schema.events_statements_summary_by_digest`, ).Check(testkit.Rows()) - - // Enable it in session scope - tk.MustExec("set session tidb_enable_stmt_summary = on") - // It should work immediately - tk.MustQuery("select * from t where a=2") - tk.MustQuery(`select schema_name, exec_count, sum_rows_affected, query_sample_text - from performance_schema.events_statements_summary_by_digest - where digest_text like 'select * from t%'`, - ).Check(testkit.Rows("test 1 0 select * from t where a=2")) - - // Disable it in global scope - tk.MustExec("set global tidb_enable_stmt_summary = off") - - // Create a new session to test - tk = testkit.NewTestKitWithInit(c, s.store) - - tk.MustQuery("select * from t where a=2") - - // Statement summary is still enabled - tk.MustQuery(`select schema_name, exec_count, sum_rows_affected, query_sample_text - from performance_schema.events_statements_summary_by_digest - where digest_text like 'select * from t%'`, - ).Check(testkit.Rows("test 2 0 select * from t where a=2")) - - // Unset session variable - tk.MustExec("set session tidb_enable_stmt_summary = ''") - tk.MustQuery("select * from t where a=2") - - // Statement summary is disabled - tk.MustQuery(`select schema_name, exec_count, sum_rows_affected, query_sample_text - from performance_schema.events_statements_summary_by_digest`, - ).Check(testkit.Rows()) } diff --git a/sessionctx/variable/sysvar.go b/sessionctx/variable/sysvar.go index dbecc46d744fa..56f76bdff62d3 100644 --- a/sessionctx/variable/sysvar.go +++ b/sessionctx/variable/sysvar.go @@ -710,7 +710,7 @@ var defaultSysVars = []*SysVar{ {ScopeSession, TiDBLowResolutionTSO, "0"}, {ScopeSession, TiDBExpensiveQueryTimeThreshold, strconv.Itoa(DefTiDBExpensiveQueryTimeThreshold)}, {ScopeSession, TiDBAllowRemoveAutoInc, BoolToIntStr(DefTiDBAllowRemoveAutoInc)}, - {ScopeGlobal | ScopeSession, TiDBEnableStmtSummary, "0"}, + {ScopeGlobal, TiDBEnableStmtSummary, BoolToIntStr(DefTiDBEnableStmtSummary)}, } // SynonymsSysVariables is synonyms of system variables. diff --git a/sessionctx/variable/tidb_vars.go b/sessionctx/variable/tidb_vars.go index eaa34a3e06f05..e47a1fbdf0798 100644 --- a/sessionctx/variable/tidb_vars.go +++ b/sessionctx/variable/tidb_vars.go @@ -351,6 +351,7 @@ const ( DefTiDBExpensiveQueryTimeThreshold = 60 // 60s DefWaitSplitRegionTimeout = 300 // 300s DefTiDBAllowRemoveAutoInc = false + DefTiDBEnableStmtSummary = false ) // Process global variables. @@ -370,4 +371,5 @@ var ( MaxOfMaxAllowedPacket uint64 = 1073741824 ExpensiveQueryTimeThreshold uint64 = DefTiDBExpensiveQueryTimeThreshold MinExpensiveQueryTimeThreshold uint64 = 10 //10s + EnableStmtSummary int32 = BoolToInt32(DefTiDBEnableStmtSummary) ) diff --git a/sessionctx/variable/varsutil.go b/sessionctx/variable/varsutil.go index a4fdf5c439a17..7b029c46df481 100644 --- a/sessionctx/variable/varsutil.go +++ b/sessionctx/variable/varsutil.go @@ -420,7 +420,7 @@ func ValidateSetSystemVar(vars *SessionVars, name string, value string) (string, TiDBOptInSubqToJoinAndAgg, TiDBEnableFastAnalyze, TiDBBatchInsert, TiDBDisableTxnAutoRetry, TiDBEnableStreaming, TiDBBatchDelete, TiDBBatchCommit, TiDBEnableCascadesPlanner, TiDBEnableWindowFunction, - TiDBCheckMb4ValueInUTF8, TiDBLowResolutionTSO, TiDBScatterRegion: + TiDBCheckMb4ValueInUTF8, TiDBLowResolutionTSO, TiDBScatterRegion, TiDBEnableStmtSummary: if strings.EqualFold(value, "ON") || value == "1" || strings.EqualFold(value, "OFF") || value == "0" { return value, nil } @@ -577,16 +577,6 @@ func ValidateSetSystemVar(vars *SessionVars, name string, value string) (string, return "off", nil } return value, ErrWrongValueForVar.GenWithStackByArgs(name, value) - case TiDBEnableStmtSummary: - switch { - case strings.EqualFold(value, "ON") || value == "1": - return "1", nil - case strings.EqualFold(value, "OFF") || value == "0": - return "0", nil - case value == "": - return "", nil - } - return value, ErrWrongValueForVar.GenWithStackByArgs(name, value) } return value, nil } diff --git a/util/stmtsummary/statement_summary.go b/util/stmtsummary/statement_summary.go index c00b42fe9be05..75e931db5a7c3 100644 --- a/util/stmtsummary/statement_summary.go +++ b/util/stmtsummary/statement_summary.go @@ -16,10 +16,12 @@ package stmtsummary import ( "strings" "sync" + "sync/atomic" "time" "github.com/pingcap/parser/mysql" "github.com/pingcap/tidb/config" + "github.com/pingcap/tidb/sessionctx/variable" "github.com/pingcap/tidb/types" "github.com/pingcap/tidb/util/hack" "github.com/pingcap/tidb/util/kvcache" @@ -53,15 +55,6 @@ type stmtSummaryByDigestMap struct { // It's rare to read concurrently, so RWMutex is not needed. sync.Mutex summaryMap *kvcache.SimpleLRUCache - - // enabledWrapper encapsulates variables needed to judge whether statement summary is enabled. - enabledWrapper struct { - sync.RWMutex - // enabled indicates whether statement summary is enabled in current server. - sessionEnabled string - // setInSession indicates whether statement summary has been set in any session. - globalEnabled string - } } // StmtSummaryByDigestMap is a global map containing all statement summaries. @@ -104,13 +97,9 @@ type StmtExecInfo struct { // newStmtSummaryByDigestMap creates an empty stmtSummaryByDigestMap. func newStmtSummaryByDigestMap() *stmtSummaryByDigestMap { maxStmtCount := config.GetGlobalConfig().StmtSummary.MaxStmtCount - ssMap := &stmtSummaryByDigestMap{ + return &stmtSummaryByDigestMap{ summaryMap: kvcache.NewSimpleLRUCache(maxStmtCount, 0, 0), } - // enabledWrapper.defaultEnabled will be initialized in package variable. - ssMap.enabledWrapper.sessionEnabled = "" - ssMap.enabledWrapper.globalEnabled = "" - return ssMap } // newStmtSummaryByDigest creates a stmtSummaryByDigest from StmtExecInfo @@ -175,7 +164,7 @@ func (ssMap *stmtSummaryByDigestMap) AddStatement(sei *StmtExecInfo) { ssMap.Lock() // Check again. Statements could be added before disabling the flag and after Clear() - if !ssMap.Enabled() { + if atomic.LoadInt32(&variable.EnableStmtSummary) == 0 { ssMap.Unlock() return } @@ -199,7 +188,7 @@ func (ssMap *stmtSummaryByDigestMap) Clear() { ssMap.Unlock() } -// ToDatum converts statement summary to Datum +// Convert statement summary to Datum func (ssMap *stmtSummaryByDigestMap) ToDatum() [][]types.Datum { ssMap.Lock() values := ssMap.summaryMap.Values() @@ -230,64 +219,12 @@ func (ssMap *stmtSummaryByDigestMap) ToDatum() [][]types.Datum { return rows } -// SetEnabled enables or disables statement summary in global(cluster) or session(server) scope. -func (ssMap *stmtSummaryByDigestMap) SetEnabled(value string, inSession bool) { - value = ssMap.normalizeEnableValue(value) - - ssMap.enabledWrapper.Lock() - if inSession { - ssMap.enabledWrapper.sessionEnabled = value - } else { - ssMap.enabledWrapper.globalEnabled = value - } - sessionEnabled := ssMap.enabledWrapper.sessionEnabled - globalEnabled := ssMap.enabledWrapper.globalEnabled - ssMap.enabledWrapper.Unlock() - - // Clear all summaries once statement summary is disabled. - var needClear bool - if ssMap.isSet(sessionEnabled) { - needClear = !ssMap.isEnabled(sessionEnabled) - } else { - needClear = !ssMap.isEnabled(globalEnabled) - } - if needClear { - ssMap.Clear() - } -} - -// Enabled returns whether statement summary is enabled. -func (ssMap *stmtSummaryByDigestMap) Enabled() bool { - ssMap.enabledWrapper.RLock() - var enabled bool - if ssMap.isSet(ssMap.enabledWrapper.sessionEnabled) { - enabled = ssMap.isEnabled(ssMap.enabledWrapper.sessionEnabled) +// OnEnableStmtSummaryModified is triggered once EnableStmtSummary is modified. +func OnEnableStmtSummaryModified(newValue string) { + if variable.TiDBOptOn(newValue) { + atomic.StoreInt32(&variable.EnableStmtSummary, 1) } else { - enabled = ssMap.isEnabled(ssMap.enabledWrapper.globalEnabled) - } - ssMap.enabledWrapper.RUnlock() - return enabled -} - -// normalizeEnableValue converts 'ON' to '1' and 'OFF' to '0' -func (ssMap *stmtSummaryByDigestMap) normalizeEnableValue(value string) string { - switch { - case strings.EqualFold(value, "ON"): - return "1" - case strings.EqualFold(value, "OFF"): - return "0" - default: - return value + atomic.StoreInt32(&variable.EnableStmtSummary, 0) + StmtSummaryByDigestMap.Clear() } } - -// isEnabled converts a string value to bool. -// 1 indicates true, 0 or '' indicates false. -func (ssMap *stmtSummaryByDigestMap) isEnabled(value string) bool { - return value == "1" -} - -// isSet judges whether the variable is set. -func (ssMap *stmtSummaryByDigestMap) isSet(value string) bool { - return value != "" -} diff --git a/util/stmtsummary/statement_summary_test.go b/util/stmtsummary/statement_summary_test.go index 5d742b1d08396..614f6f7800146 100644 --- a/util/stmtsummary/statement_summary_test.go +++ b/util/stmtsummary/statement_summary_test.go @@ -17,12 +17,14 @@ import ( "fmt" "strings" "sync" + "sync/atomic" "testing" "time" . "github.com/pingcap/check" "github.com/pingcap/parser/mysql" "github.com/pingcap/tidb/config" + "github.com/pingcap/tidb/sessionctx/variable" "github.com/pingcap/tidb/types" ) @@ -34,7 +36,7 @@ type testStmtSummarySuite struct { func (s *testStmtSummarySuite) SetUpSuite(c *C) { s.ssMap = newStmtSummaryByDigestMap() - s.ssMap.SetEnabled("1", false) + atomic.StoreInt32(&variable.EnableStmtSummary, 1) } func TestT(t *testing.T) { @@ -319,8 +321,7 @@ func (s *testStmtSummarySuite) TestMaxSQLLength(c *C) { // Test setting EnableStmtSummary to 0 func (s *testStmtSummarySuite) TestDisableStmtSummary(c *C) { s.ssMap.Clear() - // Set false in global scope, it should work. - s.ssMap.SetEnabled("0", false) + OnEnableStmtSummaryModified("0") stmtExecInfo1 := &StmtExecInfo{ SchemaName: "schema_name", @@ -337,42 +338,9 @@ func (s *testStmtSummarySuite) TestDisableStmtSummary(c *C) { datums := s.ssMap.ToDatum() c.Assert(len(datums), Equals, 0) - // Set true in session scope, it will overwrite global scope. - s.ssMap.SetEnabled("1", true) + OnEnableStmtSummaryModified("1") s.ssMap.AddStatement(stmtExecInfo1) datums = s.ssMap.ToDatum() c.Assert(len(datums), Equals, 1) - - // Set false in global scope, it shouldn't work. - s.ssMap.SetEnabled("0", false) - - stmtExecInfo2 := &StmtExecInfo{ - SchemaName: "schema_name", - OriginalSQL: "original_sql2", - NormalizedSQL: "normalized_sql2", - Digest: "digest2", - TotalLatency: 50000, - AffectedRows: 500, - SentRows: 500, - StartTime: time.Date(2019, 1, 1, 10, 10, 20, 10, time.UTC), - } - s.ssMap.AddStatement(stmtExecInfo2) - datums = s.ssMap.ToDatum() - c.Assert(len(datums), Equals, 2) - - // Unset in session scope - s.ssMap.SetEnabled("", true) - s.ssMap.AddStatement(stmtExecInfo2) - datums = s.ssMap.ToDatum() - c.Assert(len(datums), Equals, 0) - - // Unset in global scope - s.ssMap.SetEnabled("", false) - s.ssMap.AddStatement(stmtExecInfo1) - datums = s.ssMap.ToDatum() - c.Assert(len(datums), Equals, 0) - - // Set back - s.ssMap.SetEnabled("1", false) }