Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

config: fix data race caused by config.Labels #22977

Merged
merged 17 commits into from
Mar 1, 2021
Merged
8 changes: 8 additions & 0 deletions config/config_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (

"github.com/BurntSushi/toml"
"github.com/pingcap/errors"
"github.com/pingcap/failpoint"
)

// CloneConf deeply clones this config.
Expand Down Expand Up @@ -161,6 +162,13 @@ const (

// GetTxnScopeFromConfig extracts @@txn_scope value from config
func GetTxnScopeFromConfig() (bool, string) {
failpoint.Inject("injectTxnScope", func(val failpoint.Value) {
v := val.(string)
if len(v) > 0 {
failpoint.Return(false, v)
}
failpoint.Return(true, globalTxnScope)
})
v, ok := GetGlobalConfig().Labels["zone"]
if ok && len(v) > 0 {
return false, v
Expand Down
10 changes: 7 additions & 3 deletions config/config_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (

"github.com/BurntSushi/toml"
. "github.com/pingcap/check"
"github.com/pingcap/failpoint"
)

func (s *testConfigSuite) TestCloneConf(c *C) {
Expand Down Expand Up @@ -170,16 +171,19 @@ engines = ["tikv", "tiflash", "tidb"]
}

func (s *testConfigSuite) TestTxnScopeValue(c *C) {
GetGlobalConfig().Labels["zone"] = "bj"
failpoint.Enable("github.com/pingcap/tidb/config/injectTxnScope", `return("bj")`)
isGlobal, v := GetTxnScopeFromConfig()
c.Assert(isGlobal, IsFalse)
c.Assert(v, Equals, "bj")
GetGlobalConfig().Labels["zone"] = ""
failpoint.Disable("github.com/pingcap/tidb/config/injectTxnScope")
failpoint.Enable("github.com/pingcap/tidb/config/injectTxnScope", `return("")`)
isGlobal, v = GetTxnScopeFromConfig()
c.Assert(isGlobal, IsTrue)
c.Assert(v, Equals, "global")
GetGlobalConfig().Labels["zone"] = "global"
failpoint.Disable("github.com/pingcap/tidb/config/injectTxnScope")
failpoint.Enable("github.com/pingcap/tidb/config/injectTxnScope", `return("global")`)
isGlobal, v = GetTxnScopeFromConfig()
c.Assert(isGlobal, IsFalse)
c.Assert(v, Equals, "global")
failpoint.Disable("github.com/pingcap/tidb/config/injectTxnScope")
}
19 changes: 6 additions & 13 deletions ddl/placement_sql_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ import (
"sort"

. "github.com/pingcap/check"
"github.com/pingcap/failpoint"
"github.com/pingcap/parser/model"
"github.com/pingcap/tidb/config"
"github.com/pingcap/tidb/ddl"
"github.com/pingcap/tidb/ddl/placement"
"github.com/pingcap/tidb/session"
Expand Down Expand Up @@ -440,9 +440,6 @@ func (s *testDBSuite1) TestPlacementPolicyCache(c *C) {
}

func (s *testSerialDBSuite) TestTxnScopeConstraint(c *C) {
defer func() {
config.GetGlobalConfig().Labels["zone"] = ""
}()
tk := testkit.NewTestKit(c, s.store)
tk.MustExec("use test")
tk.MustExec("drop table if exists t1")
Expand Down Expand Up @@ -569,9 +566,8 @@ PARTITION BY RANGE (c) (

for _, testcase := range testCases {
c.Log(testcase.name)
config.GetGlobalConfig().Labels = map[string]string{
"zone": testcase.zone,
}
failpoint.Enable("github.com/pingcap/tidb/config/injectTxnScope",
fmt.Sprintf(`return("%v")`, testcase.zone))
se, err := session.CreateSession4Test(s.store)
c.Check(err, IsNil)
tk.Se = se
Expand All @@ -591,6 +587,7 @@ PARTITION BY RANGE (c) (
c.Assert(err, NotNil)
c.Assert(err.Error(), Matches, testcase.err.Error())
}
failpoint.Disable("github.com/pingcap/tidb/config/injectTxnScope")
}
}

Expand Down Expand Up @@ -661,9 +658,6 @@ add placement policy
}

func (s *testSerialSuite) TestGlobalTxnState(c *C) {
defer func() {
config.GetGlobalConfig().Labels["zone"] = ""
}()
tk := testkit.NewTestKit(c, s.store)
tk.MustExec("use test")
tk.MustExec("drop table if exists t1")
Expand Down Expand Up @@ -704,9 +698,8 @@ PARTITION BY RANGE (c) (
},
},
}
config.GetGlobalConfig().Labels = map[string]string{
"zone": "bj",
}
failpoint.Enable("github.com/pingcap/tidb/config/injectTxnScope", `return("bj")`)
defer failpoint.Disable("github.com/pingcap/tidb/config/injectTxnScope")
dbInfo := testGetSchemaByName(c, tk.Se, "test")
tk2 := testkit.NewTestKit(c, s.store)
var chkErr error
Expand Down
9 changes: 3 additions & 6 deletions executor/executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7480,9 +7480,6 @@ func (s *testSuite) TestIssue15563(c *C) {
}

func (s *testSerialSuite) TestStalenessTransaction(c *C) {
defer func() {
config.GetGlobalConfig().Labels["zone"] = ""
}()
c.Assert(failpoint.Enable("github.com/pingcap/tidb/executor/mockStalenessTxnSchemaVer", "return(false)"), IsNil)
defer failpoint.Disable("github.com/pingcap/tidb/executor/mockStalenessTxnSchemaVer")
testcases := []struct {
Expand Down Expand Up @@ -7544,9 +7541,8 @@ func (s *testSerialSuite) TestStalenessTransaction(c *C) {
tk.MustExec("use test")
for _, testcase := range testcases {
c.Log(testcase.name)
config.GetGlobalConfig().Labels = map[string]string{
"zone": testcase.zone,
}
failpoint.Enable("github.com/pingcap/tidb/config/injectTxnScope",
fmt.Sprintf(`return("%v")`, testcase.zone))
tk.MustExec(fmt.Sprintf("set @@txn_scope=%v", testcase.txnScope))
tk.MustExec(testcase.preSQL)
tk.MustExec(testcase.sql)
Expand All @@ -7565,6 +7561,7 @@ func (s *testSerialSuite) TestStalenessTransaction(c *C) {
}
c.Assert(tk.Se.GetSessionVars().TxnCtx.IsStaleness, Equals, testcase.IsStaleness)
tk.MustExec("commit")
failpoint.Disable("github.com/pingcap/tidb/config/injectTxnScope")
}
}

Expand Down
11 changes: 4 additions & 7 deletions expression/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ import (

. "github.com/pingcap/check"
"github.com/pingcap/errors"
"github.com/pingcap/failpoint"
"github.com/pingcap/parser/auth"
"github.com/pingcap/parser/model"
"github.com/pingcap/parser/mysql"
"github.com/pingcap/parser/terror"
"github.com/pingcap/tidb/config"
"github.com/pingcap/tidb/ddl/placement"
"github.com/pingcap/tidb/domain"
"github.com/pingcap/tidb/expression"
Expand Down Expand Up @@ -8538,9 +8538,6 @@ func (s *testIntegrationSuite) TestIssue12209(c *C) {
}

func (s *testIntegrationSerialSuite) TestCrossDCQuery(c *C) {
defer func() {
config.GetGlobalConfig().Labels["zone"] = ""
}()
tk := testkit.NewTestKit(c, s.store)
tk.MustExec("use test")
tk.MustExec("drop table if exists t1")
Expand Down Expand Up @@ -8656,9 +8653,8 @@ PARTITION BY RANGE (c) (
}
for _, testcase := range testcases {
c.Log(testcase.name)
config.GetGlobalConfig().Labels = map[string]string{
"zone": testcase.zone,
}
failpoint.Enable("github.com/pingcap/tidb/config/injectTxnScope",
fmt.Sprintf(`return("%v")`, testcase.zone))
_, err = tk.Exec(fmt.Sprintf("set @@txn_scope='%v'", testcase.txnScope))
c.Assert(err, IsNil)
res, err := tk.Exec(testcase.sql)
Expand All @@ -8675,6 +8671,7 @@ PARTITION BY RANGE (c) (
} else {
c.Assert(checkErr, IsNil)
}
failpoint.Disable("github.com/pingcap/tidb/config/injectTxnScope")
}
}

Expand Down
18 changes: 6 additions & 12 deletions session/session_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3257,10 +3257,7 @@ func (s *testSessionSuite2) TestPerStmtTaskID(c *C) {
}

func (s *testSessionSerialSuite) TestSetTxnScope(c *C) {
defer func() {
config.GetGlobalConfig().Labels["zone"] = ""
}()
config.GetGlobalConfig().Labels["zone"] = ""
failpoint.Enable("github.com/pingcap/tidb/config/injectTxnScope", `return("")`)
tk := testkit.NewTestKitWithInit(c, s.store)
// assert default value
result := tk.MustQuery("select @@txn_scope;")
Expand All @@ -3271,8 +3268,9 @@ func (s *testSessionSerialSuite) TestSetTxnScope(c *C) {
result = tk.MustQuery("select @@txn_scope;")
result.Check(testkit.Rows(oracle.GlobalTxnScope))
c.Assert(tk.Se.GetSessionVars().CheckAndGetTxnScope(), Equals, oracle.GlobalTxnScope)

config.GetGlobalConfig().Labels["zone"] = "bj"
failpoint.Disable("github.com/pingcap/tidb/config/injectTxnScope")
failpoint.Enable("github.com/pingcap/tidb/config/injectTxnScope", `return("bj")`)
defer failpoint.Disable("github.com/pingcap/tidb/config/injectTxnScope")
tk = testkit.NewTestKitWithInit(c, s.store)
// assert default value
result = tk.MustQuery("select @@txn_scope;")
Expand All @@ -3291,9 +3289,6 @@ func (s *testSessionSerialSuite) TestSetTxnScope(c *C) {
}

func (s *testSessionSerialSuite) TestGlobalAndLocalTxn(c *C) {
defer func() {
config.GetGlobalConfig().Labels["zone"] = ""
}()
// Because the PD config of check_dev_2 test is not compatible with local/global txn yet,
// so we will skip this test for now.
if *withTiKV {
Expand Down Expand Up @@ -3383,9 +3378,8 @@ PARTITION BY RANGE (c) (
result = tk.MustQuery("select * from t1") // read dc-1 and dc-2 with global scope
c.Assert(len(result.Rows()), Equals, 3)

config.GetGlobalConfig().Labels = map[string]string{
"zone": "dc-1",
}
failpoint.Enable("github.com/pingcap/tidb/config/injectTxnScope", `return("dc-1")`)
defer failpoint.Disable("github.com/pingcap/tidb/config/injectTxnScope")
// set txn_scope to local
tk.MustExec("set @@session.txn_scope = 'local';")
result = tk.MustQuery("select @@txn_scope;")
Expand Down