Skip to content

Commit

Permalink
Merge branch 'master' into fix-table-storage-stats
Browse files Browse the repository at this point in the history
  • Loading branch information
AilinKid authored Jul 19, 2021
2 parents a17fe78 + 672f3fa commit 60ec9c6
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 45 deletions.
3 changes: 1 addition & 2 deletions domain/domain.go
Original file line number Diff line number Diff line change
Expand Up @@ -622,11 +622,10 @@ func (do *Domain) Close() {
terror.Log(errors.Trace(do.etcdClient.Close()))
}

do.sysSessionPool.Close()
do.slowQuery.Close()

do.cancel()
do.wg.Wait()
do.sysSessionPool.Close()
logutil.BgLogger().Info("domain closed", zap.Duration("take time", time.Since(startTime)))
}

Expand Down
3 changes: 3 additions & 0 deletions executor/infoschema_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -1933,6 +1933,9 @@ func (e *memtableRetriever) dataForTableTiFlashReplica(ctx sessionctx.Context, s
}

func (e *memtableRetriever) setDataForStatementsSummaryEvicted(ctx sessionctx.Context) error {
if !hasPriv(ctx, mysql.ProcessPriv) {
return plannercore.ErrSpecificAccessDenied.GenWithStackByArgs("PROCESS")
}
e.rows = stmtsummary.StmtSummaryByDigestMap.ToEvictedCountDatum()
switch e.table.Name.O {
case infoschema.ClusterTableStatementsSummaryEvicted:
Expand Down
23 changes: 23 additions & 0 deletions infoschema/tables_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1436,6 +1436,29 @@ func (s *testClusterTableSuite) TestStmtSummaryEvictedCountTable(c *C) {
Check(testkit.Rows("2"))
// TODO: Add more tests.

tk.MustExec("create user 'testuser'@'localhost'")
tk.MustExec("create user 'testuser2'@'localhost'")
tk.MustExec("grant process on *.* to 'testuser2'@'localhost'")
tk1 := s.newTestKitWithRoot(c)
defer tk1.MustExec("drop user 'testuser'@'localhost'")
defer tk1.MustExec("drop user 'testuser2'@'localhost'")

c.Assert(tk.Se.Auth(&auth.UserIdentity{
Username: "testuser",
Hostname: "localhost",
}, nil, nil), Equals, true)

err := tk.QueryToErr("select * from information_schema.CLUSTER_STATEMENTS_SUMMARY_EVICTED")
c.Assert(err, NotNil)
// This error is come from cop(TiDB) fetch from rpc server.
c.Assert(err.Error(), Equals, "other error: [planner:1227]Access denied; you need (at least one of) the PROCESS privilege(s) for this operation")

c.Assert(tk.Se.Auth(&auth.UserIdentity{
Username: "testuser2",
Hostname: "localhost",
}, nil, nil), Equals, true)
err = tk.QueryToErr("select * from information_schema.CLUSTER_STATEMENTS_SUMMARY_EVICTED")
c.Assert(err, IsNil)
}

func (s *testTableSuite) TestStmtSummaryTableOther(c *C) {
Expand Down
28 changes: 12 additions & 16 deletions sessionctx/variable/sysvar.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ const (
)

// SysVar is for system variable.
// All the fields of SysVar should be READ ONLY after created.
type SysVar struct {
// Scope is for whether can be changed or not
Scope ScopeFlag
Expand Down Expand Up @@ -556,30 +557,24 @@ func UnregisterSysVar(name string) {
sysVarsLock.Unlock()
}

// Clone deep copies the sysvar struct to avoid a race
func (sv *SysVar) Clone() *SysVar {
dst := *sv
return &dst
}

// GetSysVar returns sys var info for name as key.
func GetSysVar(name string) *SysVar {
name = strings.ToLower(name)
sysVarsLock.RLock()
defer sysVarsLock.RUnlock()
if sysVars[name] == nil {
return nil
}
return sysVars[name].Clone()

return sysVars[name]
}

// SetSysVar sets a sysvar. This will not propagate to the cluster, so it should only be
// SetSysVar sets a sysvar. In fact, SysVar is immutable.
// SetSysVar is implemented by register a new SysVar with the same name again.
// This will not propagate to the cluster, so it should only be
// used for instance scoped AUTO variables such as system_time_zone.
func SetSysVar(name string, value string) {
name = strings.ToLower(name)
sysVarsLock.Lock()
defer sysVarsLock.Unlock()
sysVars[name].Value = value
old := GetSysVar(name)
tmp := *old
tmp.Value = value
RegisterSysVar(&tmp)
}

// GetSysVars deep copies the sysVars list under a RWLock
Expand All @@ -588,7 +583,8 @@ func GetSysVars() map[string]*SysVar {
defer sysVarsLock.RUnlock()
copy := make(map[string]*SysVar, len(sysVars))
for name, sv := range sysVars {
copy[name] = sv.Clone()
tmp := *sv
copy[name] = &tmp
}
return copy
}
Expand Down
27 changes: 0 additions & 27 deletions sessionctx/variable/sysvar_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -571,33 +571,6 @@ func (*testSysVarSuite) TestInstanceScopedVars(c *C) {
c.Assert(val, Equals, vars.TxnScope.GetVarValue())
}

// Calling GetSysVars/GetSysVar needs to return a deep copy, otherwise there will be data races.
// This is a bit unfortunate, since the only time the race occurs is in the testsuite (Enabling/Disabling SEM) and
// during startup (setting the .Value of ScopeNone variables). In future it might also be able
// to fix this by delaying the LoadSysVarCacheLoop start time until after the server is fully initialized.
func (*testSysVarSuite) TestDeepCopyGetSysVars(c *C) {
// Check GetSysVar
sv := SysVar{Scope: ScopeGlobal | ScopeSession, Name: "datarace", Value: On, Type: TypeBool}
RegisterSysVar(&sv)
svcopy := GetSysVar("datarace")
svcopy.Name = "datarace2"
c.Assert(sv.Name, Equals, "datarace")
c.Assert(GetSysVar("datarace").Name, Equals, "datarace")
UnregisterSysVar("datarace")

// Check GetSysVars
sv = SysVar{Scope: ScopeGlobal | ScopeSession, Name: "datarace", Value: On, Type: TypeBool}
RegisterSysVar(&sv)
for name, svcopy := range GetSysVars() {
if name == "datarace" {
svcopy.Name = "datarace2"
}
}
c.Assert(sv.Name, Equals, "datarace")
c.Assert(GetSysVar("datarace").Name, Equals, "datarace")
UnregisterSysVar("datarace")
}

// Test that sysvars defaults are logically valid. i.e.
// the default itself must validate without error provided the scope and read-only is correct.
// The default values should also be normalized for consistency.
Expand Down

0 comments on commit 60ec9c6

Please sign in to comment.