From 4312656ba774a38a1be2f1a7e17ccce1b97aebdd Mon Sep 17 00:00:00 2001 From: wshwsh12 <793703860@qq.com> Date: Thu, 3 Nov 2022 16:18:56 +0800 Subject: [PATCH 1/3] fix --- executor/executor.go | 6 ++---- session/bootstrap.go | 11 ----------- sessionctx/variable/sysvar.go | 10 ++++++++++ sessionctx/variable/tidb_vars.go | 2 +- tidb-server/main.go | 6 +++--- util/memory/tracker.go | 3 +++ 6 files changed, 19 insertions(+), 19 deletions(-) diff --git a/executor/executor.go b/executor/executor.go index 299b149e2fd9b..21523159e7a9a 100644 --- a/executor/executor.go +++ b/executor/executor.go @@ -100,8 +100,6 @@ var ( _ Executor = &TopNExec{} _ Executor = &UnionExec{} - // GlobalMemoryUsageTracker is the ancestor of all the Executors' memory tracker and GlobalMemory Tracker - GlobalMemoryUsageTracker *memory.Tracker // GlobalDiskUsageTracker is the ancestor of all the Executors' disk tracker GlobalDiskUsageTracker *disk.Tracker // GlobalAnalyzeMemoryTracker is the ancestor of all the Analyze jobs' memory tracker and child of global Tracker @@ -151,8 +149,8 @@ type globalPanicOnExceed struct { func init() { action := &globalPanicOnExceed{} - GlobalMemoryUsageTracker = memory.NewGlobalTracker(memory.LabelForGlobalMemory, -1) - GlobalMemoryUsageTracker.SetActionOnExceed(action) + memory.GlobalMemoryUsageTracker = memory.NewGlobalTracker(memory.LabelForGlobalMemory, -1) + memory.GlobalMemoryUsageTracker.SetActionOnExceed(action) GlobalDiskUsageTracker = disk.NewGlobalTrcaker(memory.LabelForGlobalStorage, -1) GlobalDiskUsageTracker.SetActionOnExceed(action) GlobalAnalyzeMemoryTracker = memory.NewTracker(memory.LabelForGlobalAnalyzeMemory, -1) diff --git a/session/bootstrap.go b/session/bootstrap.go index c449087a2689c..217f9b53d5f0b 100644 --- a/session/bootstrap.go +++ b/session/bootstrap.go @@ -636,8 +636,6 @@ const ( version94 = 94 // version95 add a column `User_attributes` to `mysql.user` version95 = 95 - // version96 converts server-memory-quota to a sysvar - version96 = 96 // version97 sets tidb_opt_range_max_size to 0 when a cluster upgrades from some version lower than v6.4.0 to v6.4.0+. // It promises the compatibility of building ranges behavior. version97 = 97 @@ -748,7 +746,6 @@ var ( upgradeToVer93, upgradeToVer94, upgradeToVer95, - upgradeToVer96, upgradeToVer97, upgradeToVer98, } @@ -1953,14 +1950,6 @@ func upgradeToVer95(s Session, ver int64) { doReentrantDDL(s, "ALTER TABLE mysql.user ADD COLUMN IF NOT EXISTS `User_attributes` JSON") } -func upgradeToVer96(s Session, ver int64) { - if ver >= version96 { - return - } - valStr := strconv.Itoa(int(config.GetGlobalConfig().Performance.ServerMemoryQuota)) - importConfigOption(s, "performance.server-memory-quota", variable.TiDBServerMemoryLimit, valStr) -} - func upgradeToVer97(s Session, ver int64) { if ver >= version97 { return diff --git a/sessionctx/variable/sysvar.go b/sessionctx/variable/sysvar.go index a7fc7fb1dd179..b58628bd55632 100644 --- a/sessionctx/variable/sysvar.go +++ b/sessionctx/variable/sysvar.go @@ -776,6 +776,16 @@ var defaultSysVars = []*SysVar{ memory.ServerMemoryLimitOriginText.Store(str) memory.ServerMemoryLimit.Store(bt) gctuner.GlobalMemoryLimitTuner.UpdateMemoryLimit() + + if bt == 0 { + if config.GetGlobalConfig().Performance.ServerMemoryQuota < 1 { + memory.GlobalMemoryUsageTracker.SetBytesLimit(-1) + } else { + memory.GlobalMemoryUsageTracker.SetBytesLimit(int64(config.GetGlobalConfig().Performance.ServerMemoryQuota)) + } + } else { + memory.GlobalMemoryUsageTracker.SetBytesLimit(-1) + } return nil }, }, diff --git a/sessionctx/variable/tidb_vars.go b/sessionctx/variable/tidb_vars.go index a4c9810e1c341..97dc4bda88456 100644 --- a/sessionctx/variable/tidb_vars.go +++ b/sessionctx/variable/tidb_vars.go @@ -1138,7 +1138,7 @@ var ( // DefTiDBServerMemoryLimit indicates the default value of TiDBServerMemoryLimit(TotalMem * 80%). // It should be a const and shouldn't be modified after tidb is started. - DefTiDBServerMemoryLimit = serverMemoryLimitDefaultValue() + DefTiDBServerMemoryLimit = "0" GOGCTunerThreshold = atomic.NewFloat64(DefTiDBGOGCTunerThreshold) ) diff --git a/tidb-server/main.go b/tidb-server/main.go index 95a95075c6019..056814222775e 100644 --- a/tidb-server/main.go +++ b/tidb-server/main.go @@ -699,11 +699,11 @@ func setGlobalVars() { executor.GlobalDiskUsageTracker.SetBytesLimit(cfg.TempStorageQuota) if cfg.Performance.ServerMemoryQuota < 1 { // If MaxMemory equals 0, it means unlimited - executor.GlobalMemoryUsageTracker.SetBytesLimit(-1) + memory.GlobalMemoryUsageTracker.SetBytesLimit(-1) } else { - executor.GlobalMemoryUsageTracker.SetBytesLimit(int64(cfg.Performance.ServerMemoryQuota)) + memory.GlobalMemoryUsageTracker.SetBytesLimit(int64(cfg.Performance.ServerMemoryQuota)) } - kvcache.GlobalLRUMemUsageTracker.AttachToGlobalTracker(executor.GlobalMemoryUsageTracker) + kvcache.GlobalLRUMemUsageTracker.AttachToGlobalTracker(memory.GlobalMemoryUsageTracker) t, err := time.ParseDuration(cfg.TiKVClient.StoreLivenessTimeout) if err != nil || t < 0 { diff --git a/util/memory/tracker.go b/util/memory/tracker.go index 6f4bad7713e5f..7f2373c1bcf3e 100644 --- a/util/memory/tracker.go +++ b/util/memory/tracker.go @@ -41,6 +41,9 @@ var ( TriggerMemoryLimitGC = atomicutil.NewBool(false) MemoryLimitGCLast = atomicutil.NewTime(time.Time{}) MemoryLimitGCTotal = atomicutil.NewInt64(0) + + // GlobalMemoryUsageTracker is the ancestor of all the Executors' memory tracker and GlobalMemory Tracker + GlobalMemoryUsageTracker *Tracker ) // Tracker is used to track the memory usage during query execution. From 9319aa65e19c8648e254ed43b998b695621648fb Mon Sep 17 00:00:00 2001 From: wshwsh12 <793703860@qq.com> Date: Thu, 3 Nov 2022 16:48:00 +0800 Subject: [PATCH 2/3] address comments --- executor/executor_test.go | 14 ++++++++++++++ server/conn.go | 3 +++ sessionctx/variable/session.go | 1 + 3 files changed, 18 insertions(+) diff --git a/executor/executor_test.go b/executor/executor_test.go index af5608a78aefa..44af013e43d0b 100644 --- a/executor/executor_test.go +++ b/executor/executor_test.go @@ -6199,3 +6199,17 @@ func TestSessionRootTrackerDetach(t *testing.T) { require.NoError(t, err) require.Nil(t, tk.Session().GetSessionVars().MemTracker.GetFallbackForTest(false)) } + +func TestServerMemoryQuota(t *testing.T) { + config.UpdateGlobal(func(conf *config.Config) { + conf.Performance.ServerMemoryQuota = 123456789000 + }) + store := testkit.CreateMockStore(t) + tk := testkit.NewTestKit(t, store) + + require.Equal(t, memory.GlobalMemoryUsageTracker.GetBytesLimit(), int64(123456789000)) + tk.MustExec("set global tidb_server_memory_limit = 3 << 30") + require.Equal(t, memory.GlobalMemoryUsageTracker.GetBytesLimit(), int64(-1)) + tk.MustExec("set global tidb_server_memory_limit = 0") + require.Equal(t, memory.GlobalMemoryUsageTracker.GetBytesLimit(), int64(123456789000)) +} diff --git a/server/conn.go b/server/conn.go index 8fdd6785b4c2b..46cb9086505b4 100644 --- a/server/conn.go +++ b/server/conn.go @@ -1049,6 +1049,9 @@ func (cc *clientConn) initConnect(ctx context.Context) error { // This function returns and the connection is closed if there is an IO error or there is a panic. func (cc *clientConn) Run(ctx context.Context) { defer func() { + if tracker := cc.ctx.GetSessionVars().MemTracker; tracker != nil { + tracker.Detach() + } r := recover() if r != nil { logutil.Logger(ctx).Error("connection running loop panic", diff --git a/sessionctx/variable/session.go b/sessionctx/variable/session.go index ee449c5eb14c4..06a1ac9010823 100644 --- a/sessionctx/variable/session.go +++ b/sessionctx/variable/session.go @@ -1625,6 +1625,7 @@ func NewSessionVars(hctx HookContext) *SessionVars { vars.DiskTracker = disk.NewTracker(memory.LabelForSession, -1) vars.MemTracker = memory.NewTracker(memory.LabelForSession, vars.MemQuotaQuery) vars.MemTracker.IsRootTrackerOfSess = true + vars.MemTracker.AttachToGlobalTracker(memory.GlobalMemoryUsageTracker) for _, engine := range config.GetGlobalConfig().IsolationRead.Engines { switch engine { From a1c0c75bc55c0a62097f73b6fe12777c3b60aaca Mon Sep 17 00:00:00 2001 From: wshwsh12 <793703860@qq.com> Date: Thu, 3 Nov 2022 18:09:40 +0800 Subject: [PATCH 3/3] address comments --- executor/executor_test.go | 4 ++++ server/conn.go | 3 --- session/session.go | 3 +++ util/memory/tracker.go | 5 +++++ 4 files changed, 12 insertions(+), 3 deletions(-) diff --git a/executor/executor_test.go b/executor/executor_test.go index 44af013e43d0b..d7cc8b0cf0563 100644 --- a/executor/executor_test.go +++ b/executor/executor_test.go @@ -6204,6 +6204,7 @@ func TestServerMemoryQuota(t *testing.T) { config.UpdateGlobal(func(conf *config.Config) { conf.Performance.ServerMemoryQuota = 123456789000 }) + defer config.RestoreFunc()() store := testkit.CreateMockStore(t) tk := testkit.NewTestKit(t, store) @@ -6212,4 +6213,7 @@ func TestServerMemoryQuota(t *testing.T) { require.Equal(t, memory.GlobalMemoryUsageTracker.GetBytesLimit(), int64(-1)) tk.MustExec("set global tidb_server_memory_limit = 0") require.Equal(t, memory.GlobalMemoryUsageTracker.GetBytesLimit(), int64(123456789000)) + require.Equal(t, tk.Session().GetSessionVars().MemTracker.GetParentForTest(), memory.GlobalMemoryUsageTracker) + tk.Session().Close() + require.Nil(t, tk.Session().GetSessionVars().MemTracker.GetParentForTest()) } diff --git a/server/conn.go b/server/conn.go index 46cb9086505b4..8fdd6785b4c2b 100644 --- a/server/conn.go +++ b/server/conn.go @@ -1049,9 +1049,6 @@ func (cc *clientConn) initConnect(ctx context.Context) error { // This function returns and the connection is closed if there is an IO error or there is a panic. func (cc *clientConn) Run(ctx context.Context) { defer func() { - if tracker := cc.ctx.GetSessionVars().MemTracker; tracker != nil { - tracker.Detach() - } r := recover() if r != nil { logutil.Logger(ctx).Error("connection running loop panic", diff --git a/session/session.go b/session/session.go index 71783d7a6fa85..a182af9b565d8 100644 --- a/session/session.go +++ b/session/session.go @@ -2571,6 +2571,9 @@ func (s *session) Close() { s.RollbackTxn(ctx) if s.sessionVars != nil { s.sessionVars.WithdrawAllPreparedStmt() + if s.sessionVars.MemTracker != nil { + s.sessionVars.MemTracker.Detach() + } } if s.stmtStats != nil { s.stmtStats.SetFinished() diff --git a/util/memory/tracker.go b/util/memory/tracker.go index 7f2373c1bcf3e..82d592859db7d 100644 --- a/util/memory/tracker.go +++ b/util/memory/tracker.go @@ -762,6 +762,11 @@ func (t *Tracker) setParent(parent *Tracker) { t.parMu.parent = parent } +// GetParentForTest return the parent of the Tracker. Only used by test. +func (t *Tracker) GetParentForTest() *Tracker { + return t.getParent() +} + // CountAllChildrenMemUse return memory used tree for the tracker func (t *Tracker) CountAllChildrenMemUse() map[string]int64 { trackerMemUseMap := make(map[string]int64, 1024)