From 205b5bbd210423aa7690fcd31a7e4550e0a5618a Mon Sep 17 00:00:00 2001 From: jiyfhust Date: Thu, 11 Jan 2024 18:52:55 +0800 Subject: [PATCH] variable: fix information_schema.VARIABLES_INFO DEFAULT_VALUE not right problem (#49524) close pingcap/tidb#49461, close pingcap/tidb#49968, close pingcap/tidb#50155 --- pkg/executor/infoschema_reader.go | 6 +- pkg/executor/set.go | 3 +- pkg/session/bootstrap.go | 32 +--------- pkg/session/test/variable/variable_test.go | 4 ++ pkg/sessionctx/variable/BUILD.bazel | 1 + pkg/sessionctx/variable/sysvar.go | 32 ++++++++++ pkg/sessionctx/variable/sysvar_test.go | 58 +++++++++++++++++++ .../r/executor/infoschema_reader.result | 28 +++++++++ .../t/executor/infoschema_reader.test | 12 ++++ 9 files changed, 143 insertions(+), 33 deletions(-) diff --git a/pkg/executor/infoschema_reader.go b/pkg/executor/infoschema_reader.go index afff3420d8bf6..5013669a05c6e 100644 --- a/pkg/executor/infoschema_reader.go +++ b/pkg/executor/infoschema_reader.go @@ -264,10 +264,14 @@ func (e *memtableRetriever) setDataForVariablesInfo(ctx sessionctx.Context) erro if sv.IsNoop { isNoop = "YES" } + defVal := sv.Value + if sv.HasGlobalScope() { + defVal = variable.GlobalSystemVariableInitialValue(sv.Name, defVal) + } row := types.MakeDatums( sv.Name, // VARIABLE_NAME sv.Scope.String(), // VARIABLE_SCOPE - sv.Value, // DEFAULT_VALUE + defVal, // DEFAULT_VALUE currentVal, // CURRENT_VALUE sv.MinValue, // MIN_VALUE sv.MaxValue, // MAX_VALUE diff --git a/pkg/executor/set.go b/pkg/executor/set.go index c20acce07dff6..37d13806947b9 100644 --- a/pkg/executor/set.go +++ b/pkg/executor/set.go @@ -301,7 +301,8 @@ func (e *SetExecutor) getVarValue(ctx context.Context, v *expression.VarAssignme // to the compiled-in MySQL default value, use the DEFAULT keyword. // See http://dev.mysql.com/doc/refman/5.7/en/set-statement.html if sysVar != nil { - return sysVar.Value, nil + defVal := variable.GlobalSystemVariableInitialValue(sysVar.Name, sysVar.Value) + return defVal, nil } return e.Ctx().GetSessionVars().GetGlobalSystemVar(ctx, v.Name) } diff --git a/pkg/session/bootstrap.go b/pkg/session/bootstrap.go index 428d2dd2701a6..66277ab97d327 100644 --- a/pkg/session/bootstrap.go +++ b/pkg/session/bootstrap.go @@ -3194,37 +3194,7 @@ func doDMLWorks(s sessiontypes.Session) { if !v.HasGlobalScope() { continue } - vVal := v.Value - switch v.Name { - case variable.TiDBTxnMode: - if config.GetGlobalConfig().Store == "tikv" || config.GetGlobalConfig().Store == "unistore" { - vVal = "pessimistic" - } - case variable.TiDBEnableAsyncCommit, variable.TiDBEnable1PC: - if config.GetGlobalConfig().Store == "tikv" { - vVal = variable.On - } - case variable.TiDBMemOOMAction: - if intest.InTest { - vVal = variable.OOMActionLog - } - case variable.TiDBEnableAutoAnalyze: - if intest.InTest { - vVal = variable.Off - } - // For the following sysvars, we change the default - // FOR NEW INSTALLS ONLY. In most cases you don't want to do this. - // It is better to change the value in the Sysvar struct, so that - // all installs will have the same value. - case variable.TiDBRowFormatVersion: - vVal = strconv.Itoa(variable.DefTiDBRowFormatV2) - case variable.TiDBTxnAssertionLevel: - vVal = variable.AssertionFastStr - case variable.TiDBEnableMutationChecker: - vVal = variable.On - case variable.TiDBPessimisticTransactionFairLocking: - vVal = variable.On - } + vVal := variable.GlobalSystemVariableInitialValue(v.Name, v.Value) // sanitize k and vVal value := fmt.Sprintf(`("%s", "%s")`, sqlescape.EscapeString(k), sqlescape.EscapeString(vVal)) diff --git a/pkg/session/test/variable/variable_test.go b/pkg/session/test/variable/variable_test.go index 94a04dc8f454a..005d9eb122688 100644 --- a/pkg/session/test/variable/variable_test.go +++ b/pkg/session/test/variable/variable_test.go @@ -121,6 +121,8 @@ func TestCoprocessorOOMAction(t *testing.T) { disableOOM := func(tk *testkit.TestKit, name, sql string) { t.Logf("disable OOM, testcase: %v", name) quota := 5*copr.MockResponseSizeForTest - 100 + tk.MustExec("SET GLOBAL tidb_mem_oom_action='CANCEL'") + defer tk.MustExec("SET GLOBAL tidb_mem_oom_action = DEFAULT") tk.MustExec("use testoom") tk.MustExec("set @@tidb_enable_rate_limit_action=0") tk.MustExec("set @@tidb_distsql_scan_concurrency = 10") @@ -172,9 +174,11 @@ func TestCoprocessorOOMAction(t *testing.T) { tk.MustExec("use testoom") tk.MustExec("set tidb_distsql_scan_concurrency = 1") tk.MustExec("set @@tidb_mem_quota_query=1;") + tk.MustExec("SET GLOBAL tidb_mem_oom_action='CANCEL'") err = tk.QueryToErr(testcase.sql) require.Error(t, err) require.True(t, exeerrors.ErrMemoryExceedForQuery.Equal(err)) + tk.MustExec("SET GLOBAL tidb_mem_oom_action = DEFAULT") se.Close() } } diff --git a/pkg/sessionctx/variable/BUILD.bazel b/pkg/sessionctx/variable/BUILD.bazel index b416a7dd46928..d7ad78566993b 100644 --- a/pkg/sessionctx/variable/BUILD.bazel +++ b/pkg/sessionctx/variable/BUILD.bazel @@ -50,6 +50,7 @@ go_library( "//pkg/util/distrole", "//pkg/util/execdetails", "//pkg/util/gctuner", + "//pkg/util/intest", "//pkg/util/kvcache", "//pkg/util/logutil", "//pkg/util/mathutil", diff --git a/pkg/sessionctx/variable/sysvar.go b/pkg/sessionctx/variable/sysvar.go index 4bba2721d0800..465af49cf6f4f 100644 --- a/pkg/sessionctx/variable/sysvar.go +++ b/pkg/sessionctx/variable/sysvar.go @@ -44,6 +44,7 @@ import ( "github.com/pingcap/tidb/pkg/util/collate" distroleutil "github.com/pingcap/tidb/pkg/util/distrole" "github.com/pingcap/tidb/pkg/util/gctuner" + "github.com/pingcap/tidb/pkg/util/intest" "github.com/pingcap/tidb/pkg/util/logutil" "github.com/pingcap/tidb/pkg/util/mathutil" "github.com/pingcap/tidb/pkg/util/memory" @@ -3002,6 +3003,37 @@ var defaultSysVars = []*SysVar{ }}, } +// GlobalSystemVariableInitialValue gets the default value for a system variable including ones that are dynamically set (e.g. based on the store) +func GlobalSystemVariableInitialValue(varName, varVal string) string { + switch varName { + case TiDBEnableAsyncCommit, TiDBEnable1PC: + if config.GetGlobalConfig().Store == "tikv" { + varVal = On + } + case TiDBMemOOMAction: + if intest.InTest { + varVal = OOMActionLog + } + case TiDBEnableAutoAnalyze: + if intest.InTest { + varVal = Off + } + // For the following sysvars, we change the default + // FOR NEW INSTALLS ONLY. In most cases you don't want to do this. + // It is better to change the value in the Sysvar struct, so that + // all installs will have the same value. + case TiDBRowFormatVersion: + varVal = strconv.Itoa(DefTiDBRowFormatV2) + case TiDBTxnAssertionLevel: + varVal = AssertionFastStr + case TiDBEnableMutationChecker: + varVal = On + case TiDBPessimisticTransactionFairLocking: + varVal = On + } + return varVal +} + func setTiFlashComputeDispatchPolicy(s *SessionVars, val string) error { p, err := tiflashcompute.GetDispatchPolicyByStr(val) if err != nil { diff --git a/pkg/sessionctx/variable/sysvar_test.go b/pkg/sessionctx/variable/sysvar_test.go index e275aa7be2fde..0408cff31d293 100644 --- a/pkg/sessionctx/variable/sysvar_test.go +++ b/pkg/sessionctx/variable/sysvar_test.go @@ -1453,3 +1453,61 @@ func TestSetTiDBCloudStorageURI(t *testing.T) { require.Len(t, val, 0) cancel() } + +func TestGlobalSystemVariableInitialValue(t *testing.T) { + vars := []struct { + name string + val string + initVal string + }{ + { + TiDBTxnMode, + DefTiDBTxnMode, + "pessimistic", + }, + { + TiDBEnableAsyncCommit, + BoolToOnOff(DefTiDBEnableAsyncCommit), + BoolToOnOff(DefTiDBEnableAsyncCommit), + }, + { + TiDBEnable1PC, + BoolToOnOff(DefTiDBEnable1PC), + BoolToOnOff(DefTiDBEnable1PC), + }, + { + TiDBMemOOMAction, + DefTiDBMemOOMAction, + OOMActionLog, + }, + { + TiDBEnableAutoAnalyze, + BoolToOnOff(DefTiDBEnableAutoAnalyze), + Off, + }, + { + TiDBRowFormatVersion, + strconv.Itoa(DefTiDBRowFormatV1), + strconv.Itoa(DefTiDBRowFormatV2), + }, + { + TiDBTxnAssertionLevel, + DefTiDBTxnAssertionLevel, + AssertionFastStr, + }, + { + TiDBEnableMutationChecker, + BoolToOnOff(DefTiDBEnableMutationChecker), + On, + }, + { + TiDBPessimisticTransactionFairLocking, + BoolToOnOff(DefTiDBPessimisticTransactionFairLocking), + On, + }, + } + for _, v := range vars { + initVal := GlobalSystemVariableInitialValue(v.name, v.val) + require.Equal(t, v.initVal, initVal) + } +} diff --git a/tests/integrationtest/r/executor/infoschema_reader.result b/tests/integrationtest/r/executor/infoschema_reader.result index 15ab1dfb7ce46..4745b06b79a02 100644 --- a/tests/integrationtest/r/executor/infoschema_reader.result +++ b/tests/integrationtest/r/executor/infoschema_reader.result @@ -283,3 +283,31 @@ SELECT * FROM information_schema.user_privileges WHERE grantee="'usageuser'@'%'" GRANTEE TABLE_CATALOG PRIVILEGE_TYPE IS_GRANTABLE 'usageuser'@'%' def BACKUP_ADMIN NO 'usageuser'@'%' def SELECT YES +select VARIABLE_NAME from information_schema.VARIABLES_INFO where DEFAULT_VALUE = CURRENT_VALUE and variable_name in ('tidb_enable_async_commit','tidb_enable_1pc', 'tidb_mem_oom_action', 'tidb_enable_auto_analyze', 'tidb_row_format_version', 'tidb_txn_assertion_level', 'tidb_enable_mutation_checker', 'tidb_pessimistic_txn_fair_locking') order by VARIABLE_NAME; +VARIABLE_NAME +tidb_enable_1pc +tidb_enable_async_commit +tidb_enable_auto_analyze +tidb_enable_mutation_checker +tidb_mem_oom_action +tidb_pessimistic_txn_fair_locking +tidb_row_format_version +tidb_txn_assertion_level +set global tidb_enable_async_commit = default; +set global tidb_enable_1pc = default; +set global tidb_mem_oom_action = default; +set global tidb_enable_auto_analyze = default; +set global tidb_row_format_version = default; +set global tidb_txn_assertion_level = default; +set global tidb_enable_mutation_checker = default; +set global tidb_pessimistic_txn_fair_locking = default; +select a.VARIABLE_NAME from information_schema.VARIABLES_INFO as a, mysql.GLOBAL_VARIABLES as b where a.VARIABLE_NAME = b.VARIABLE_NAME and a.DEFAULT_VALUE = b.VARIABLE_VALUE and a.CURRENT_VALUE = b.VARIABLE_VALUE and a.variable_name in ('tidb_enable_async_commit','tidb_enable_1pc', 'tidb_mem_oom_action', 'tidb_enable_auto_analyze', 'tidb_row_format_version', 'tidb_txn_assertion_level', 'tidb_enable_mutation_checker', 'tidb_pessimistic_txn_fair_locking') order by VARIABLE_NAME; +VARIABLE_NAME +tidb_enable_1pc +tidb_enable_async_commit +tidb_enable_auto_analyze +tidb_enable_mutation_checker +tidb_mem_oom_action +tidb_pessimistic_txn_fair_locking +tidb_row_format_version +tidb_txn_assertion_level diff --git a/tests/integrationtest/t/executor/infoschema_reader.test b/tests/integrationtest/t/executor/infoschema_reader.test index b25bb80e93da6..f996f0f990c67 100644 --- a/tests/integrationtest/t/executor/infoschema_reader.test +++ b/tests/integrationtest/t/executor/infoschema_reader.test @@ -239,3 +239,15 @@ SELECT * FROM information_schema.user_privileges WHERE grantee="'usageuser'@'%'" connection default; disconnect conn1; + +# test information_schema.VARIABLES_INFO DEFAULT_VALUE +select VARIABLE_NAME from information_schema.VARIABLES_INFO where DEFAULT_VALUE = CURRENT_VALUE and variable_name in ('tidb_enable_async_commit','tidb_enable_1pc', 'tidb_mem_oom_action', 'tidb_enable_auto_analyze', 'tidb_row_format_version', 'tidb_txn_assertion_level', 'tidb_enable_mutation_checker', 'tidb_pessimistic_txn_fair_locking') order by VARIABLE_NAME; +set global tidb_enable_async_commit = default; +set global tidb_enable_1pc = default; +set global tidb_mem_oom_action = default; +set global tidb_enable_auto_analyze = default; +set global tidb_row_format_version = default; +set global tidb_txn_assertion_level = default; +set global tidb_enable_mutation_checker = default; +set global tidb_pessimistic_txn_fair_locking = default; +select a.VARIABLE_NAME from information_schema.VARIABLES_INFO as a, mysql.GLOBAL_VARIABLES as b where a.VARIABLE_NAME = b.VARIABLE_NAME and a.DEFAULT_VALUE = b.VARIABLE_VALUE and a.CURRENT_VALUE = b.VARIABLE_VALUE and a.variable_name in ('tidb_enable_async_commit','tidb_enable_1pc', 'tidb_mem_oom_action', 'tidb_enable_auto_analyze', 'tidb_row_format_version', 'tidb_txn_assertion_level', 'tidb_enable_mutation_checker', 'tidb_pessimistic_txn_fair_locking') order by VARIABLE_NAME;