Skip to content

Commit

Permalink
*: move config file option oom-action to sysvar (#34644)
Browse files Browse the repository at this point in the history
ref #33769
  • Loading branch information
ramanich1 authored May 18, 2022
1 parent eae2ae8 commit a961485
Show file tree
Hide file tree
Showing 24 changed files with 62 additions and 176 deletions.
22 changes: 0 additions & 22 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,6 @@ type Config struct {
TokenLimit uint `toml:"token-limit" json:"token-limit"`
OOMUseTmpStorage bool `toml:"oom-use-tmp-storage" json:"oom-use-tmp-storage"`
TempStoragePath string `toml:"tmp-storage-path" json:"tmp-storage-path"`
OOMAction string `toml:"oom-action" json:"oom-action"`
// TempStorageQuota describe the temporary storage Quota during query exector when OOMUseTmpStorage is enabled
// If the quota exceed the capacity of the TempStoragePath, the tidb-server would exit with fatal error
TempStorageQuota int64 `toml:"tmp-storage-quota" json:"tmp-storage-quota"` // Bytes
Expand Down Expand Up @@ -753,7 +752,6 @@ var defaultConf = Config{
OOMUseTmpStorage: true,
TempStorageQuota: -1,
TempStoragePath: tempStorageDirName,
OOMAction: OOMActionCancel,
EnableBatchDML: false,
CheckMb4ValueInUTF8: *NewAtomicBool(true),
MaxIndexLength: 3072,
Expand Down Expand Up @@ -961,10 +959,6 @@ func isAllDeprecatedConfigItems(items []string) bool {
return true
}

// IsOOMActionSetByUser indicates whether the config item mem-action is set by
// the user.
var IsOOMActionSetByUser bool

// InitializeConfig initialize the global config handler.
// The function enforceCmdArgs is used to merge the config file with command arguments:
// For example, if you start TiDB by the command "./tidb-server --port=3000", the port number should be
Expand Down Expand Up @@ -1023,9 +1017,6 @@ func (c *Config) Load(confFile string) error {
if c.TokenLimit == 0 {
c.TokenLimit = 1000
}
if metaData.IsDefined("oom-action") {
IsOOMActionSetByUser = true
}
// If any items in confFile file are not mapped into the Config struct, issue
// an error and stop the server from starting.
undecoded := metaData.Undecoded()
Expand Down Expand Up @@ -1101,10 +1092,6 @@ func (c *Config) Valid() error {
if c.Log.File.MaxSize > MaxLogFileSize {
return fmt.Errorf("invalid max log file size=%v which is larger than max=%v", c.Log.File.MaxSize, MaxLogFileSize)
}
c.OOMAction = strings.ToLower(c.OOMAction)
if c.OOMAction != OOMActionLog && c.OOMAction != OOMActionCancel {
return fmt.Errorf("unsupported OOMAction %v, TiDB only supports [%v, %v]", c.OOMAction, OOMActionLog, OOMActionCancel)
}
if c.TableColumnCountLimit < DefTableColumnCountLimit || c.TableColumnCountLimit > DefMaxOfTableColumnCountLimit {
return fmt.Errorf("table-column-limit should be [%d, %d]", DefIndexLimit, DefMaxOfTableColumnCountLimit)
}
Expand Down Expand Up @@ -1235,15 +1222,6 @@ func initByLDFlags(edition, checkBeforeDropLDFlag string) {
}
}

// The following constants represents the valid action configurations for OOMAction.
// NOTE: Although the values is case-insensitive, we should use lower-case
// strings because the configuration value will be transformed to lower-case
// string and compared with these constants in the further usage.
const (
OOMActionCancel = "cancel"
OOMActionLog = "log"
)

// hideConfig is used to filter a single line of config for hiding.
var hideConfig = []string{
"index-usage-sync-lease",
Expand Down
4 changes: 0 additions & 4 deletions config/config.toml.example
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,6 @@ oom-use-tmp-storage = true
# The default value of tmp-storage-quota is under 0 which means tidb-server wouldn't check the capacity.
tmp-storage-quota = -1

# Specifies what operation TiDB performs when a single SQL statement exceeds the memory quota specified by the memory quota and cannot be spilled over to disk.
# Valid options: ["log", "cancel"]
oom-action = "cancel"

# Make "kill query" behavior compatible with MySQL. It's not recommend to
# turn on this option when TiDB server is behind a proxy.
compatible-kill-query = false
Expand Down
18 changes: 0 additions & 18 deletions config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -473,24 +473,6 @@ xkNuJ2BlEGkwWLiRbKy1lNBBFUXKuhh3L/EIY10WTnr3TQzeL6H1
}
}

func TestOOMActionValid(t *testing.T) {
c1 := NewConfig()
tests := []struct {
oomAction string
valid bool
}{
{"log", true},
{"Log", true},
{"Cancel", true},
{"cANceL", true},
{"quit", false},
}
for _, tt := range tests {
c1.OOMAction = tt.oomAction
require.Equal(t, tt.valid, c1.Valid() == nil)
}
}

func TestTxnTotalSizeLimitValid(t *testing.T) {
conf := NewConfig()
tests := []struct {
Expand Down
1 change: 0 additions & 1 deletion config/config_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ var (
"Performance.PseudoEstimateRatio": {},
"Performance.StmtCountLimit": {},
"Performance.TCPKeepAlive": {},
"OOMAction": {},
"TiKVClient.StoreLimit": {},
"Log.Level": {},
"Log.ExpensiveThreshold": {},
Expand Down
4 changes: 1 addition & 3 deletions config/config_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ func TestMergeConfigItems(t *testing.T) {
newConf.Performance.FeedbackProbability = 123
newConf.Performance.QueryFeedbackLimit = 123
newConf.Performance.PseudoEstimateRatio = 123
newConf.OOMAction = "panic"
newConf.TiKVClient.StoreLimit = 123

// rejected
Expand All @@ -63,7 +62,7 @@ func TestMergeConfigItems(t *testing.T) {
newConf.Instance.SlowThreshold = 2345

as, rs := MergeConfigItems(oldConf, newConf)
require.Equal(t, 9, len(as))
require.Equal(t, 8, len(as))
require.Equal(t, 3, len(rs))
for _, a := range as {
_, ok := dynamicConfigItems[a]
Expand All @@ -80,7 +79,6 @@ func TestMergeConfigItems(t *testing.T) {
require.Equal(t, newConf.Performance.FeedbackProbability, oldConf.Performance.FeedbackProbability)
require.Equal(t, newConf.Performance.QueryFeedbackLimit, oldConf.Performance.QueryFeedbackLimit)
require.Equal(t, newConf.Performance.PseudoEstimateRatio, oldConf.Performance.PseudoEstimateRatio)
require.Equal(t, newConf.OOMAction, oldConf.OOMAction)
require.Equal(t, newConf.TiKVClient.StoreLimit, oldConf.TiKVClient.StoreLimit)
require.Equal(t, newConf.Instance.SlowThreshold, oldConf.Instance.SlowThreshold)

Expand Down
6 changes: 3 additions & 3 deletions executor/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -1792,12 +1792,12 @@ func ResetContextOfStmt(ctx sessionctx.Context, s ast.StmtNode) (err error) {
if globalConfig.OOMUseTmpStorage && GlobalDiskUsageTracker != nil {
sc.DiskTracker.AttachToGlobalTracker(GlobalDiskUsageTracker)
}
switch globalConfig.OOMAction {
case config.OOMActionCancel:
switch variable.OOMAction.Load() {
case variable.OOMActionCancel:
action := &memory.PanicOnExceed{ConnID: ctx.GetSessionVars().ConnectionID}
action.SetLogHook(domain.GetDomain(ctx).ExpensiveQueryHandle().LogOnQueryExceedMemQuota)
sc.MemTracker.SetActionOnExceed(action)
case config.OOMActionLog:
case variable.OOMActionLog:
fallthrough
default:
action := &memory.LogOnExceed{ConnID: ctx.GetSessionVars().ConnectionID}
Expand Down
6 changes: 2 additions & 4 deletions executor/executor_failpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -299,10 +299,8 @@ func TestCoprocessorOOMTiCase(t *testing.T) {
tk.MustExec(fmt.Sprintf("insert into t5 (id) values (%v)", i))
tk.MustExec(fmt.Sprintf("insert into t6 (id) values (%v)", i))
}
defer config.RestoreFunc()()
config.UpdateGlobal(func(conf *config.Config) {
conf.OOMAction = config.OOMActionLog
})
defer tk.MustExec("SET GLOBAL tidb_mem_oom_action = DEFAULT")
tk.MustExec("SET GLOBAL tidb_mem_oom_action='LOG'")
testcases := []struct {
name string
sql string
Expand Down
16 changes: 4 additions & 12 deletions executor/executor_issue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -304,14 +304,8 @@ func TestIssue28650(t *testing.T) {
for i := 0; i < 10; i++ {
tk.MustExec("insert into t1 select rand()*400 from t1;")
}
config.UpdateGlobal(func(conf *config.Config) {
conf.OOMAction = config.OOMActionCancel
})
defer func() {
config.UpdateGlobal(func(conf *config.Config) {
conf.OOMAction = config.OOMActionLog
})
}()
tk.MustExec("SET GLOBAL tidb_mem_oom_action = 'CANCEL'")
defer tk.MustExec("SET GLOBAL tidb_mem_oom_action='LOG'")
wg.Wait()
for _, sql := range sqls {
tk.MustExec("set @@tidb_mem_quota_query = 1073741824") // 1GB
Expand Down Expand Up @@ -439,10 +433,8 @@ func TestIndexJoin31494(t *testing.T) {
}
tk.Session().SetSessionManager(sm)
dom.ExpensiveQueryHandle().SetSessionManager(sm)
defer config.RestoreFunc()()
config.UpdateGlobal(func(conf *config.Config) {
conf.OOMAction = config.OOMActionCancel
})
defer tk.MustExec("SET GLOBAL tidb_mem_oom_action = DEFAULT")
tk.MustExec("SET GLOBAL tidb_mem_oom_action='CANCEL'")
require.True(t, tk.Session().Auth(&auth.UserIdentity{Username: "root", Hostname: "%"}, nil, nil))
tk.MustExec("set @@tidb_mem_quota_query=2097152;")
// This bug will be reproduced in 10 times.
Expand Down
12 changes: 4 additions & 8 deletions executor/executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3626,10 +3626,8 @@ func TestOOMPanicAction(t *testing.T) {
}
tk.Session().SetSessionManager(sm)
dom.ExpensiveQueryHandle().SetSessionManager(sm)
defer config.RestoreFunc()()
config.UpdateGlobal(func(conf *config.Config) {
conf.OOMAction = config.OOMActionCancel
})
defer tk.MustExec("SET GLOBAL tidb_mem_oom_action = DEFAULT")
tk.MustExec("SET GLOBAL tidb_mem_oom_action='CANCEL'")
tk.MustExec("set @@tidb_mem_quota_query=1;")
err := tk.QueryToErr("select sum(b) from t group by a;")
require.Error(t, err)
Expand Down Expand Up @@ -6060,10 +6058,8 @@ func TestSummaryFailedUpdate(t *testing.T) {
}
tk.Session().SetSessionManager(sm)
dom.ExpensiveQueryHandle().SetSessionManager(sm)
defer config.RestoreFunc()()
config.UpdateGlobal(func(conf *config.Config) {
conf.OOMAction = config.OOMActionCancel
})
defer tk.MustExec("SET GLOBAL tidb_mem_oom_action = DEFAULT")
tk.MustExec("SET GLOBAL tidb_mem_oom_action='CANCEL'")
require.True(t, tk.Session().Auth(&auth.UserIdentity{Username: "root", Hostname: "%"}, nil, nil))
tk.MustExec("set @@tidb_mem_quota_query=1")
tk.MustMatchErrMsg("update t set t.a = t.a - 1 where t.a in (select a from t where a < 4)", "Out Of Memory Quota!.*")
Expand Down
20 changes: 6 additions & 14 deletions executor/join_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,13 @@ func TestJoinInDisk(t *testing.T) {
defer origin()
config.UpdateGlobal(func(conf *config.Config) {
conf.OOMUseTmpStorage = true
conf.OOMAction = config.OOMActionLog
})

store, dom, clean := testkit.CreateMockStoreAndDomain(t)
defer clean()
tk := testkit.NewTestKit(t, store)
defer tk.MustExec("SET GLOBAL tidb_mem_oom_action = DEFAULT")
tk.MustExec("SET GLOBAL tidb_mem_oom_action='LOG'")
tk.MustExec("use test")

sm := &testkit.MockSessionManager{
Expand Down Expand Up @@ -2331,14 +2332,11 @@ func TestInlineProjection4HashJoinIssue15316(t *testing.T) {
}

func TestIssue18070(t *testing.T) {
restoreFunc := config.RestoreFunc()
defer restoreFunc()
config.UpdateGlobal(func(conf *config.Config) {
conf.OOMAction = config.OOMActionCancel
})
store, clean := testkit.CreateMockStore(t)
defer clean()
tk := testkit.NewTestKit(t, store)
defer tk.MustExec("SET GLOBAL tidb_mem_oom_action = DEFAULT")
tk.MustExec("SET GLOBAL tidb_mem_oom_action='CANCEL'")
tk.MustExec("use test")
tk.MustExec("drop table if exists t1, t2")
tk.MustExec("create table t1(a int, index(a))")
Expand Down Expand Up @@ -2770,14 +2768,8 @@ func TestIssue30211(t *testing.T) {
tk.MustExec("insert into t2 values(1),(1),(2),(2);")
tk.MustExec("set @@tidb_mem_quota_query=8000;")
tk.MustExec("set tidb_index_join_batch_size = 1;")
config.UpdateGlobal(func(conf *config.Config) {
conf.OOMAction = config.OOMActionCancel
})
defer func() {
config.UpdateGlobal(func(conf *config.Config) {
conf.OOMAction = config.OOMActionLog
})
}()
tk.MustExec("SET GLOBAL tidb_mem_oom_action = 'CANCEL'")
defer tk.MustExec("SET GLOBAL tidb_mem_oom_action='LOG'")
err := tk.QueryToErr("select /*+ inl_join(t1) */ * from t1 join t2 on t1.a = t2.a;").Error()
require.True(t, strings.Contains(err, "Out Of Memory Quota"))
err = tk.QueryToErr("select /*+ inl_hash_join(t1) */ * from t1 join t2 on t1.a = t2.a;").Error()
Expand Down
1 change: 0 additions & 1 deletion executor/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ func TestMain(m *testing.M) {
conf.TiKVClient.AsyncCommit.SafeWindow = 0
conf.TiKVClient.AsyncCommit.AllowedClockDrift = 0
conf.Experimental.AllowsExpressionIndex = true
conf.OOMAction = config.OOMActionLog
})
tikv.EnableFailpoints()

Expand Down
3 changes: 2 additions & 1 deletion executor/merge_join_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,6 @@ func TestMergeJoinInDisk(t *testing.T) {
defer restore()
config.UpdateGlobal(func(conf *config.Config) {
conf.OOMUseTmpStorage = true
conf.OOMAction = config.OOMActionLog
conf.TempStoragePath = t.TempDir()
})

Expand All @@ -293,6 +292,8 @@ func TestMergeJoinInDisk(t *testing.T) {
store, dom, clean := testkit.CreateMockStoreAndDomain(t)
defer clean()
tk := testkit.NewTestKit(t, store)
defer tk.MustExec("SET GLOBAL tidb_mem_oom_action = DEFAULT")
tk.MustExec("SET GLOBAL tidb_mem_oom_action='LOG'")
tk.MustExec("use test")

sm := &testkit.MockSessionManager{
Expand Down
4 changes: 0 additions & 4 deletions executor/oomtest/oom_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"testing"

"github.com/pingcap/log"
"github.com/pingcap/tidb/config"
"github.com/pingcap/tidb/testkit"
"github.com/pingcap/tidb/testkit/testsetup"
"github.com/stretchr/testify/require"
Expand All @@ -35,9 +34,6 @@ import (
func TestMain(m *testing.M) {
testsetup.SetupForCommonTest()
registerHook()
config.UpdateGlobal(func(conf *config.Config) {
conf.OOMAction = config.OOMActionLog
})
opts := []goleak.Option{
goleak.IgnoreTopFunction("github.com/golang/glog.(*loggingT).flushDaemon"),
goleak.IgnoreTopFunction("go.etcd.io/etcd/client/pkg/v3/logutil.(*MergeLogger).outputLoop"),
Expand Down
6 changes: 4 additions & 2 deletions executor/sort_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ func testSortInDisk(t *testing.T, removeDir bool) {
defer restore()
config.UpdateGlobal(func(conf *config.Config) {
conf.OOMUseTmpStorage = true
conf.OOMAction = config.OOMActionLog
conf.TempStoragePath = t.TempDir()
})
require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/executor/testSortedRowContainerSpill", "return(true)"))
Expand All @@ -49,6 +48,8 @@ func testSortInDisk(t *testing.T, removeDir bool) {
store, dom, clean := testkit.CreateMockStoreAndDomain(t)
defer clean()
tk := testkit.NewTestKit(t, store)
defer tk.MustExec("SET GLOBAL tidb_mem_oom_action = DEFAULT")
tk.MustExec("SET GLOBAL tidb_mem_oom_action='LOG'")
tk.MustExec("use test")

sm := &testkit.MockSessionManager{
Expand Down Expand Up @@ -98,7 +99,6 @@ func TestIssue16696(t *testing.T) {
defer config.RestoreFunc()()
config.UpdateGlobal(func(conf *config.Config) {
conf.OOMUseTmpStorage = true
conf.OOMAction = config.OOMActionLog
conf.TempStoragePath = t.TempDir()
})
alarmRatio := variable.MemoryUsageAlarmRatio.Load()
Expand All @@ -112,6 +112,8 @@ func TestIssue16696(t *testing.T) {
store, clean := testkit.CreateMockStore(t)
defer clean()
tk := testkit.NewTestKit(t, store)
defer tk.MustExec("SET GLOBAL tidb_mem_oom_action = DEFAULT")
tk.MustExec("SET GLOBAL tidb_mem_oom_action='LOG'")
tk.MustExec("use test")
tk.MustExec("drop table if exists t")
tk.MustExec("CREATE TABLE `t` (`a` int(11) DEFAULT NULL,`b` int(11) DEFAULT NULL)")
Expand Down
5 changes: 3 additions & 2 deletions infoschema/cluster_tables_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -673,14 +673,15 @@ select * from t1;
originCfg := config.GetGlobalConfig()
newCfg := *originCfg
newCfg.Log.SlowQueryFile = f.Name()
newCfg.OOMAction = config.OOMActionCancel
config.StoreGlobalConfig(&newCfg)
defer func() {
executor.ParseSlowLogBatchSize = 64
config.StoreGlobalConfig(originCfg)
require.NoError(t, os.Remove(newCfg.Log.SlowQueryFile))
}()

// The server default is CANCEL, but the testsuite defaults to LOG
tk.MustExec("set global tidb_mem_oom_action='CANCEL'")
defer tk.MustExec("set global tidb_mem_oom_action='LOG'")
tk.MustExec(fmt.Sprintf("set @@tidb_slow_query_file='%v'", f.Name()))
checkFn := func(quota int) {
tk.MustExec("set tidb_mem_quota_query=" + strconv.Itoa(quota)) // session
Expand Down
6 changes: 3 additions & 3 deletions server/rpc_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"github.com/pingcap/tidb/privilege"
"github.com/pingcap/tidb/privilege/privileges"
"github.com/pingcap/tidb/session"
"github.com/pingcap/tidb/sessionctx/variable"
"github.com/pingcap/tidb/util"
"github.com/pingcap/tidb/util/logutil"
"github.com/pingcap/tidb/util/memory"
Expand Down Expand Up @@ -223,9 +224,8 @@ func (s *rpcServer) createSession() (session.Session, error) {
vars.SetHashAggFinalConcurrency(1)
vars.StmtCtx.InitMemTracker(memory.LabelForSQLText, vars.MemQuotaQuery)
vars.StmtCtx.MemTracker.AttachToGlobalTracker(executor.GlobalMemoryUsageTracker)
globalConfig := config.GetGlobalConfig()
switch globalConfig.OOMAction {
case config.OOMActionCancel:
switch variable.OOMAction.Load() {
case variable.OOMActionCancel:
action := &memory.PanicOnExceed{}
action.SetLogHook(domain.GetDomain(se).ExpensiveQueryHandle().LogOnQueryExceedMemQuota)
vars.StmtCtx.MemTracker.SetActionOnExceed(action)
Expand Down
Loading

0 comments on commit a961485

Please sign in to comment.