Revert "session, executor: support setting tidb_enable_stmt_summary i…

…n session scope (pingcap#12217) (pingcap#12308)"

This reverts commit f33c908.

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
 		}
 	}

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))
 }

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

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
 }
 

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())
 }

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.

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)
 )

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
 }

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 != ""
-}