From 2ee2c5a3269acdaa13194d6af02462dd6fd0fabe Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Fri, 4 Jan 2019 15:43:37 +0800 Subject: [PATCH 1/6] variable: remove tidb_ddl_reorg_worker_cnt and tidb_ddl_reorg_batch_size session level. --- ddl/failtest/fail_db_test.go | 2 +- executor/ddl_test.go | 28 ++++++++++++++-------------- session/session.go | 5 ++++- sessionctx/variable/session.go | 14 ++++++++++---- sessionctx/variable/sysvar.go | 4 ++-- sessionctx/variable/varsutil.go | 4 ++++ 6 files changed, 35 insertions(+), 22 deletions(-) diff --git a/ddl/failtest/fail_db_test.go b/ddl/failtest/fail_db_test.go index 491704bca3eb9..c603b73b1c51d 100644 --- a/ddl/failtest/fail_db_test.go +++ b/ddl/failtest/fail_db_test.go @@ -364,7 +364,7 @@ LOOP: c.Assert(err, IsNil, Commentf("err:%v", errors.ErrorStack(err))) case <-ddl.TestCheckWorkerNumCh: lastSetWorkerCnt = int32(rand.Intn(8) + 8) - tk.MustExec(fmt.Sprintf("set @@tidb_ddl_reorg_worker_cnt=%d", lastSetWorkerCnt)) + tk.MustExec(fmt.Sprintf("set @@global.tidb_ddl_reorg_worker_cnt=%d", lastSetWorkerCnt)) atomic.StoreInt32(&ddl.TestCheckWorkerNumber, lastSetWorkerCnt) checkNum++ } diff --git a/executor/ddl_test.go b/executor/ddl_test.go index cf88c198a5584..1489bd02e570c 100644 --- a/executor/ddl_test.go +++ b/executor/ddl_test.go @@ -492,23 +492,23 @@ func (s *testSuite3) TestSetDDLReorgWorkerCnt(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec("use test") c.Assert(variable.GetDDLReorgWorkerCounter(), Equals, int32(variable.DefTiDBDDLReorgWorkerCount)) - tk.MustExec("set tidb_ddl_reorg_worker_cnt = 1") + tk.MustExec("set @@global.tidb_ddl_reorg_worker_cnt = 1") c.Assert(variable.GetDDLReorgWorkerCounter(), Equals, int32(1)) - tk.MustExec("set tidb_ddl_reorg_worker_cnt = 100") + tk.MustExec("set @@global.tidb_ddl_reorg_worker_cnt = 100") c.Assert(variable.GetDDLReorgWorkerCounter(), Equals, int32(100)) - _, err := tk.Exec("set tidb_ddl_reorg_worker_cnt = invalid_val") + _, err := tk.Exec("set @@global.tidb_ddl_reorg_worker_cnt = invalid_val") c.Assert(terror.ErrorEqual(err, variable.ErrWrongTypeForVar), IsTrue, Commentf("err %v", err)) - tk.MustExec("set tidb_ddl_reorg_worker_cnt = 100") + tk.MustExec("set @@global.tidb_ddl_reorg_worker_cnt = 100") c.Assert(variable.GetDDLReorgWorkerCounter(), Equals, int32(100)) - _, err = tk.Exec("set tidb_ddl_reorg_worker_cnt = -1") + _, err = tk.Exec("set @@global.tidb_ddl_reorg_worker_cnt = -1") c.Assert(terror.ErrorEqual(err, variable.ErrWrongValueForVar), IsTrue, Commentf("err %v", err)) - tk.MustExec("set tidb_ddl_reorg_worker_cnt = 100") + tk.MustExec("set @@global.tidb_ddl_reorg_worker_cnt = 100") res := tk.MustQuery("select @@tidb_ddl_reorg_worker_cnt") res.Check(testkit.Rows("100")) res = tk.MustQuery("select @@global.tidb_ddl_reorg_worker_cnt") - res.Check(testkit.Rows(fmt.Sprintf("%v", variable.DefTiDBDDLReorgWorkerCount))) + res.Check(testkit.Rows(fmt.Sprintf("%v", 100))) tk.MustExec("set @@global.tidb_ddl_reorg_worker_cnt = 100") res = tk.MustQuery("select @@global.tidb_ddl_reorg_worker_cnt") res.Check(testkit.Rows("100")) @@ -519,25 +519,25 @@ func (s *testSuite3) TestSetDDLReorgBatchSize(c *C) { tk.MustExec("use test") c.Assert(variable.GetDDLReorgBatchSize(), Equals, int32(variable.DefTiDBDDLReorgBatchSize)) - tk.MustExec("set tidb_ddl_reorg_batch_size = 1") + tk.MustExec("set @@global.tidb_ddl_reorg_batch_size = 1") tk.MustQuery("show warnings;").Check(testkit.Rows("Warning 1292 Truncated incorrect tidb_ddl_reorg_batch_size value: '1'")) c.Assert(variable.GetDDLReorgBatchSize(), Equals, int32(variable.MinDDLReorgBatchSize)) - tk.MustExec(fmt.Sprintf("set tidb_ddl_reorg_batch_size = %v", variable.MaxDDLReorgBatchSize+1)) + tk.MustExec(fmt.Sprintf("set @@global.tidb_ddl_reorg_batch_size = %v", variable.MaxDDLReorgBatchSize+1)) tk.MustQuery("show warnings;").Check(testkit.Rows(fmt.Sprintf("Warning 1292 Truncated incorrect tidb_ddl_reorg_batch_size value: '%d'", variable.MaxDDLReorgBatchSize+1))) c.Assert(variable.GetDDLReorgBatchSize(), Equals, int32(variable.MaxDDLReorgBatchSize)) - _, err := tk.Exec("set tidb_ddl_reorg_batch_size = invalid_val") + _, err := tk.Exec("set @@global.tidb_ddl_reorg_batch_size = invalid_val") c.Assert(terror.ErrorEqual(err, variable.ErrWrongTypeForVar), IsTrue, Commentf("err %v", err)) - tk.MustExec("set tidb_ddl_reorg_batch_size = 100") + tk.MustExec("set @@global.tidb_ddl_reorg_batch_size = 100") c.Assert(variable.GetDDLReorgBatchSize(), Equals, int32(100)) - tk.MustExec("set tidb_ddl_reorg_batch_size = -1") + tk.MustExec("set @@global.tidb_ddl_reorg_batch_size = -1") tk.MustQuery("show warnings;").Check(testkit.Rows("Warning 1292 Truncated incorrect tidb_ddl_reorg_batch_size value: '-1'")) - tk.MustExec("set tidb_ddl_reorg_batch_size = 100") + tk.MustExec("set @@global.tidb_ddl_reorg_batch_size = 100") res := tk.MustQuery("select @@tidb_ddl_reorg_batch_size") res.Check(testkit.Rows("100")) res = tk.MustQuery("select @@global.tidb_ddl_reorg_batch_size") - res.Check(testkit.Rows(fmt.Sprintf("%v", variable.DefTiDBDDLReorgBatchSize))) + res.Check(testkit.Rows(fmt.Sprintf("%v", 100))) tk.MustExec("set @@global.tidb_ddl_reorg_batch_size = 1000") res = tk.MustQuery("select @@global.tidb_ddl_reorg_batch_size") res.Check(testkit.Rows("1000")) diff --git a/session/session.go b/session/session.go index f8a0a4e999412..a65df5d8d72e5 100644 --- a/session/session.go +++ b/session/session.go @@ -825,9 +825,12 @@ func (s *session) SetGlobalSysVar(name, value string) error { if err != nil { return errors.Trace(err) } + name = strings.ToLower(name) sql := fmt.Sprintf(`REPLACE %s.%s VALUES ('%s', '%s');`, - mysql.SystemDB, mysql.GlobalVariablesTable, strings.ToLower(name), sVal) + mysql.SystemDB, mysql.GlobalVariablesTable, name, sVal) _, _, err = s.ExecRestrictedSQL(s, sql) + // update local + variable.SetLocalSystemVar(name, sVal) return errors.Trace(err) } diff --git a/sessionctx/variable/session.go b/sessionctx/variable/session.go index 86fd36b5837d2..05ac67180563a 100644 --- a/sessionctx/variable/session.go +++ b/sessionctx/variable/session.go @@ -678,10 +678,6 @@ func (s *SessionVars) SetSystemVar(name string, val string) error { s.OptimizerSelectivityLevel = tidbOptPositiveInt32(val, DefTiDBOptimizerSelectivityLevel) case TiDBEnableTablePartition: s.EnableTablePartition = val - case TiDBDDLReorgWorkerCount: - SetDDLReorgWorkerCounter(int32(tidbOptPositiveInt32(val, DefTiDBDDLReorgWorkerCount))) - case TiDBDDLReorgBatchSize: - SetDDLReorgBatchSize(int32(tidbOptPositiveInt32(val, DefTiDBDDLReorgBatchSize))) case TiDBDDLReorgPriority: s.setDDLReorgPriority(val) case TiDBForcePriority: @@ -691,10 +687,20 @@ func (s *SessionVars) SetSystemVar(name string, val string) error { case TiDBEnableWindowFunction: s.EnableWindowFunction = TiDBOptOn(val) } + SetLocalSystemVar(name, val) s.systems[name] = val return nil } +func SetLocalSystemVar(name string, val string) { + switch name { + case TiDBDDLReorgWorkerCount: + SetDDLReorgWorkerCounter(int32(tidbOptPositiveInt32(val, DefTiDBDDLReorgWorkerCount))) + case TiDBDDLReorgBatchSize: + SetDDLReorgBatchSize(int32(tidbOptPositiveInt32(val, DefTiDBDDLReorgBatchSize))) + } +} + // special session variables. const ( SQLModeVar = "sql_mode" diff --git a/sessionctx/variable/sysvar.go b/sessionctx/variable/sysvar.go index 1e7437931ae78..4f9f96d758258 100644 --- a/sessionctx/variable/sysvar.go +++ b/sessionctx/variable/sysvar.go @@ -671,8 +671,8 @@ var defaultSysVars = []*SysVar{ {ScopeSession, TiDBSlowLogThreshold, strconv.Itoa(logutil.DefaultSlowThreshold)}, {ScopeSession, TiDBQueryLogMaxLen, strconv.Itoa(logutil.DefaultQueryLogMaxLen)}, {ScopeSession, TiDBConfig, ""}, - {ScopeGlobal | ScopeSession, TiDBDDLReorgWorkerCount, strconv.Itoa(DefTiDBDDLReorgWorkerCount)}, - {ScopeGlobal | ScopeSession, TiDBDDLReorgBatchSize, strconv.Itoa(DefTiDBDDLReorgBatchSize)}, + {ScopeGlobal, TiDBDDLReorgWorkerCount, strconv.Itoa(DefTiDBDDLReorgWorkerCount)}, + {ScopeGlobal, TiDBDDLReorgBatchSize, strconv.Itoa(DefTiDBDDLReorgBatchSize)}, {ScopeSession, TiDBDDLReorgPriority, "PRIORITY_LOW"}, {ScopeSession, TiDBForcePriority, mysql.Priority2Str[DefTiDBForcePriority]}, {ScopeSession, TiDBEnableRadixJoin, boolToIntStr(DefTiDBUseRadixJoin)}, diff --git a/sessionctx/variable/varsutil.go b/sessionctx/variable/varsutil.go index 17a687f612cc1..c77049be95c95 100644 --- a/sessionctx/variable/varsutil.go +++ b/sessionctx/variable/varsutil.go @@ -106,6 +106,10 @@ func GetSessionOnlySysVars(s *SessionVars, key string) (string, bool, error) { return strconv.FormatUint(atomic.LoadUint64(&config.GetGlobalConfig().Log.SlowThreshold), 10), true, nil case TiDBQueryLogMaxLen: return strconv.FormatUint(atomic.LoadUint64(&config.GetGlobalConfig().Log.QueryLogMaxLen), 10), true, nil + case TiDBDDLReorgWorkerCount: + return strconv.FormatInt(int64(GetDDLReorgWorkerCounter()), 10), true, nil + case TiDBDDLReorgBatchSize: + return strconv.FormatInt(int64(GetDDLReorgBatchSize()), 10), true, nil } sVal, ok := s.systems[key] if ok { From e7ea8c2c6d765ce11cd5cbee0911690a4de59a5f Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Fri, 4 Jan 2019 16:15:55 +0800 Subject: [PATCH 2/6] add comment --- sessionctx/variable/session.go | 1 + 1 file changed, 1 insertion(+) diff --git a/sessionctx/variable/session.go b/sessionctx/variable/session.go index 05ac67180563a..fa08282ef1625 100644 --- a/sessionctx/variable/session.go +++ b/sessionctx/variable/session.go @@ -692,6 +692,7 @@ func (s *SessionVars) SetSystemVar(name string, val string) error { return nil } +// SetLocalSystemVar sets values of the local variables which in "server" scope. func SetLocalSystemVar(name string, val string) { switch name { case TiDBDDLReorgWorkerCount: From ab60e57042af0f79adf3758c7c873fe7d726fbaa Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Mon, 7 Jan 2019 19:26:14 +0800 Subject: [PATCH 3/6] address comment --- executor/ddl_test.go | 4 ++-- sessionctx/variable/varsutil.go | 4 ---- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/executor/ddl_test.go b/executor/ddl_test.go index 1489bd02e570c..606c51b3e4bbd 100644 --- a/executor/ddl_test.go +++ b/executor/ddl_test.go @@ -504,7 +504,7 @@ func (s *testSuite3) TestSetDDLReorgWorkerCnt(c *C) { c.Assert(terror.ErrorEqual(err, variable.ErrWrongValueForVar), IsTrue, Commentf("err %v", err)) tk.MustExec("set @@global.tidb_ddl_reorg_worker_cnt = 100") - res := tk.MustQuery("select @@tidb_ddl_reorg_worker_cnt") + res := tk.MustQuery("select @@global.tidb_ddl_reorg_worker_cnt") res.Check(testkit.Rows("100")) res = tk.MustQuery("select @@global.tidb_ddl_reorg_worker_cnt") @@ -533,7 +533,7 @@ func (s *testSuite3) TestSetDDLReorgBatchSize(c *C) { tk.MustQuery("show warnings;").Check(testkit.Rows("Warning 1292 Truncated incorrect tidb_ddl_reorg_batch_size value: '-1'")) tk.MustExec("set @@global.tidb_ddl_reorg_batch_size = 100") - res := tk.MustQuery("select @@tidb_ddl_reorg_batch_size") + res := tk.MustQuery("select @@global.tidb_ddl_reorg_batch_size") res.Check(testkit.Rows("100")) res = tk.MustQuery("select @@global.tidb_ddl_reorg_batch_size") diff --git a/sessionctx/variable/varsutil.go b/sessionctx/variable/varsutil.go index bcee596e810ea..9c20d2e4d261a 100644 --- a/sessionctx/variable/varsutil.go +++ b/sessionctx/variable/varsutil.go @@ -106,10 +106,6 @@ func GetSessionOnlySysVars(s *SessionVars, key string) (string, bool, error) { return strconv.FormatUint(atomic.LoadUint64(&config.GetGlobalConfig().Log.SlowThreshold), 10), true, nil case TiDBQueryLogMaxLen: return strconv.FormatUint(atomic.LoadUint64(&config.GetGlobalConfig().Log.QueryLogMaxLen), 10), true, nil - case TiDBDDLReorgWorkerCount: - return strconv.FormatInt(int64(GetDDLReorgWorkerCounter()), 10), true, nil - case TiDBDDLReorgBatchSize: - return strconv.FormatInt(int64(GetDDLReorgBatchSize()), 10), true, nil } sVal, ok := s.systems[key] if ok { From 64c7574d6cb365776f7e83bd87678a53e881e773 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Mon, 7 Jan 2019 20:15:27 +0800 Subject: [PATCH 4/6] fix ci --- session/session.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/session/session.go b/session/session.go index b2884ea2d0877..ab716a2a099d4 100644 --- a/session/session.go +++ b/session/session.go @@ -1451,8 +1451,6 @@ const loadCommonGlobalVarsSQL = "select HIGH_PRIORITY * from mysql.global_variab variable.TiDBHashAggFinalConcurrency + quoteCommaQuote + variable.TiDBBackoffLockFast + quoteCommaQuote + variable.TiDBConstraintCheckInPlace + quoteCommaQuote + - variable.TiDBDDLReorgWorkerCount + quoteCommaQuote + - variable.TiDBDDLReorgBatchSize + quoteCommaQuote + variable.TiDBOptInSubqToJoinAndAgg + quoteCommaQuote + variable.TiDBDistSQLScanConcurrency + quoteCommaQuote + variable.TiDBInitChunkSize + quoteCommaQuote + From 49176ebe19439a159985361efa38bac7f4675f77 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Tue, 8 Jan 2019 12:24:44 +0800 Subject: [PATCH 5/6] load ddl reorg variable from mysql.global_variable every time --- ddl/failtest/fail_db_test.go | 5 ++++- ddl/index.go | 15 +++++++++++++++ ddl/util/util.go | 21 +++++++++++++++++++++ executor/ddl_test.go | 26 +++++++++++++++++++++++--- session/session.go | 2 -- sessionctx/variable/session.go | 1 - sessionctx/variable/varsutil_test.go | 4 ---- 7 files changed, 63 insertions(+), 11 deletions(-) diff --git a/ddl/failtest/fail_db_test.go b/ddl/failtest/fail_db_test.go index c603b73b1c51d..adfac9551c04d 100644 --- a/ddl/failtest/fail_db_test.go +++ b/ddl/failtest/fail_db_test.go @@ -29,6 +29,7 @@ import ( "github.com/pingcap/parser/model" "github.com/pingcap/tidb/ddl" "github.com/pingcap/tidb/ddl/testutil" + ddlutil "github.com/pingcap/tidb/ddl/util" "github.com/pingcap/tidb/domain" "github.com/pingcap/tidb/kv" "github.com/pingcap/tidb/session" @@ -342,11 +343,13 @@ func (s *testFailDBSuite) TestAddIndexWorkerNum(c *C) { // Split table to multi region. s.cluster.SplitTable(s.mvccStore, tbl.Meta().ID, splitCount) + err = ddlutil.LoadDDLReorgVars(tk.Se) + c.Assert(err, IsNil) originDDLAddIndexWorkerCnt := variable.GetDDLReorgWorkerCounter() lastSetWorkerCnt := originDDLAddIndexWorkerCnt atomic.StoreInt32(&ddl.TestCheckWorkerNumber, lastSetWorkerCnt) ddl.TestCheckWorkerNumber = lastSetWorkerCnt - defer variable.SetDDLReorgWorkerCounter(originDDLAddIndexWorkerCnt) + defer tk.MustExec(fmt.Sprintf("set @@global.tidb_ddl_reorg_worker_cnt=%d", originDDLAddIndexWorkerCnt)) gofail.Enable("github.com/pingcap/tidb/ddl/checkIndexWorkerNum", `return(true)`) defer gofail.Disable("github.com/pingcap/tidb/ddl/checkIndexWorkerNum") diff --git a/ddl/index.go b/ddl/index.go index d70e0fb13736b..da659754a2509 100644 --- a/ddl/index.go +++ b/ddl/index.go @@ -23,6 +23,7 @@ import ( "github.com/pingcap/parser/ast" "github.com/pingcap/parser/model" "github.com/pingcap/parser/mysql" + ddlutil "github.com/pingcap/tidb/ddl/util" "github.com/pingcap/tidb/expression" "github.com/pingcap/tidb/infoschema" "github.com/pingcap/tidb/kv" @@ -1070,6 +1071,17 @@ var ( TestCheckWorkerNumber = int32(16) ) +func loadDDLReorgVars(w *worker) error { + // Get sessionctx from context resource pool. + var ctx sessionctx.Context + ctx, err := w.sessPool.get() + if err != nil { + return errors.Trace(err) + } + defer w.sessPool.put(ctx) + return ddlutil.LoadDDLReorgVars(ctx) +} + // addPhysicalTableIndex handles the add index reorganization state for a non-partitioned table or a partition. // For a partitioned table, it should be handled partition by partition. // @@ -1110,6 +1122,9 @@ func (w *worker) addPhysicalTableIndex(t table.PhysicalTable, indexInfo *model.I } // For dynamic adjust add index worker number. + if err := loadDDLReorgVars(w); err != nil { + log.Error(err) + } workerCnt = variable.GetDDLReorgWorkerCounter() // If only have 1 range, we can only start 1 worker. if len(kvRanges) < int(workerCnt) { diff --git a/ddl/util/util.go b/ddl/util/util.go index 9635c66380db2..24c97625827b3 100644 --- a/ddl/util/util.go +++ b/ddl/util/util.go @@ -17,6 +17,7 @@ import ( "context" "encoding/hex" "fmt" + "github.com/pingcap/tidb/sessionctx/variable" "github.com/pingcap/errors" "github.com/pingcap/parser/terror" @@ -128,3 +129,23 @@ func UpdateDeleteRange(ctx sessionctx.Context, dr DelRangeTask, newStartKey, old _, err := ctx.(sqlexec.SQLExecutor).Execute(context.TODO(), sql) return errors.Trace(err) } + +const loadDDLReorgVarsSQL = "select HIGH_PRIORITY variable_name, variable_value from mysql.global_variables where variable_name in ('" + + variable.TiDBDDLReorgWorkerCount + "', '" + + variable.TiDBDDLReorgBatchSize + "')" + +// LoadDDLReorgVars loads ddl reorg variable from mysql.global_variables. +func LoadDDLReorgVars(ctx sessionctx.Context) error { + if sctx, ok := ctx.(sqlexec.RestrictedSQLExecutor); ok { + rows, _, err := sctx.ExecRestrictedSQL(ctx, loadDDLReorgVarsSQL) + if err != nil { + return errors.Trace(err) + } + for _, row := range rows { + varName := row.GetString(0) + varValue := row.GetString(1) + variable.SetLocalSystemVar(varName, varValue) + } + } + return nil +} diff --git a/executor/ddl_test.go b/executor/ddl_test.go index 606c51b3e4bbd..3778bec4a535d 100644 --- a/executor/ddl_test.go +++ b/executor/ddl_test.go @@ -25,6 +25,7 @@ import ( "github.com/pingcap/parser/mysql" "github.com/pingcap/parser/terror" "github.com/pingcap/tidb/ddl" + ddlutil "github.com/pingcap/tidb/ddl/util" "github.com/pingcap/tidb/domain" plannercore "github.com/pingcap/tidb/planner/core" "github.com/pingcap/tidb/sessionctx/variable" @@ -491,14 +492,22 @@ func (s *testSuite3) TestMaxHandleAddIndex(c *C) { func (s *testSuite3) TestSetDDLReorgWorkerCnt(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec("use test") + err := ddlutil.LoadDDLReorgVars(tk.Se) + c.Assert(err, IsNil) c.Assert(variable.GetDDLReorgWorkerCounter(), Equals, int32(variable.DefTiDBDDLReorgWorkerCount)) tk.MustExec("set @@global.tidb_ddl_reorg_worker_cnt = 1") + err = ddlutil.LoadDDLReorgVars(tk.Se) + c.Assert(err, IsNil) c.Assert(variable.GetDDLReorgWorkerCounter(), Equals, int32(1)) tk.MustExec("set @@global.tidb_ddl_reorg_worker_cnt = 100") + err = ddlutil.LoadDDLReorgVars(tk.Se) + c.Assert(err, IsNil) c.Assert(variable.GetDDLReorgWorkerCounter(), Equals, int32(100)) - _, err := tk.Exec("set @@global.tidb_ddl_reorg_worker_cnt = invalid_val") + _, err = tk.Exec("set @@global.tidb_ddl_reorg_worker_cnt = invalid_val") c.Assert(terror.ErrorEqual(err, variable.ErrWrongTypeForVar), IsTrue, Commentf("err %v", err)) tk.MustExec("set @@global.tidb_ddl_reorg_worker_cnt = 100") + err = ddlutil.LoadDDLReorgVars(tk.Se) + c.Assert(err, IsNil) c.Assert(variable.GetDDLReorgWorkerCounter(), Equals, int32(100)) _, err = tk.Exec("set @@global.tidb_ddl_reorg_worker_cnt = -1") c.Assert(terror.ErrorEqual(err, variable.ErrWrongValueForVar), IsTrue, Commentf("err %v", err)) @@ -508,7 +517,7 @@ func (s *testSuite3) TestSetDDLReorgWorkerCnt(c *C) { res.Check(testkit.Rows("100")) res = tk.MustQuery("select @@global.tidb_ddl_reorg_worker_cnt") - res.Check(testkit.Rows(fmt.Sprintf("%v", 100))) + res.Check(testkit.Rows("100")) tk.MustExec("set @@global.tidb_ddl_reorg_worker_cnt = 100") res = tk.MustQuery("select @@global.tidb_ddl_reorg_worker_cnt") res.Check(testkit.Rows("100")) @@ -517,17 +526,25 @@ func (s *testSuite3) TestSetDDLReorgWorkerCnt(c *C) { func (s *testSuite3) TestSetDDLReorgBatchSize(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec("use test") + err := ddlutil.LoadDDLReorgVars(tk.Se) + c.Assert(err, IsNil) c.Assert(variable.GetDDLReorgBatchSize(), Equals, int32(variable.DefTiDBDDLReorgBatchSize)) tk.MustExec("set @@global.tidb_ddl_reorg_batch_size = 1") tk.MustQuery("show warnings;").Check(testkit.Rows("Warning 1292 Truncated incorrect tidb_ddl_reorg_batch_size value: '1'")) + err = ddlutil.LoadDDLReorgVars(tk.Se) + c.Assert(err, IsNil) c.Assert(variable.GetDDLReorgBatchSize(), Equals, int32(variable.MinDDLReorgBatchSize)) tk.MustExec(fmt.Sprintf("set @@global.tidb_ddl_reorg_batch_size = %v", variable.MaxDDLReorgBatchSize+1)) tk.MustQuery("show warnings;").Check(testkit.Rows(fmt.Sprintf("Warning 1292 Truncated incorrect tidb_ddl_reorg_batch_size value: '%d'", variable.MaxDDLReorgBatchSize+1))) + err = ddlutil.LoadDDLReorgVars(tk.Se) + c.Assert(err, IsNil) c.Assert(variable.GetDDLReorgBatchSize(), Equals, int32(variable.MaxDDLReorgBatchSize)) - _, err := tk.Exec("set @@global.tidb_ddl_reorg_batch_size = invalid_val") + _, err = tk.Exec("set @@global.tidb_ddl_reorg_batch_size = invalid_val") c.Assert(terror.ErrorEqual(err, variable.ErrWrongTypeForVar), IsTrue, Commentf("err %v", err)) tk.MustExec("set @@global.tidb_ddl_reorg_batch_size = 100") + err = ddlutil.LoadDDLReorgVars(tk.Se) + c.Assert(err, IsNil) c.Assert(variable.GetDDLReorgBatchSize(), Equals, int32(100)) tk.MustExec("set @@global.tidb_ddl_reorg_batch_size = -1") tk.MustQuery("show warnings;").Check(testkit.Rows("Warning 1292 Truncated incorrect tidb_ddl_reorg_batch_size value: '-1'")) @@ -541,4 +558,7 @@ func (s *testSuite3) TestSetDDLReorgBatchSize(c *C) { tk.MustExec("set @@global.tidb_ddl_reorg_batch_size = 1000") res = tk.MustQuery("select @@global.tidb_ddl_reorg_batch_size") res.Check(testkit.Rows("1000")) + + // If do not LoadDDLReorgVars, the local variable will be the last loaded value. + c.Assert(variable.GetDDLReorgBatchSize(), Equals, int32(100)) } diff --git a/session/session.go b/session/session.go index ab716a2a099d4..e3e1cce343c5d 100644 --- a/session/session.go +++ b/session/session.go @@ -829,8 +829,6 @@ func (s *session) SetGlobalSysVar(name, value string) error { sql := fmt.Sprintf(`REPLACE %s.%s VALUES ('%s', '%s');`, mysql.SystemDB, mysql.GlobalVariablesTable, name, sVal) _, _, err = s.ExecRestrictedSQL(s, sql) - // update local - variable.SetLocalSystemVar(name, sVal) return errors.Trace(err) } diff --git a/sessionctx/variable/session.go b/sessionctx/variable/session.go index 08ebc5ba5f601..66ba563f0d422 100644 --- a/sessionctx/variable/session.go +++ b/sessionctx/variable/session.go @@ -690,7 +690,6 @@ func (s *SessionVars) SetSystemVar(name string, val string) error { case TiDBEnableWindowFunction: s.EnableWindowFunction = TiDBOptOn(val) } - SetLocalSystemVar(name, val) s.systems[name] = val return nil } diff --git a/sessionctx/variable/varsutil_test.go b/sessionctx/variable/varsutil_test.go index fa7cc100b0956..b54dd8d392b96 100644 --- a/sessionctx/variable/varsutil_test.go +++ b/sessionctx/variable/varsutil_test.go @@ -216,10 +216,6 @@ func (s *testVarsutilSuite) TestVarsutil(c *C) { SetSessionSystemVar(v, TiDBOptimizerSelectivityLevel, types.NewIntDatum(1)) c.Assert(v.OptimizerSelectivityLevel, Equals, 1) - c.Assert(GetDDLReorgWorkerCounter(), Equals, int32(DefTiDBDDLReorgWorkerCount)) - SetSessionSystemVar(v, TiDBDDLReorgWorkerCount, types.NewIntDatum(1)) - c.Assert(GetDDLReorgWorkerCounter(), Equals, int32(1)) - err = SetSessionSystemVar(v, TiDBDDLReorgWorkerCount, types.NewIntDatum(-1)) c.Assert(terror.ErrorEqual(err, ErrWrongValueForVar), IsTrue) From 36e6b0d126fbbb927c33620addbdc4822129510c Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Wed, 9 Jan 2019 13:48:56 +0800 Subject: [PATCH 6/6] address comment --- ddl/util/util.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddl/util/util.go b/ddl/util/util.go index 24c97625827b3..da6a7f38b3b31 100644 --- a/ddl/util/util.go +++ b/ddl/util/util.go @@ -17,12 +17,12 @@ import ( "context" "encoding/hex" "fmt" - "github.com/pingcap/tidb/sessionctx/variable" "github.com/pingcap/errors" "github.com/pingcap/parser/terror" "github.com/pingcap/tidb/kv" "github.com/pingcap/tidb/sessionctx" + "github.com/pingcap/tidb/sessionctx/variable" "github.com/pingcap/tidb/util/chunk" "github.com/pingcap/tidb/util/sqlexec" )