Skip to content

Commit

Permalink
variables: add constraints on tidb_super_read_only when tidb_restrict…
Browse files Browse the repository at this point in the history
…ed_read_only is turned on (#31746) (#31842)

close #31745
  • Loading branch information
ti-srebot authored Feb 17, 2022
1 parent 55f3b24 commit b0d865c
Show file tree
Hide file tree
Showing 8 changed files with 200 additions and 29 deletions.
2 changes: 2 additions & 0 deletions domain/sysvar_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,8 @@ func (do *Domain) checkEnableServerGlobalVar(name, sVal string) {
break
}
topsqlstate.GlobalState.ReportIntervalSeconds.Store(val)
case variable.TiDBSuperReadOnly:
variable.VarTiDBSuperReadOnly.Store(variable.TiDBOptOn(sVal))
case variable.TiDBRestrictedReadOnly:
variable.RestrictedReadOnly.Store(variable.TiDBOptOn(sVal))
case variable.TiDBStoreLimit:
Expand Down
2 changes: 1 addition & 1 deletion planner/optimize.go
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ func optimize(ctx context.Context, sctx sessionctx.Context, node ast.Node, is in
return nil, nil, 0, err
}

if !sctx.GetSessionVars().InRestrictedSQL && variable.RestrictedReadOnly.Load() {
if !sctx.GetSessionVars().InRestrictedSQL && variable.RestrictedReadOnly.Load() || variable.VarTiDBSuperReadOnly.Load() {
allowed, err := allowInReadOnlyMode(sctx, node)
if err != nil {
return nil, nil, 0, err
Expand Down
28 changes: 27 additions & 1 deletion sessionctx/variable/sysvar.go
Original file line number Diff line number Diff line change
Expand Up @@ -1132,7 +1132,33 @@ var defaultSysVars = []*SysVar{
errors.RedactLogEnabled.Store(s.EnableRedactLog)
return nil
}},
{Scope: ScopeGlobal, Name: TiDBRestrictedReadOnly, Value: BoolToOnOff(DefTiDBRestrictedReadOnly), Type: TypeBool},
{Scope: ScopeGlobal, Name: TiDBRestrictedReadOnly, Value: BoolToOnOff(DefTiDBRestrictedReadOnly), Type: TypeBool, SetGlobal: func(s *SessionVars, val string) error {
on := TiDBOptOn(val)
if on {
err := s.GlobalVarsAccessor.SetGlobalSysVar(TiDBSuperReadOnly, "ON")
if err != nil {
return err
}
}
RestrictedReadOnly.Store(on)
return nil
}},
{Scope: ScopeGlobal, Name: TiDBSuperReadOnly, Value: BoolToOnOff(DefTiDBSuperReadOnly), Type: TypeBool, Validation: func(vars *SessionVars, normalizedValue string, _ string, _ ScopeFlag) (string, error) {
on := TiDBOptOn(normalizedValue)
if !on {
result, err := vars.GlobalVarsAccessor.GetGlobalSysVar(TiDBRestrictedReadOnly)
if err != nil {
return normalizedValue, err
}
if TiDBOptOn(result) {
return normalizedValue, fmt.Errorf("can't turn off %s when %s is on", TiDBSuperReadOnly, TiDBRestrictedReadOnly)
}
}
return normalizedValue, nil
}, SetGlobal: func(s *SessionVars, val string) error {
VarTiDBSuperReadOnly.Store(TiDBOptOn(val))
return nil
}},
{Scope: ScopeGlobal | ScopeSession, Name: TiDBShardAllocateStep, Value: strconv.Itoa(DefTiDBShardAllocateStep), Type: TypeInt, MinValue: 1, MaxValue: uint64(math.MaxInt64), SetSession: func(s *SessionVars, val string) error {
s.ShardAllocateStep = tidbOptInt64(val, DefTiDBShardAllocateStep)
return nil
Expand Down
37 changes: 37 additions & 0 deletions sessionctx/variable/sysvar_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -540,6 +540,43 @@ func TestIsNoop(t *testing.T) {
require.True(t, sv.IsNoop)
}

func TestTiDBReadOnly(t *testing.T) {
rro := GetSysVar(TiDBRestrictedReadOnly)
sro := GetSysVar(TiDBSuperReadOnly)

vars := NewSessionVars()
mock := NewMockGlobalAccessor4Tests()
mock.SessionVars = vars
vars.GlobalVarsAccessor = mock

// turn on tidb_restricted_read_only should turn on tidb_super_read_only
require.NoError(t, mock.SetGlobalSysVar(rro.Name, "ON"))
result, err := mock.GetGlobalSysVar(sro.Name)
require.NoError(t, err)
require.Equal(t, "ON", result)

// can't turn off tidb_super_read_only if tidb_restricted_read_only is on
err = mock.SetGlobalSysVar(sro.Name, "OFF")
require.Error(t, err)
require.Equal(t, "can't turn off tidb_super_read_only when tidb_restricted_read_only is on", err.Error())

// turn off tidb_restricted_read_only won't affect tidb_super_read_only
require.NoError(t, mock.SetGlobalSysVar(rro.Name, "OFF"))
result, err = mock.GetGlobalSysVar(rro.Name)
require.NoError(t, err)
require.Equal(t, "OFF", result)

result, err = mock.GetGlobalSysVar(sro.Name)
require.NoError(t, err)
require.Equal(t, "ON", result)

// it is ok to turn off tidb_super_read_only now
require.NoError(t, mock.SetGlobalSysVar(sro.Name, "OFF"))
result, err = mock.GetGlobalSysVar(sro.Name)
require.NoError(t, err)
require.Equal(t, "OFF", result)
}

func TestInstanceScopedVars(t *testing.T) {
// This tests instance scoped variables through GetSessionOrGlobalSystemVar().
// Eventually these should be changed to use getters so that the switch
Expand Down
5 changes: 5 additions & 0 deletions sessionctx/variable/tidb_vars.go
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,9 @@ const (
// TiDBRestrictedReadOnly is meant for the cloud admin to toggle the cluster read only
TiDBRestrictedReadOnly = "tidb_restricted_read_only"

// TiDBSuperReadOnly is tidb's variant of mysql's super_read_only, which has some differences from mysql's super_read_only.
TiDBSuperReadOnly = "tidb_super_read_only"

// TiDBShardAllocateStep indicates the max size of continuous rowid shard in one transaction.
TiDBShardAllocateStep = "tidb_shard_allocate_step"
// TiDBEnableTelemetry indicates that whether usage data report to PingCAP is enabled.
Expand Down Expand Up @@ -767,6 +770,7 @@ const (
DefTiDBEnableClusteredIndex = ClusteredIndexDefModeIntOnly
DefTiDBRedactLog = false
DefTiDBRestrictedReadOnly = false
DefTiDBSuperReadOnly = false
DefTiDBShardAllocateStep = math.MaxInt64
DefTiDBEnableTelemetry = true
DefTiDBEnableParallelApply = false
Expand Down Expand Up @@ -823,6 +827,7 @@ var (
MaxTSOBatchWaitInterval = atomic.NewFloat64(DefTiDBTSOClientBatchMaxWaitTime)
EnableTSOFollowerProxy = atomic.NewBool(DefTiDBEnableTSOFollowerProxy)
RestrictedReadOnly = atomic.NewBool(DefTiDBRestrictedReadOnly)
VarTiDBSuperReadOnly = atomic.NewBool(DefTiDBSuperReadOnly)
PersistAnalyzeOptions = atomic.NewBool(DefTiDBPersistAnalyzeOptions)
EnableColumnTracking = atomic.NewBool(DefTiDBEnableColumnTracking)
StatsLoadSyncWait = atomic.NewInt64(DefTiDBStatsLoadSyncWait)
Expand Down
2 changes: 1 addition & 1 deletion tests/readonlytest/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ require (
github.com/go-sql-driver/mysql v1.6.0
github.com/pingcap/tidb v2.0.11+incompatible
github.com/stretchr/testify v1.7.0
go.uber.org/goleak v1.1.11
go.uber.org/goleak v1.1.12
)

replace github.com/pingcap/tidb => ../../
Expand Down
Loading

0 comments on commit b0d865c

Please sign in to comment.