Skip to content

Commit

Permalink
executor: fix the data race on TestDataForTableStatsField and TestPar…
Browse files Browse the repository at this point in the history
…titionsTable (pingcap#15260)
  • Loading branch information
reafans authored Mar 10, 2020
1 parent e4762f2 commit 12fa28f
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 29 deletions.
28 changes: 10 additions & 18 deletions executor/infoschema_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,7 @@ func getDataAndIndexLength(info *model.TableInfo, physicalID int64, rowCount uin
}

type statsCache struct {
mu sync.Mutex
loading bool
mu sync.RWMutex
modifyTime time.Time
tableRows map[int64]uint64
colLength map[tableHistID]uint64
Expand All @@ -193,39 +192,32 @@ var tableStatsCache = &statsCache{}
// TableStatsCacheExpiry is the expiry time for table stats cache.
var TableStatsCacheExpiry = 3 * time.Second

func (c *statsCache) setLoading(loading bool) {
c.mu.Lock()
c.loading = loading
c.mu.Unlock()
}

func (c *statsCache) get(ctx sessionctx.Context) (map[int64]uint64, map[tableHistID]uint64, error) {
c.mu.Lock()
if time.Since(c.modifyTime) < TableStatsCacheExpiry || c.loading {
c.mu.RLock()
if time.Since(c.modifyTime) < TableStatsCacheExpiry {
tableRows, colLength := c.tableRows, c.colLength
c.mu.Unlock()
c.mu.RUnlock()
return tableRows, colLength, nil
}
c.loading = true
c.mu.Unlock()
c.mu.RUnlock()

c.mu.Lock()
defer c.mu.Unlock()
if time.Since(c.modifyTime) < TableStatsCacheExpiry {
return c.tableRows, c.colLength, nil
}
tableRows, err := getRowCountAllTable(ctx)
if err != nil {
c.setLoading(false)
return nil, nil, err
}
colLength, err := getColLengthAllTables(ctx)
if err != nil {
c.setLoading(false)
return nil, nil, err
}

c.mu.Lock()
c.loading = false
c.tableRows = tableRows
c.colLength = colLength
c.modifyTime = time.Now()
c.mu.Unlock()
return tableRows, colLength, nil
}

Expand Down
39 changes: 28 additions & 11 deletions executor/infoschema_reader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ package executor_test

import (
"strconv"
"sync"

. "github.com/pingcap/check"
"github.com/pingcap/parser/auth"
Expand All @@ -29,11 +28,34 @@ import (

var _ = Suite(&testInfoschemaTableSuite{})

// this SerialSuites is used to solve the data race caused by TableStatsCacheExpiry,
// if your test not change the TableStatsCacheExpiry variable, please use testInfoschemaTableSuite for test.
var _ = SerialSuites(&testInfoschemaTableSerialSuite{})

type testInfoschemaTableSuite struct {
store kv.Storage
dom *domain.Domain
// mu is used to protect the TableStatsCacheExpiry global variable.
mu sync.Mutex
}

type testInfoschemaTableSerialSuite struct {
store kv.Storage
dom *domain.Domain
}

func (s *testInfoschemaTableSerialSuite) SetUpSuite(c *C) {
store, dom, err := newStoreWithBootstrap()
c.Assert(err, IsNil)
s.store = store
s.dom = dom
originCfg := config.GetGlobalConfig()
newConf := *originCfg
newConf.OOMAction = config.OOMActionLog
config.StoreGlobalConfig(&newConf)
}

func (s *testInfoschemaTableSerialSuite) TearDownSuite(c *C) {
s.dom.Close()
s.store.Close()
}

func (s *testInfoschemaTableSuite) SetUpSuite(c *C) {
Expand Down Expand Up @@ -216,13 +238,11 @@ func (s *testInfoschemaTableSuite) TestUserPrivileges(c *C) {
c.Assert(len(result.Rows()), Greater, 0)
}

func (s *testInfoschemaTableSuite) TestDataForTableStatsField(c *C) {
s.mu.Lock()
func (s *testInfoschemaTableSerialSuite) TestDataForTableStatsField(c *C) {
s.dom.SetStatsUpdating(true)
oldExpiryTime := executor.TableStatsCacheExpiry
executor.TableStatsCacheExpiry = 0
defer func() { executor.TableStatsCacheExpiry = oldExpiryTime }()

do := s.dom
h := do.StatsHandle()
h.Clear()
Expand Down Expand Up @@ -265,15 +285,13 @@ func (s *testInfoschemaTableSuite) TestDataForTableStatsField(c *C) {
c.Assert(h.Update(is), IsNil)
tk.MustQuery("select table_rows, avg_row_length, data_length, index_length from information_schema.tables where table_name='t'").Check(
testkit.Rows("3 18 54 6"))
s.mu.Unlock()
}

func (s *testInfoschemaTableSuite) TestPartitionsTable(c *C) {
s.mu.Lock()
func (s *testInfoschemaTableSerialSuite) TestPartitionsTable(c *C) {
s.dom.SetStatsUpdating(true)
oldExpiryTime := executor.TableStatsCacheExpiry
executor.TableStatsCacheExpiry = 0
defer func() { executor.TableStatsCacheExpiry = oldExpiryTime }()

do := s.dom
h := do.StatsHandle()
h.Clear()
Expand Down Expand Up @@ -319,5 +337,4 @@ func (s *testInfoschemaTableSuite) TestPartitionsTable(c *C) {
testkit.Rows("<nil> 3 18 54 6"))

tk.MustExec("DROP TABLE `test_partitions`;")
s.mu.Unlock()
}

0 comments on commit 12fa28f

Please sign in to comment.