From 37f1a21e3179bd5a8b49c4dc9d73d2661deed228 Mon Sep 17 00:00:00 2001 From: crazycs Date: Thu, 20 Jun 2019 10:47:05 +0800 Subject: [PATCH 1/4] ddl: fix race in table lock config (#10848) --- ddl/db_test.go | 31 ++++++++----------------------- ddl/serial_test.go | 20 +++++++++++++++++++- 2 files changed, 27 insertions(+), 24 deletions(-) diff --git a/ddl/db_test.go b/ddl/db_test.go index 976f03c1f40c9..ccba0f1012188 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -48,6 +48,7 @@ import ( "github.com/pingcap/tidb/tablecodec" "github.com/pingcap/tidb/types" "github.com/pingcap/tidb/util/admin" + "github.com/pingcap/tidb/util/israce" "github.com/pingcap/tidb/util/mock" "github.com/pingcap/tidb/util/testkit" "github.com/pingcap/tidb/util/testutil" @@ -92,6 +93,8 @@ func setUpSuite(s *testDBSuite, c *C) { s.schemaName = "test_db" s.autoIDStep = autoid.GetStep() ddl.WaitTimeWhenErrorOccured = 0 + // Test for table lock. + config.GetGlobalConfig().EnableTableLock = true s.cluster = mocktikv.NewCluster() mocktikv.BootstrapWithSingleStore(s.cluster) @@ -3224,18 +3227,6 @@ func (s *testDBSuite2) TestLockTables(c *C) { tk.MustExec("create table t1 (a int)") tk.MustExec("create table t2 (a int)") - // recover table lock config. - originValue := config.GetGlobalConfig().EnableTableLock - defer func() { - config.GetGlobalConfig().EnableTableLock = originValue - }() - - // Test for enable table lock config. - config.GetGlobalConfig().EnableTableLock = false - tk.MustExec("lock tables t1 write") - checkTableLock(c, tk.Se, "test", "t1", model.TableLockNone) - config.GetGlobalConfig().EnableTableLock = true - // Test lock 1 table. tk.MustExec("lock tables t1 write") checkTableLock(c, tk.Se, "test", "t1", model.TableLockWrite) @@ -3411,7 +3402,10 @@ func (s *testDBSuite2) TestLockTables(c *C) { } // TestConcurrentLockTables test concurrent lock/unlock tables. -func (s *testDBSuite2) TestConcurrentLockTables(c *C) { +func (s *testDBSuite4) TestConcurrentLockTables(c *C) { + if israce.RaceEnabled { + c.Skip("skip race test") + } s.tk = testkit.NewTestKit(c, s.store) tk2 := testkit.NewTestKit(c, s.store) tk := s.tk @@ -3421,15 +3415,6 @@ func (s *testDBSuite2) TestConcurrentLockTables(c *C) { tk.MustExec("create table t1 (a int)") tk2.MustExec("use test") - // recover table lock config. - originValue := config.GetGlobalConfig().EnableTableLock - defer func() { - config.GetGlobalConfig().EnableTableLock = originValue - }() - - // Test for enable table lock config. - config.GetGlobalConfig().EnableTableLock = true - // Test concurrent lock tables read. sql1 := "lock tables t1 read" sql2 := "lock tables t1 read" @@ -3462,7 +3447,7 @@ func (s *testDBSuite2) TestConcurrentLockTables(c *C) { tk2.MustExec("unlock tables") } -func (s *testDBSuite2) testParallelExecSQL(c *C, sql1, sql2 string, se1, se2 session.Session, f checkRet) { +func (s *testDBSuite4) testParallelExecSQL(c *C, sql1, sql2 string, se1, se2 session.Session, f checkRet) { callback := &ddl.TestDDLCallback{} times := 0 callback.OnJobRunBeforeExported = func(job *model.Job) { diff --git a/ddl/serial_test.go b/ddl/serial_test.go index d7c85c95a868f..2ba8ddd5908c4 100644 --- a/ddl/serial_test.go +++ b/ddl/serial_test.go @@ -16,7 +16,6 @@ package ddl_test import ( "context" "fmt" - "github.com/pingcap/tidb/config" "math" "strings" "sync" @@ -27,6 +26,7 @@ import ( "github.com/pingcap/failpoint" "github.com/pingcap/parser/model" "github.com/pingcap/parser/mysql" + "github.com/pingcap/tidb/config" "github.com/pingcap/tidb/ddl" "github.com/pingcap/tidb/domain" "github.com/pingcap/tidb/infoschema" @@ -802,3 +802,21 @@ func (s *testSerialSuite) TestCanceledJobTakeTime(c *C) { sub := time.Since(startTime) c.Assert(sub, Less, ddl.WaitTimeWhenErrorOccured) } + +func (s *testSerialSuite) TestTableLocksEnable(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("use test") + tk.MustExec("drop table if exists t1") + defer tk.MustExec("drop table if exists t1") + tk.MustExec("create table t1 (a int)") + // recover table lock config. + originValue := config.GetGlobalConfig().EnableTableLock + defer func() { + config.GetGlobalConfig().EnableTableLock = originValue + }() + + // Test for enable table lock config. + config.GetGlobalConfig().EnableTableLock = false + tk.MustExec("lock tables t1 write") + checkTableLock(c, tk.Se, "test", "t1", model.TableLockNone) +} From 1913622b02383064bc849f24300bb53ae428a29c Mon Sep 17 00:00:00 2001 From: crazycs Date: Thu, 4 Jul 2019 14:59:37 +0800 Subject: [PATCH 2/4] lock: add delay clean table lock when session close (#11038) --- config/config.go | 9 ++++++++- config/config.toml.example | 3 +++ config/config_test.go | 2 ++ ddl/db_test.go | 33 +++++++++++++++++++++++++++++++++ session/session.go | 3 +++ 5 files changed, 49 insertions(+), 1 deletion(-) diff --git a/config/config.go b/config/config.go index 0549be42cb539..b4f512da2914a 100644 --- a/config/config.go +++ b/config/config.go @@ -105,7 +105,8 @@ type Config struct { StmtSummary StmtSummary `toml:"stmt-summary" json:"stmt-summary"` // EnableTableLock indicate whether enable table lock. // TODO: remove this after table lock features stable. - EnableTableLock bool `toml:"enable-table-lock" json:"enable-table-lock"` + EnableTableLock bool `toml:"enable-table-lock" json:"enable-table-lock"` + DelayCleanTableLock uint64 `toml:"delay-clean-table-lock" json:"delay-clean-table-lock"` } // Log is the log section of config. @@ -380,6 +381,7 @@ var defaultConf = Config{ TreatOldVersionUTF8AsUTF8MB4: true, SplitRegionMaxNum: 1000, EnableTableLock: false, + DelayCleanTableLock: 0, TxnLocalLatches: TxnLocalLatches{ Enabled: false, Capacity: 2048000, @@ -661,6 +663,11 @@ func TableLockEnabled() bool { return GetGlobalConfig().EnableTableLock } +// TableLockDelayClean uses to get the time of delay clean table lock. +var TableLockDelayClean = func() uint64 { + return GetGlobalConfig().DelayCleanTableLock +} + // ToLogConfig converts *Log to *logutil.LogConfig. func (l *Log) ToLogConfig() *logutil.LogConfig { return logutil.NewLogConfig(l.Level, l.Format, l.SlowQueryFile, l.File, l.DisableTimestamp, func(config *zaplog.Config) { config.DisableErrorVerbose = l.DisableErrorStack }) diff --git a/config/config.toml.example b/config/config.toml.example index 594d1886672e9..afdc1d27b1782 100644 --- a/config/config.toml.example +++ b/config/config.toml.example @@ -77,6 +77,9 @@ server-version = "" # enable-table-lock is used to control table lock feature. Default is false, indicate the table lock feature is disabled. enable-table-lock = false +# delay-clean-table-lock is used to control whether delayed-release the table lock in the abnormal situation. (Milliseconds) +delay-clean-table-lock = 0 + [log] # Log level: debug, info, warn, error, fatal. level = "info" diff --git a/config/config_test.go b/config/config_test.go index 9a864c1c3c74c..c259ca3ed1901 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -70,6 +70,7 @@ split-region-max-num=10000 server-version = "test_version" max-index-length = 3080 enable-table-lock = true +delay-clean-table-lock = 5 [performance] txn-entry-count-limit=2000 txn-total-size-limit=2000 @@ -117,6 +118,7 @@ history-size=100 c.Assert(conf.StmtSummary.HistorySize, Equals, 100) c.Assert(conf.MaxIndexLength, Equals, 3080) c.Assert(conf.EnableTableLock, IsTrue) + c.Assert(conf.DelayCleanTableLock, Equals, uint64(5)) c.Assert(f.Close(), IsNil) c.Assert(os.Remove(configFile), IsNil) diff --git a/ddl/db_test.go b/ddl/db_test.go index ccba0f1012188..3731bfbeeb4d8 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -3401,6 +3401,39 @@ func (s *testDBSuite2) TestLockTables(c *C) { tk2.MustExec("unlock tables") } +func (s *testDBSuite2) TestTablesLockDelayClean(c *C) { + if israce.RaceEnabled { + c.Skip("skip race test") + } + s.tk = testkit.NewTestKit(c, s.store) + tk := s.tk + tk2 := testkit.NewTestKit(c, s.store) + tk2.MustExec("use test") + tk.MustExec("use test") + tk.MustExec("drop table if exists t1,t2") + defer tk.MustExec("drop table if exists t1,t2") + tk.MustExec("create table t1 (a int)") + tk.MustExec("create table t2 (a int)") + + tk.MustExec("lock tables t1 write") + checkTableLock(c, tk.Se, "test", "t1", model.TableLockWrite) + config.GetGlobalConfig().DelayCleanTableLock = 100 + var wg sync.WaitGroup + wg.Add(1) + var startTime time.Time + go func() { + startTime = time.Now() + tk.Se.Close() + wg.Done() + }() + time.Sleep(50) + checkTableLock(c, tk.Se, "test", "t1", model.TableLockWrite) + wg.Wait() + c.Assert(time.Since(startTime).Seconds() > 0.1, IsTrue) + checkTableLock(c, tk.Se, "test", "t1", model.TableLockNone) + config.GetGlobalConfig().DelayCleanTableLock = 0 +} + // TestConcurrentLockTables test concurrent lock/unlock tables. func (s *testDBSuite4) TestConcurrentLockTables(c *C) { if israce.RaceEnabled { diff --git a/session/session.go b/session/session.go index 9fcccf32981aa..a0f37976486be 100644 --- a/session/session.go +++ b/session/session.go @@ -1405,6 +1405,9 @@ func (s *session) Close() { // TODO: do clean table locks when session exited without execute Close. // TODO: do clean table locks when tidb-server was `kill -9`. if s.HasLockedTables() && config.TableLockEnabled() { + if ds := config.TableLockDelayClean(); ds > 0 { + time.Sleep(time.Duration(ds) * time.Millisecond) + } lockedTables := s.GetAllTableLocks() err := domain.GetDomain(s).DDL().UnlockTables(s, lockedTables) if err != nil { From 14e395c89153a68e905fadeea6fdeda89eee12d8 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Tue, 28 Apr 2020 13:08:26 +0800 Subject: [PATCH 3/4] fix race Signed-off-by: crazycs520 --- ddl/db_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ddl/db_test.go b/ddl/db_test.go index 3731bfbeeb4d8..8162fde406b9a 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -94,7 +94,10 @@ func setUpSuite(s *testDBSuite, c *C) { s.autoIDStep = autoid.GetStep() ddl.WaitTimeWhenErrorOccured = 0 // Test for table lock. - config.GetGlobalConfig().EnableTableLock = true + cfg := config.GetGlobalConfig() + newCfg := *cfg + newCfg.EnableTableLock = true + config.StoreGlobalConfig(&newCfg) s.cluster = mocktikv.NewCluster() mocktikv.BootstrapWithSingleStore(s.cluster) From dab3db9a0c9d006261bad676554a4396a9d9d2ed Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Tue, 28 Apr 2020 13:12:41 +0800 Subject: [PATCH 4/4] refine fix race Signed-off-by: crazycs520 --- ddl/db_test.go | 6 ------ ddl/ddl_test.go | 2 ++ 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/ddl/db_test.go b/ddl/db_test.go index 8162fde406b9a..163a68a18c942 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -93,12 +93,6 @@ func setUpSuite(s *testDBSuite, c *C) { s.schemaName = "test_db" s.autoIDStep = autoid.GetStep() ddl.WaitTimeWhenErrorOccured = 0 - // Test for table lock. - cfg := config.GetGlobalConfig() - newCfg := *cfg - newCfg.EnableTableLock = true - config.StoreGlobalConfig(&newCfg) - s.cluster = mocktikv.NewCluster() mocktikv.BootstrapWithSingleStore(s.cluster) s.mvccStore = mocktikv.MustNewMVCCStore() diff --git a/ddl/ddl_test.go b/ddl/ddl_test.go index c392bfea00afa..3259202c49a2d 100644 --- a/ddl/ddl_test.go +++ b/ddl/ddl_test.go @@ -115,6 +115,8 @@ func TestT(t *testing.T) { cfg := config.GetGlobalConfig() newCfg := *cfg + // Test for table lock. + newCfg.EnableTableLock = true newCfg.Log.SlowThreshold = 10000 // Test for add/drop primary key. newCfg.AlterPrimaryKey = true