Skip to content

Commit

Permalink
Merge branch 'release-6.5' into cherry-pick-40229-to-release-6.5
Browse files Browse the repository at this point in the history
  • Loading branch information
tangenta authored Jan 4, 2023
2 parents b1a6c38 + 12f24d2 commit f922928
Show file tree
Hide file tree
Showing 7 changed files with 21 additions and 18 deletions.
8 changes: 5 additions & 3 deletions domain/domain.go
Original file line number Diff line number Diff line change
Expand Up @@ -2110,7 +2110,7 @@ func (do *Domain) NotifyUpdatePrivilege() error {
// NotifyUpdateSysVarCache updates the sysvar cache key in etcd, which other TiDB
// clients are subscribed to for updates. For the caller, the cache is also built
// synchronously so that the effect is immediate.
func (do *Domain) NotifyUpdateSysVarCache() {
func (do *Domain) NotifyUpdateSysVarCache(updateLocal bool) {
if do.etcdClient != nil {
row := do.etcdClient.KV
_, err := row.Put(context.Background(), sysVarCacheKey, "")
Expand All @@ -2119,8 +2119,10 @@ func (do *Domain) NotifyUpdateSysVarCache() {
}
}
// update locally
if err := do.rebuildSysVarCache(nil); err != nil {
logutil.BgLogger().Error("rebuilding sysvar cache failed", zap.Error(err))
if updateLocal {
if err := do.rebuildSysVarCache(nil); err != nil {
logutil.BgLogger().Error("rebuilding sysvar cache failed", zap.Error(err))
}
}
}

Expand Down
11 changes: 6 additions & 5 deletions session/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -1442,13 +1442,13 @@ func (s *session) getTableValue(ctx context.Context, tblName string, varName str

// replaceGlobalVariablesTableValue executes restricted sql updates the variable value
// It will then notify the etcd channel that the value has changed.
func (s *session) replaceGlobalVariablesTableValue(ctx context.Context, varName, val string) error {
func (s *session) replaceGlobalVariablesTableValue(ctx context.Context, varName, val string, updateLocal bool) error {
ctx = kv.WithInternalSourceType(ctx, kv.InternalTxnSysVar)
_, _, err := s.ExecRestrictedSQL(ctx, nil, `REPLACE INTO %n.%n (variable_name, variable_value) VALUES (%?, %?)`, mysql.SystemDB, mysql.GlobalVariablesTable, varName, val)
if err != nil {
return err
}
domain.GetDomain(s).NotifyUpdateSysVarCache()
domain.GetDomain(s).NotifyUpdateSysVarCache(updateLocal)
return err
}

Expand Down Expand Up @@ -1510,12 +1510,13 @@ func (s *session) SetGlobalSysVar(ctx context.Context, name string, value string
if sv.GlobalConfigName != "" {
domain.GetDomain(s).NotifyGlobalConfigChange(sv.GlobalConfigName, variable.OnOffToTrueFalse(value))
}
return s.replaceGlobalVariablesTableValue(context.TODO(), sv.Name, value)
return s.replaceGlobalVariablesTableValue(context.TODO(), sv.Name, value, true)
}

// SetGlobalSysVarOnly updates the sysvar, but does not call the validation function or update aliases.
// This is helpful to prevent duplicate warnings being appended from aliases, or recursion.
func (s *session) SetGlobalSysVarOnly(ctx context.Context, name string, value string) (err error) {
// updateLocal indicates whether to rebuild the local SysVar Cache. This is helpful to prevent recursion.
func (s *session) SetGlobalSysVarOnly(ctx context.Context, name string, value string, updateLocal bool) (err error) {
sv := variable.GetSysVar(name)
if sv == nil {
return variable.ErrUnknownSystemVar.GenWithStackByArgs(name)
Expand All @@ -1526,7 +1527,7 @@ func (s *session) SetGlobalSysVarOnly(ctx context.Context, name string, value st
if sv.HasInstanceScope() { // skip for INSTANCE scope
return nil
}
return s.replaceGlobalVariablesTableValue(ctx, sv.Name, value)
return s.replaceGlobalVariablesTableValue(ctx, sv.Name, value, updateLocal)
}

// SetTiDBTableValue implements GlobalVarAccessor.SetTiDBTableValue interface.
Expand Down
8 changes: 4 additions & 4 deletions session/session_test/session_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3778,7 +3778,7 @@ func TestUpgradeSysvars(t *testing.T) {
// i.e. implying that it was set from an earlier version of TiDB.

tk.MustExec(`REPLACE INTO mysql.global_variables (variable_name, variable_value) VALUES ('tidb_enable_noop_functions', '0')`)
domain.GetDomain(tk.Session()).NotifyUpdateSysVarCache() // update cache
domain.GetDomain(tk.Session()).NotifyUpdateSysVarCache(true) // update cache
v, err := se.GetGlobalSysVar("tidb_enable_noop_functions")
require.NoError(t, err)
require.Equal(t, "OFF", v)
Expand All @@ -3789,7 +3789,7 @@ func TestUpgradeSysvars(t *testing.T) {
// to handle upgrade/downgrade issues correctly.

tk.MustExec(`REPLACE INTO mysql.global_variables (variable_name, variable_value) VALUES ('rpl_semi_sync_slave_enabled', '')`)
domain.GetDomain(tk.Session()).NotifyUpdateSysVarCache() // update cache
domain.GetDomain(tk.Session()).NotifyUpdateSysVarCache(true) // update cache
v, err = se.GetGlobalSysVar("rpl_semi_sync_slave_enabled")
require.NoError(t, err)
require.Equal(t, "OFF", v) // the default value is restored.
Expand All @@ -3800,7 +3800,7 @@ func TestUpgradeSysvars(t *testing.T) {
// This further helps for https://github.com/pingcap/tidb/pull/28842

tk.MustExec(`REPLACE INTO mysql.global_variables (variable_name, variable_value) VALUES ('tidb_executor_concurrency', '999')`)
domain.GetDomain(tk.Session()).NotifyUpdateSysVarCache() // update cache
domain.GetDomain(tk.Session()).NotifyUpdateSysVarCache(true) // update cache
v, err = se.GetGlobalSysVar("tidb_executor_concurrency")
require.NoError(t, err)
require.Equal(t, "256", v) // the max value is restored.
Expand All @@ -3809,7 +3809,7 @@ func TestUpgradeSysvars(t *testing.T) {
// This could be the case if an ENUM sysvar removes a value.

tk.MustExec(`REPLACE INTO mysql.global_variables (variable_name, variable_value) VALUES ('tidb_enable_noop_functions', 'SOMEVAL')`)
domain.GetDomain(tk.Session()).NotifyUpdateSysVarCache() // update cache
domain.GetDomain(tk.Session()).NotifyUpdateSysVarCache(true) // update cache
v, err = se.GetGlobalSysVar("tidb_enable_noop_functions")
require.NoError(t, err)
require.Equal(t, "OFF", v) // the default value is restored.
Expand Down
2 changes: 1 addition & 1 deletion sessionctx/variable/mock_globalaccessor.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func (m *MockGlobalAccessor) SetGlobalSysVar(ctx context.Context, name string, v
}

// SetGlobalSysVarOnly implements GlobalVarAccessor.SetGlobalSysVarOnly interface.
func (m *MockGlobalAccessor) SetGlobalSysVarOnly(ctx context.Context, name string, value string) error {
func (m *MockGlobalAccessor) SetGlobalSysVarOnly(ctx context.Context, name string, value string, _ bool) error {
sv := GetSysVar(name)
if sv == nil {
return ErrUnknownSystemVar.GenWithStackByArgs(name)
Expand Down
4 changes: 2 additions & 2 deletions sessionctx/variable/mock_globalaccessor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func TestMockAPI(t *testing.T) {
// invalid option name
err = mock.SetGlobalSysVar(context.Background(), "illegalopt", "val")
require.Error(t, err)
err = mock.SetGlobalSysVarOnly(context.Background(), "illegalopt", "val")
err = mock.SetGlobalSysVarOnly(context.Background(), "illegalopt", "val", true)
require.Error(t, err)

// valid option, invalid value
Expand All @@ -46,7 +46,7 @@ func TestMockAPI(t *testing.T) {
// valid option, valid value
err = mock.SetGlobalSysVar(context.Background(), DefaultAuthPlugin, "mysql_native_password")
require.NoError(t, err)
err = mock.SetGlobalSysVarOnly(context.Background(), DefaultAuthPlugin, "mysql_native_password")
err = mock.SetGlobalSysVarOnly(context.Background(), DefaultAuthPlugin, "mysql_native_password", true)
require.NoError(t, err)

// Test GetTiDBTableValue
Expand Down
4 changes: 2 additions & 2 deletions sessionctx/variable/variable.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ func (sv *SysVar) SetGlobalFromHook(ctx context.Context, s *SessionVars, val str

if !skipAliases && sv.Aliases != nil {
for _, aliasName := range sv.Aliases {
if err := s.GlobalVarsAccessor.SetGlobalSysVarOnly(ctx, aliasName, val); err != nil {
if err := s.GlobalVarsAccessor.SetGlobalSysVarOnly(ctx, aliasName, val, true); err != nil {
return err
}
}
Expand Down Expand Up @@ -631,7 +631,7 @@ type GlobalVarAccessor interface {
// SetGlobalSysVar sets the global system variable name to value.
SetGlobalSysVar(ctx context.Context, name string, value string) error
// SetGlobalSysVarOnly sets the global system variable without calling the validation function or updating aliases.
SetGlobalSysVarOnly(ctx context.Context, name string, value string) error
SetGlobalSysVarOnly(ctx context.Context, name string, value string, updateLocal bool) error
// GetTiDBTableValue gets a value from mysql.tidb for the key 'name'
GetTiDBTableValue(name string) (string, error)
// SetTiDBTableValue sets a value+comment for the mysql.tidb key 'name'
Expand Down
2 changes: 1 addition & 1 deletion sessionctx/variable/varsutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,7 @@ func collectAllowFuncName4ExpressionIndex() string {
}

func updatePasswordValidationLength(s *SessionVars, length int32) error {
err := s.GlobalVarsAccessor.SetGlobalSysVarOnly(context.Background(), ValidatePasswordLength, strconv.FormatInt(int64(length), 10))
err := s.GlobalVarsAccessor.SetGlobalSysVarOnly(context.Background(), ValidatePasswordLength, strconv.FormatInt(int64(length), 10), false)
if err != nil {
return err
}
Expand Down

0 comments on commit f922928

Please sign in to comment.