From 921aa11935b152942e7a5fea6c2ae824844969a4 Mon Sep 17 00:00:00 2001 From: hi-rustin Date: Thu, 28 Mar 2024 15:29:37 +0800 Subject: [PATCH 01/10] sysvar: Do not allow set tidb_auto_analyze_ratio to 0 Signed-off-by: hi-rustin --- pkg/sessionctx/variable/sysvar.go | 21 ++++++++++++- pkg/sessionctx/variable/sysvar_test.go | 41 ++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/pkg/sessionctx/variable/sysvar.go b/pkg/sessionctx/variable/sysvar.go index 190a275b8ff0c..1f3e6b84530e8 100644 --- a/pkg/sessionctx/variable/sysvar.go +++ b/pkg/sessionctx/variable/sysvar.go @@ -723,7 +723,26 @@ var defaultSysVars = []*SysVar{ EnableLocalTxn.Store(newVal) return nil }}, - {Scope: ScopeGlobal, Name: TiDBAutoAnalyzeRatio, Value: strconv.FormatFloat(DefAutoAnalyzeRatio, 'f', -1, 64), Type: TypeFloat, MinValue: 0, MaxValue: math.MaxUint64}, + { + Scope: ScopeGlobal, + Name: TiDBAutoAnalyzeRatio, + Value: strconv.FormatFloat(DefAutoAnalyzeRatio, 'f', -1, 64), + Type: TypeFloat, + MinValue: 0, + MaxValue: math.MaxUint64, + Validation: func(vars *SessionVars, normalizedValue string, originalValue string, scope ScopeFlag) (string, error) { + ratio, err := strconv.ParseFloat(normalizedValue, 64) + if err != nil { + return "", err + } + const minRatio = 0.00001 + const tolerance = 1e-9 + if ratio < minRatio && math.Abs(ratio-minRatio) > tolerance { + return "", ErrWrongValueForVar.GenWithStackByArgs(TiDBAutoAnalyzeRatio, normalizedValue) + } + return normalizedValue, nil + }, + }, {Scope: ScopeGlobal, Name: TiDBAutoAnalyzeStartTime, Value: DefAutoAnalyzeStartTime, Type: TypeTime}, {Scope: ScopeGlobal, Name: TiDBAutoAnalyzeEndTime, Value: DefAutoAnalyzeEndTime, Type: TypeTime}, {Scope: ScopeGlobal, Name: TiDBMemQuotaBindingCache, Value: strconv.FormatInt(DefTiDBMemQuotaBindingCache, 10), Type: TypeUnsigned, MaxValue: math.MaxInt32, GetGlobal: func(_ context.Context, sv *SessionVars) (string, error) { diff --git a/pkg/sessionctx/variable/sysvar_test.go b/pkg/sessionctx/variable/sysvar_test.go index 06dbda61ddc89..e7a964f57da66 100644 --- a/pkg/sessionctx/variable/sysvar_test.go +++ b/pkg/sessionctx/variable/sysvar_test.go @@ -1353,6 +1353,47 @@ func TestTiDBEnableRowLevelChecksum(t *testing.T) { require.Equal(t, Off, val) } +func TestTiDBAutoAnalyzeRatio(t *testing.T) { + ctx := context.Background() + vars := NewSessionVars(nil) + mock := NewMockGlobalAccessor4Tests() + mock.SessionVars = vars + vars.GlobalVarsAccessor = mock + + // default to 0.5 + val, err := mock.GetGlobalSysVar(TiDBAutoAnalyzeRatio) + require.NoError(t, err) + require.Equal(t, "0.5", val) + + // set to 0.1 + err = mock.SetGlobalSysVar(ctx, TiDBAutoAnalyzeRatio, "0.1") + require.NoError(t, err) + val, err = mock.GetGlobalSysVar(TiDBAutoAnalyzeRatio) + require.NoError(t, err) + require.Equal(t, "0.1", val) + + // set to 1.1 + err = mock.SetGlobalSysVar(ctx, TiDBAutoAnalyzeRatio, "1.1") + require.NoError(t, err) + val, err = mock.GetGlobalSysVar(TiDBAutoAnalyzeRatio) + require.NoError(t, err) + require.Equal(t, "1.1", val) + + // set to 0 + err = mock.SetGlobalSysVar(ctx, TiDBAutoAnalyzeRatio, "0") + require.Error(t, err) + val, err = mock.GetGlobalSysVar(TiDBAutoAnalyzeRatio) + require.NoError(t, err) + require.Equal(t, "1.1", val) + + // set to 0.0000000001 + err = mock.SetGlobalSysVar(ctx, TiDBAutoAnalyzeRatio, "0.0000000001") + require.Error(t, err) + val, err = mock.GetGlobalSysVar(TiDBAutoAnalyzeRatio) + require.NoError(t, err) + require.Equal(t, "1.1", val) +} + func TestTiDBTiFlashReplicaRead(t *testing.T) { vars := NewSessionVars(nil) mock := NewMockGlobalAccessor4Tests() From d7e7297f98ba18b14a1bac247ed91f8756d64ccf Mon Sep 17 00:00:00 2001 From: hi-rustin Date: Thu, 28 Mar 2024 15:34:19 +0800 Subject: [PATCH 02/10] chore: better error message Signed-off-by: hi-rustin --- pkg/sessionctx/variable/sysvar.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/sessionctx/variable/sysvar.go b/pkg/sessionctx/variable/sysvar.go index 1f3e6b84530e8..1c0edf577aeaa 100644 --- a/pkg/sessionctx/variable/sysvar.go +++ b/pkg/sessionctx/variable/sysvar.go @@ -730,6 +730,7 @@ var defaultSysVars = []*SysVar{ Type: TypeFloat, MinValue: 0, MaxValue: math.MaxUint64, + // The value of TiDBAutoAnalyzeRatio should be greater than 0.00001. Validation: func(vars *SessionVars, normalizedValue string, originalValue string, scope ScopeFlag) (string, error) { ratio, err := strconv.ParseFloat(normalizedValue, 64) if err != nil { @@ -738,7 +739,7 @@ var defaultSysVars = []*SysVar{ const minRatio = 0.00001 const tolerance = 1e-9 if ratio < minRatio && math.Abs(ratio-minRatio) > tolerance { - return "", ErrWrongValueForVar.GenWithStackByArgs(TiDBAutoAnalyzeRatio, normalizedValue) + return "", errors.Errorf("the value of %s should be greater than or equal to %f", TiDBAutoAnalyzeRatio, minRatio) } return normalizedValue, nil }, From 570b222e27047a9fbc8533e62386549af3270f17 Mon Sep 17 00:00:00 2001 From: hi-rustin Date: Thu, 28 Mar 2024 15:47:57 +0800 Subject: [PATCH 03/10] test: add more tests Signed-off-by: hi-rustin --- pkg/sessionctx/variable/sysvar.go | 2 +- pkg/sessionctx/variable/sysvar_test.go | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/pkg/sessionctx/variable/sysvar.go b/pkg/sessionctx/variable/sysvar.go index 1c0edf577aeaa..6e1918ab1d2c3 100644 --- a/pkg/sessionctx/variable/sysvar.go +++ b/pkg/sessionctx/variable/sysvar.go @@ -730,7 +730,7 @@ var defaultSysVars = []*SysVar{ Type: TypeFloat, MinValue: 0, MaxValue: math.MaxUint64, - // The value of TiDBAutoAnalyzeRatio should be greater than 0.00001. + // The value of TiDBAutoAnalyzeRatio should be greater than 0.00001 or equal to 0.00001. Validation: func(vars *SessionVars, normalizedValue string, originalValue string, scope ScopeFlag) (string, error) { ratio, err := strconv.ParseFloat(normalizedValue, 64) if err != nil { diff --git a/pkg/sessionctx/variable/sysvar_test.go b/pkg/sessionctx/variable/sysvar_test.go index e7a964f57da66..1da7bbbcdee7f 100644 --- a/pkg/sessionctx/variable/sysvar_test.go +++ b/pkg/sessionctx/variable/sysvar_test.go @@ -1392,6 +1392,13 @@ func TestTiDBAutoAnalyzeRatio(t *testing.T) { val, err = mock.GetGlobalSysVar(TiDBAutoAnalyzeRatio) require.NoError(t, err) require.Equal(t, "1.1", val) + + // set to 0.00001 + err = mock.SetGlobalSysVar(ctx, TiDBAutoAnalyzeRatio, "0.00001") + require.NoError(t, err) + val, err = mock.GetGlobalSysVar(TiDBAutoAnalyzeRatio) + require.NoError(t, err) + require.Equal(t, "0.00001", val) } func TestTiDBTiFlashReplicaRead(t *testing.T) { From bdda2290d76dec8fbb0c75a7071fe10b8eaa070d Mon Sep 17 00:00:00 2001 From: hi-rustin Date: Thu, 28 Mar 2024 16:28:39 +0800 Subject: [PATCH 04/10] test: fix broken tests Signed-off-by: hi-rustin --- pkg/sessionctx/variable/sysvar_test.go | 7 +++++++ pkg/statistics/handle/updatetest/update_test.go | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/pkg/sessionctx/variable/sysvar_test.go b/pkg/sessionctx/variable/sysvar_test.go index 1da7bbbcdee7f..e71dc91dd0bf8 100644 --- a/pkg/sessionctx/variable/sysvar_test.go +++ b/pkg/sessionctx/variable/sysvar_test.go @@ -1399,6 +1399,13 @@ func TestTiDBAutoAnalyzeRatio(t *testing.T) { val, err = mock.GetGlobalSysVar(TiDBAutoAnalyzeRatio) require.NoError(t, err) require.Equal(t, "0.00001", val) + + // set to 0.000009999 + err = mock.SetGlobalSysVar(ctx, TiDBAutoAnalyzeRatio, "0.000009999") + require.Error(t, err) + val, err = mock.GetGlobalSysVar(TiDBAutoAnalyzeRatio) + require.NoError(t, err) + require.Equal(t, "0.00001", val) } func TestTiDBTiFlashReplicaRead(t *testing.T) { diff --git a/pkg/statistics/handle/updatetest/update_test.go b/pkg/statistics/handle/updatetest/update_test.go index ed49d794a01b9..9db043d212343 100644 --- a/pkg/statistics/handle/updatetest/update_test.go +++ b/pkg/statistics/handle/updatetest/update_test.go @@ -806,7 +806,7 @@ func TestAutoUpdatePartitionInDynamicOnlyMode(t *testing.T) { testKit.MustExec("set global tidb_auto_analyze_ratio = 0.1") defer func() { exec.AutoAnalyzeMinCnt = 1000 - testKit.MustExec("set global tidb_auto_analyze_ratio = 0.0") + testKit.MustExec("set global tidb_auto_analyze_ratio = 0.5") }() require.NoError(t, h.Update(is)) From 9fb8d65284db6a343be62e1fac37d8b18646172f Mon Sep 17 00:00:00 2001 From: hi-rustin Date: Thu, 28 Mar 2024 16:31:24 +0800 Subject: [PATCH 05/10] test: fix broken tests Signed-off-by: hi-rustin --- pkg/executor/test/analyzetest/analyze_test.go | 6 +++--- .../test/analyzetest/memorycontrol/memory_control_test.go | 2 +- pkg/statistics/handle/updatetest/update_test.go | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/executor/test/analyzetest/analyze_test.go b/pkg/executor/test/analyzetest/analyze_test.go index 90fbcff71bc9f..312d5612ac68b 100644 --- a/pkg/executor/test/analyzetest/analyze_test.go +++ b/pkg/executor/test/analyzetest/analyze_test.go @@ -688,7 +688,7 @@ func TestSavedAnalyzeOptions(t *testing.T) { defer func() { tk.MustExec(fmt.Sprintf("set global tidb_auto_analyze_ratio = %v", originalVal2)) }() - tk.MustExec("set global tidb_auto_analyze_ratio = 0.01") + tk.MustExec("set global tidb_auto_analyze_ratio = 0.51") originalVal3 := exec.AutoAnalyzeMinCnt defer func() { exec.AutoAnalyzeMinCnt = originalVal3 @@ -1030,7 +1030,7 @@ func TestSavedAnalyzeColumnOptions(t *testing.T) { defer func() { tk.MustExec(fmt.Sprintf("set global tidb_auto_analyze_ratio = %v", originalVal2)) }() - tk.MustExec("set global tidb_auto_analyze_ratio = 0.01") + tk.MustExec("set global tidb_auto_analyze_ratio = 0.51") originalVal3 := exec.AutoAnalyzeMinCnt defer func() { exec.AutoAnalyzeMinCnt = originalVal3 @@ -2732,7 +2732,7 @@ func TestAutoAnalyzeAwareGlobalVariableChange(t *testing.T) { originalVal1 := exec.AutoAnalyzeMinCnt originalVal2 := tk.MustQuery("select @@global.tidb_auto_analyze_ratio").Rows()[0][0].(string) exec.AutoAnalyzeMinCnt = 0 - tk.MustExec("set global tidb_auto_analyze_ratio = 0.001") + tk.MustExec("set global tidb_auto_analyze_ratio = 0.501") defer func() { exec.AutoAnalyzeMinCnt = originalVal1 tk.MustExec(fmt.Sprintf("set global tidb_auto_analyze_ratio = %v", originalVal2)) diff --git a/pkg/executor/test/analyzetest/memorycontrol/memory_control_test.go b/pkg/executor/test/analyzetest/memorycontrol/memory_control_test.go index b50ecced58529..1975b549aca14 100644 --- a/pkg/executor/test/analyzetest/memorycontrol/memory_control_test.go +++ b/pkg/executor/test/analyzetest/memorycontrol/memory_control_test.go @@ -146,7 +146,7 @@ func TestGlobalMemoryControlForAutoAnalyze(t *testing.T) { originalVal4 := exec.AutoAnalyzeMinCnt originalVal5 := tk.MustQuery("select @@global.tidb_auto_analyze_ratio").Rows()[0][0].(string) exec.AutoAnalyzeMinCnt = 0 - tk.MustExec("set global tidb_auto_analyze_ratio = 0.001") + tk.MustExec("set global tidb_auto_analyze_ratio = 0.501") defer func() { exec.AutoAnalyzeMinCnt = originalVal4 tk.MustExec(fmt.Sprintf("set global tidb_auto_analyze_ratio = %v", originalVal5)) diff --git a/pkg/statistics/handle/updatetest/update_test.go b/pkg/statistics/handle/updatetest/update_test.go index 9db043d212343..5d01f0aa4a1c2 100644 --- a/pkg/statistics/handle/updatetest/update_test.go +++ b/pkg/statistics/handle/updatetest/update_test.go @@ -368,7 +368,7 @@ func TestAutoUpdate(t *testing.T) { testKit.MustExec("set global tidb_auto_analyze_ratio = 0.2") defer func() { exec.AutoAnalyzeMinCnt = 1000 - testKit.MustExec("set global tidb_auto_analyze_ratio = 0.0") + testKit.MustExec("set global tidb_auto_analyze_ratio = 0.5") }() do := dom @@ -473,7 +473,7 @@ func TestAutoUpdatePartition(t *testing.T) { testKit.MustExec("set global tidb_auto_analyze_ratio = 0.6") defer func() { exec.AutoAnalyzeMinCnt = 1000 - testKit.MustExec("set global tidb_auto_analyze_ratio = 0.0") + testKit.MustExec("set global tidb_auto_analyze_ratio = 0.5") }() do := dom From ddf1ce1d280329d34f02d5e4553e4a04abf1aa83 Mon Sep 17 00:00:00 2001 From: hi-rustin Date: Thu, 28 Mar 2024 16:35:23 +0800 Subject: [PATCH 06/10] fix: use 0.5 Signed-off-by: hi-rustin --- pkg/executor/test/analyzetest/analyze_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/executor/test/analyzetest/analyze_test.go b/pkg/executor/test/analyzetest/analyze_test.go index 312d5612ac68b..98aa32c95da2b 100644 --- a/pkg/executor/test/analyzetest/analyze_test.go +++ b/pkg/executor/test/analyzetest/analyze_test.go @@ -688,7 +688,7 @@ func TestSavedAnalyzeOptions(t *testing.T) { defer func() { tk.MustExec(fmt.Sprintf("set global tidb_auto_analyze_ratio = %v", originalVal2)) }() - tk.MustExec("set global tidb_auto_analyze_ratio = 0.51") + tk.MustExec("set global tidb_auto_analyze_ratio = 0.5") originalVal3 := exec.AutoAnalyzeMinCnt defer func() { exec.AutoAnalyzeMinCnt = originalVal3 @@ -1030,7 +1030,7 @@ func TestSavedAnalyzeColumnOptions(t *testing.T) { defer func() { tk.MustExec(fmt.Sprintf("set global tidb_auto_analyze_ratio = %v", originalVal2)) }() - tk.MustExec("set global tidb_auto_analyze_ratio = 0.51") + tk.MustExec("set global tidb_auto_analyze_ratio = 0.5") originalVal3 := exec.AutoAnalyzeMinCnt defer func() { exec.AutoAnalyzeMinCnt = originalVal3 From 4aa696fff7f82f39a071f0e9518f72e585c87558 Mon Sep 17 00:00:00 2001 From: hi-rustin Date: Thu, 28 Mar 2024 16:36:30 +0800 Subject: [PATCH 07/10] fix Signed-off-by: hi-rustin --- pkg/executor/test/analyzetest/analyze_test.go | 2 +- .../test/analyzetest/memorycontrol/memory_control_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/executor/test/analyzetest/analyze_test.go b/pkg/executor/test/analyzetest/analyze_test.go index 98aa32c95da2b..32c02e27b5701 100644 --- a/pkg/executor/test/analyzetest/analyze_test.go +++ b/pkg/executor/test/analyzetest/analyze_test.go @@ -2732,7 +2732,7 @@ func TestAutoAnalyzeAwareGlobalVariableChange(t *testing.T) { originalVal1 := exec.AutoAnalyzeMinCnt originalVal2 := tk.MustQuery("select @@global.tidb_auto_analyze_ratio").Rows()[0][0].(string) exec.AutoAnalyzeMinCnt = 0 - tk.MustExec("set global tidb_auto_analyze_ratio = 0.501") + tk.MustExec("set global tidb_auto_analyze_ratio = 0.001") defer func() { exec.AutoAnalyzeMinCnt = originalVal1 tk.MustExec(fmt.Sprintf("set global tidb_auto_analyze_ratio = %v", originalVal2)) diff --git a/pkg/executor/test/analyzetest/memorycontrol/memory_control_test.go b/pkg/executor/test/analyzetest/memorycontrol/memory_control_test.go index 1975b549aca14..b50ecced58529 100644 --- a/pkg/executor/test/analyzetest/memorycontrol/memory_control_test.go +++ b/pkg/executor/test/analyzetest/memorycontrol/memory_control_test.go @@ -146,7 +146,7 @@ func TestGlobalMemoryControlForAutoAnalyze(t *testing.T) { originalVal4 := exec.AutoAnalyzeMinCnt originalVal5 := tk.MustQuery("select @@global.tidb_auto_analyze_ratio").Rows()[0][0].(string) exec.AutoAnalyzeMinCnt = 0 - tk.MustExec("set global tidb_auto_analyze_ratio = 0.501") + tk.MustExec("set global tidb_auto_analyze_ratio = 0.001") defer func() { exec.AutoAnalyzeMinCnt = originalVal4 tk.MustExec(fmt.Sprintf("set global tidb_auto_analyze_ratio = %v", originalVal5)) From e5a7a6c827dc9d3ba20de4c2ea2f6564e13aab9b Mon Sep 17 00:00:00 2001 From: hi-rustin Date: Thu, 28 Mar 2024 16:37:20 +0800 Subject: [PATCH 08/10] fix Signed-off-by: hi-rustin --- pkg/executor/test/analyzetest/analyze_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/executor/test/analyzetest/analyze_test.go b/pkg/executor/test/analyzetest/analyze_test.go index 32c02e27b5701..90fbcff71bc9f 100644 --- a/pkg/executor/test/analyzetest/analyze_test.go +++ b/pkg/executor/test/analyzetest/analyze_test.go @@ -688,7 +688,7 @@ func TestSavedAnalyzeOptions(t *testing.T) { defer func() { tk.MustExec(fmt.Sprintf("set global tidb_auto_analyze_ratio = %v", originalVal2)) }() - tk.MustExec("set global tidb_auto_analyze_ratio = 0.5") + tk.MustExec("set global tidb_auto_analyze_ratio = 0.01") originalVal3 := exec.AutoAnalyzeMinCnt defer func() { exec.AutoAnalyzeMinCnt = originalVal3 @@ -1030,7 +1030,7 @@ func TestSavedAnalyzeColumnOptions(t *testing.T) { defer func() { tk.MustExec(fmt.Sprintf("set global tidb_auto_analyze_ratio = %v", originalVal2)) }() - tk.MustExec("set global tidb_auto_analyze_ratio = 0.5") + tk.MustExec("set global tidb_auto_analyze_ratio = 0.01") originalVal3 := exec.AutoAnalyzeMinCnt defer func() { exec.AutoAnalyzeMinCnt = originalVal3 From b2bd0b4fbfda97e7f0a698baffc9c1269a84fb6c Mon Sep 17 00:00:00 2001 From: hi-rustin Date: Thu, 28 Mar 2024 17:00:51 +0800 Subject: [PATCH 09/10] test: fix broken test Signed-off-by: hi-rustin --- pkg/statistics/handle/autoanalyze/autoanalyze_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/statistics/handle/autoanalyze/autoanalyze_test.go b/pkg/statistics/handle/autoanalyze/autoanalyze_test.go index 3d95d0638cfff..2acfe70d5de11 100644 --- a/pkg/statistics/handle/autoanalyze/autoanalyze_test.go +++ b/pkg/statistics/handle/autoanalyze/autoanalyze_test.go @@ -103,8 +103,7 @@ func TestDisableAutoAnalyze(t *testing.T) { is := dom.InfoSchema() require.NoError(t, h.Update(is)) - // Set auto analyze ratio to 0. - tk.MustExec("set @@global.tidb_auto_analyze_ratio = 0") + tk.MustExec("set @@global.tidb_enable_auto_analyze = 0") exec.AutoAnalyzeMinCnt = 0 defer func() { exec.AutoAnalyzeMinCnt = 1000 From 3f4828370bb91cd965075a59592d4da737e83c6e Mon Sep 17 00:00:00 2001 From: hi-rustin Date: Thu, 28 Mar 2024 17:17:40 +0800 Subject: [PATCH 10/10] test: fix broken tests Signed-off-by: hi-rustin --- .../handle/autoanalyze/refresher/refresher_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pkg/statistics/handle/autoanalyze/refresher/refresher_test.go b/pkg/statistics/handle/autoanalyze/refresher/refresher_test.go index d5cdbaf8968b9..02ad638ec0646 100644 --- a/pkg/statistics/handle/autoanalyze/refresher/refresher_test.go +++ b/pkg/statistics/handle/autoanalyze/refresher/refresher_test.go @@ -54,8 +54,10 @@ func TestSkipAnalyzeTableWhenAutoAnalyzeRatioIsZero(t *testing.T) { ) tk.MustExec("insert into t1 values (1, 1), (2, 2), (3, 3)") tk.MustExec("insert into t2 values (1, 1), (2, 2), (3, 3)") - // Set the auto analyze ratio to 0. - tk.MustExec("set global tidb_auto_analyze_ratio = 0") + // HACK: Set the auto analyze ratio to 0. + // We don't allow users to set the ratio to 0 anymore, but we still need to test this case. + // Because we need to compilable with the old configuration. + tk.MustExec("update mysql.global_variables set variable_value = '0' where variable_name = 'tidb_auto_analyze_ratio'") handle := dom.StatsHandle() require.NoError(t, handle.DumpStatsDeltaToKV(true)) require.NoError(t, handle.Update(dom.InfoSchema()))