Skip to content

Commit

Permalink
config: fix data race raised by EnableCollectExecutionInfo
Browse files Browse the repository at this point in the history
  • Loading branch information
CbcWestwolf committed Oct 11, 2022
1 parent f343ffe commit db8bad9
Show file tree
Hide file tree
Showing 10 changed files with 15 additions and 15 deletions.
8 changes: 4 additions & 4 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -485,9 +485,9 @@ type Instance struct {
ForcePriority string `toml:"tidb_force_priority" json:"tidb_force_priority"`
MemoryUsageAlarmRatio float64 `toml:"tidb_memory_usage_alarm_ratio" json:"tidb_memory_usage_alarm_ratio"`
// EnableCollectExecutionInfo enables the TiDB to collect execution info.
EnableCollectExecutionInfo bool `toml:"tidb_enable_collect_execution_info" json:"tidb_enable_collect_execution_info"`
PluginDir string `toml:"plugin_dir" json:"plugin_dir"`
PluginLoad string `toml:"plugin_load" json:"plugin_load"`
EnableCollectExecutionInfo AtomicBool `toml:"tidb_enable_collect_execution_info" json:"tidb_enable_collect_execution_info"`
PluginDir string `toml:"plugin_dir" json:"plugin_dir"`
PluginLoad string `toml:"plugin_load" json:"plugin_load"`
// MaxConnections is the maximum permitted number of simultaneous client connections.
MaxConnections uint32 `toml:"max_connections" json:"max_connections"`
TiDBEnableDDL AtomicBool `toml:"tidb_enable_ddl" json:"tidb_enable_ddl"`
Expand Down Expand Up @@ -864,7 +864,7 @@ var defaultConf = Config{
CheckMb4ValueInUTF8: *NewAtomicBool(true),
ForcePriority: "NO_PRIORITY",
MemoryUsageAlarmRatio: DefMemoryUsageAlarmRatio,
EnableCollectExecutionInfo: true,
EnableCollectExecutionInfo: *NewAtomicBool(true),
PluginDir: "/data/deploy/plugin",
PluginLoad: "",
MaxConnections: 0,
Expand Down
2 changes: 1 addition & 1 deletion distsql/distsql.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func Select(ctx context.Context, sctx sessionctx.Context, kvReq *kv.Request, fie
SessionMemTracker: sctx.GetSessionVars().StmtCtx.MemTracker,
EnabledRateLimitAction: enabledRateLimitAction,
EventCb: eventCb,
EnableCollectExecutionInfo: config.GetGlobalConfig().Instance.EnableCollectExecutionInfo,
EnableCollectExecutionInfo: config.GetGlobalConfig().Instance.EnableCollectExecutionInfo.Load(),
}

if kvReq.StoreType == kv.TiFlash {
Expand Down
2 changes: 1 addition & 1 deletion executor/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -2109,7 +2109,7 @@ func ResetContextOfStmt(ctx sessionctx.Context, s ast.StmtNode) (err error) {
} else if vars.StmtCtx.InSelectStmt {
sc.PrevAffectedRows = -1
}
if globalConfig.Instance.EnableCollectExecutionInfo {
if globalConfig.Instance.EnableCollectExecutionInfo.Load() {
// In ExplainFor case, RuntimeStatsColl should not be reset for reuse,
// because ExplainFor need to display the last statement information.
reuseObj := vars.StmtCtx.RuntimeStatsColl
Expand Down
2 changes: 1 addition & 1 deletion executor/executor_issue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,7 @@ func TestIndexJoin31494(t *testing.T) {
func TestFix31038(t *testing.T) {
defer config.RestoreFunc()()
config.UpdateGlobal(func(conf *config.Config) {
conf.Instance.EnableCollectExecutionInfo = false
conf.Instance.EnableCollectExecutionInfo.Store(false)
})
store := testkit.CreateMockStore(t)
tk := testkit.NewTestKit(t, store)
Expand Down
2 changes: 1 addition & 1 deletion infoschema/cluster_tables_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -602,7 +602,7 @@ func TestStmtSummaryResultRows(t *testing.T) {
tk.MustExec("set global tidb_stmt_summary_max_sql_length=4096")
tk.MustExec("set global tidb_enable_stmt_summary=0")
tk.MustExec("set global tidb_enable_stmt_summary=1")
if !config.GetGlobalConfig().Instance.EnableCollectExecutionInfo {
if !config.GetGlobalConfig().Instance.EnableCollectExecutionInfo.Load() {
tk.MustExec("set @@tidb_enable_collect_execution_info=1")
defer tk.MustExec("set @@tidb_enable_collect_execution_info=0")
}
Expand Down
2 changes: 1 addition & 1 deletion infoschema/tables_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1533,7 +1533,7 @@ func TestVariablesInfo(t *testing.T) {

// stabalize timestamp val and EnableCollectExecutionInfo
tk.MustExec("SET TIMESTAMP=123456789")
config.GetGlobalConfig().Instance.EnableCollectExecutionInfo = false
config.GetGlobalConfig().Instance.EnableCollectExecutionInfo.Store(false)
// Test that in the current_value matches the default value in all
// but a few permitted special cases.
// See session/bootstrap.go:doDMLWorks() for where the exceptions are defined.
Expand Down
6 changes: 3 additions & 3 deletions sessionctx/variable/sysvar.go
Original file line number Diff line number Diff line change
Expand Up @@ -435,14 +435,14 @@ var defaultSysVars = []*SysVar{
{Scope: ScopeInstance, Name: TiDBEnableCollectExecutionInfo, Value: BoolToOnOff(DefTiDBEnableCollectExecutionInfo), Type: TypeBool, SetGlobal: func(s *SessionVars, val string) error {
oldConfig := config.GetGlobalConfig()
newValue := TiDBOptOn(val)
if oldConfig.Instance.EnableCollectExecutionInfo != newValue {
if oldConfig.Instance.EnableCollectExecutionInfo.Load() != newValue {
newConfig := *oldConfig
newConfig.Instance.EnableCollectExecutionInfo = newValue
newConfig.Instance.EnableCollectExecutionInfo.Store(newValue)
config.StoreGlobalConfig(&newConfig)
}
return nil
}, GetGlobal: func(s *SessionVars) (string, error) {
return BoolToOnOff(config.GetGlobalConfig().Instance.EnableCollectExecutionInfo), nil
return BoolToOnOff(config.GetGlobalConfig().Instance.EnableCollectExecutionInfo.Load()), nil
}},
{Scope: ScopeInstance, Name: PluginLoad, Value: "", ReadOnly: true, GetGlobal: func(s *SessionVars) (string, error) {
return config.GetGlobalConfig().Instance.PluginLoad, nil
Expand Down
2 changes: 1 addition & 1 deletion sessionctx/variable/sysvar_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ func TestInstanceScopedVars(t *testing.T) {

val, err = vars.GetSessionOrGlobalSystemVar(TiDBEnableCollectExecutionInfo)
require.NoError(t, err)
require.Equal(t, BoolToOnOff(config.GetGlobalConfig().Instance.EnableCollectExecutionInfo), val)
require.Equal(t, BoolToOnOff(config.GetGlobalConfig().Instance.EnableCollectExecutionInfo.Load()), val)

val, err = vars.GetSessionOrGlobalSystemVar(TiDBConfig)
require.NoError(t, err)
Expand Down
2 changes: 1 addition & 1 deletion store/copr/mpp.go
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ func (c *MPPClient) DispatchMPPTasks(ctx context.Context, variables interface{},
startTs: startTs,
vars: vars,
needTriggerFallback: needTriggerFallback,
enableCollectExecutionInfo: config.GetGlobalConfig().Instance.EnableCollectExecutionInfo,
enableCollectExecutionInfo: config.GetGlobalConfig().Instance.EnableCollectExecutionInfo.Load(),
}
go iter.run(ctxChild)
return iter
Expand Down
2 changes: 1 addition & 1 deletion tidb-server/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,7 @@ func setGlobalVars() {
case "check-mb4-value-in-utf8":
cfg.Instance.CheckMb4ValueInUTF8.Store(cfg.CheckMb4ValueInUTF8.Load())
case "enable-collect-execution-info":
cfg.Instance.EnableCollectExecutionInfo = cfg.EnableCollectExecutionInfo
cfg.Instance.EnableCollectExecutionInfo.Store(cfg.EnableCollectExecutionInfo)
case "max-server-connections":
cfg.Instance.MaxConnections = cfg.MaxServerConnections
case "run-ddl":
Expand Down

0 comments on commit db8bad9

Please sign in to comment.