sysvar: Do not allow set tidb_auto_analyze_ratio to 0 #52190

Rustin170506 · 2024-03-28T07:31:24Z

What problem does this PR solve?

Issue Number: close #51582

Problem Summary:

Currently, if you set tidb_auto_analyze_ratio to 0, we will disable auto-analysis based on row changes, but we will still process the newly added indexes and tables.

This behavior is difficult to understand. I suppose most users will think 0 means we always need to analyze the table, no matter how many rows have been updated.

The historic reason we introduced it is that before #34643 this PR, there was no way to disable the auto-analyze feature without restarting the server. So we chose this special value to allow users to disable the auto-analysis feature. But now enable_tidb_auto_analyze is a global session variable. So we don't depend on this value.

What changed and how does it work?

Added verification for tidb_auto_analyze_ratio to make sure it cannot be zero and the minimum value is 0.00001.

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No need to test
- I checked and no code files have been changed.

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

不允许将 tidb_auto_analyze_ratio 参数设置为0
Do not allow set tidb_auto_analyze_ratio to 0

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

Rustin170506

🔢 Self-check (PR reviewed by myself and ready for feedback.)

hawkingrei · 2024-03-28T07:56:11Z

/retest

codecov · 2024-03-28T08:20:45Z

Codecov Report

Merging #52190 (570b222) into master (ce2cf92) will decrease coverage by 0.2248%.
Report is 1 commits behind head on master.
The diff coverage is 0.0000%.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #52190        +/-   ##
================================================
- Coverage   69.2555%   69.0307%   -0.2248%     
================================================
  Files          1490       1490                
  Lines        420134     436249     +16115     
================================================
+ Hits         290966     301146     +10180     
- Misses       109658     114905      +5247     
- Partials      19510      20198       +688

Flag	Coverage Δ
integration	`23.2799% <0.0000%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
dumpling	`56.2971% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`47.8358% <ø> (+1.4589%)`	⬆️

codecov · 2024-03-28T08:24:59Z

Codecov Report

Merging #52190 (3f48283) into master (ce2cf92) will increase coverage by 3.5124%.
Report is 13 commits behind head on master.
The diff coverage is 72.7272%.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #52190        +/-   ##
================================================
+ Coverage   69.2555%   72.7679%   +3.5124%     
================================================
  Files          1490       1490                
  Lines        420134     440180     +20046     
================================================
+ Hits         290966     320310     +29344     
+ Misses       109658      99685      -9973     
- Partials      19510      20185       +675

Flag	Coverage Δ
integration	`48.7654% <0.0000%> (?)`
unit	`70.4413% <72.7272%> (+1.4101%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
dumpling	`53.9957% <ø> (-2.3014%)`	⬇️
parser	`∅ <ø> (∅)`
br	`51.5489% <ø> (+5.1720%)`	⬆️

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

Rustin170506

🔢 Self-check (PR reviewed by myself and ready for feedback.)

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

Rustin170506 · 2024-03-28T09:14:16Z

Test locally:

start the v7.5.0 tidb cluser

tiup cluster deploy upstream v7.5.0 upstream.yaml -p
tiup cluster start upstream

Check the version and set the ratio to 0

MySQL [(none)]> select tidb_version();
+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| tidb_version()                                                                                                                                                                                                                                                 |
+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| Release Version: v7.5.0
Edition: Community
Git Commit Hash: 069631e2ecfedc000ffb92c67207bea81380f020
Git Branch: heads/refs/tags/v7.5.0
UTC Build Time: 2023-11-24 08:50:14
GoVersion: go1.21.3
Race Enabled: false
Check Table Before Drop: false
Store: tikv |
+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
1 row in set (0.00 sec)

MySQL [(none)]> set global tidb_auto_analyze_ratio=0;
Query OK, 0 rows affected (0.02 sec)

MySQL [(none)]> select @@tidb_auto_analyze_ratio;
+---------------------------+
| @@tidb_auto_analyze_ratio |
+---------------------------+
| 0                         |
+---------------------------+
1 row in set (0.00 sec)

MySQL [(none)]> exit

Upgrade the tidb: tiup cluster patch upstream ../../../tidb/bin/tidb-hotfix-linux-amd64.tar.gz -N 10.2.6.x:4000,10.2.6.x:4000,10.2.7.x:4000 -R tidb
Check the version and value:

MySQL [(none)]> select tidb_version();
+------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| tidb_version()                                                                                                                                                                                                                                                               |
+------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| Release Version: v8.0.0-alpha-664-gb2bd0b4fbf
Edition: Community
Git Commit Hash: b2bd0b4fbfda97e7f0a698baffc9c1269a84fb6c
Git Branch: rustin-patch-0
UTC Build Time: 2024-03-28 09:04:11
GoVersion: go1.21.1
Race Enabled: false
Check Table Before Drop: false
Store: tikv |
+------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
1 row in set (0.00 sec)

MySQL [(none)]> select @@tidb_auto_analyze_ratio;
+---------------------------+
| @@tidb_auto_analyze_ratio |
+---------------------------+
| 0                         |
+---------------------------+
1 row in set (0.00 sec)

MySQL [(none)]> set global tidb_auto_analyze_ratio=0;
ERROR 1105 (HY000): the value of tidb_auto_analyze_ratio should be greater than or equal to 0.000010
MySQL [(none)]>

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

Rustin170506 · 2024-03-28T09:36:06Z

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.


It didn't change the old behavior.

Rustin170506 · 2024-03-28T09:38:44Z

/unhold

ti-chi-bot · 2024-03-28T13:18:17Z

[LGTM Timeline notifier]

Timeline:

2024-03-28 12:41:37.011076108 +0000 UTC m=+2158124.033322494: ☑️ agreed by time-and-fate.
2024-03-28 13:18:16.93707513 +0000 UTC m=+2160323.959321515: ☑️ agreed by qw4990.

ti-chi-bot · 2024-03-29T06:05:02Z

@songrijie: adding LGTM is restricted to approvers and reviewers in OWNERS files.

In response to this:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

easonn7 · 2024-03-29T06:12:40Z

/approve

ti-chi-bot · 2024-03-29T06:12:47Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: easonn7, qw4990, songrijie, time-and-fate

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [qw4990,time-and-fate]
~~pkg/sessionctx/variable/OWNERS~~ [easonn7]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

sysvar: Do not allow set tidb_auto_analyze_ratio to 0

921aa11

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

ti-chi-bot bot added release-note-none Denotes a PR that doesn't merit a release note. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 28, 2024

chore: better error message

d7e7297

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

Rustin170506 requested review from qw4990, songrijie and time-and-fate March 28, 2024 07:39

test: add more tests

570b222

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

Rustin170506 commented Mar 28, 2024

View reviewed changes

ti-chi-bot bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Mar 28, 2024

This comment was marked as outdated.

Sign in to view

ti-chi-bot bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 28, 2024

Rustin170506 added 2 commits March 28, 2024 16:28

test: fix broken tests

bdda229

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

test: fix broken tests

9fb8d65

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

ti-chi-bot bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 28, 2024

Rustin170506 added 2 commits March 28, 2024 16:35

fix: use 0.5

ddf1ce1

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

fix

4aa696f

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

ti-chi-bot bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 28, 2024

fix

e5a7a6c

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

Rustin170506 commented Mar 28, 2024

View reviewed changes

test: fix broken test

b2bd0b4

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

test: fix broken tests

3f48283

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

ti-chi-bot bot removed the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Mar 28, 2024

ti-chi-bot bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 28, 2024

Rustin170506 commented Mar 28, 2024

View reviewed changes

ti-chi-bot bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 28, 2024

Rustin170506 added sig/planner SIG: Planner do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Mar 28, 2024

time-and-fate approved these changes Mar 28, 2024

View reviewed changes

ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Mar 28, 2024

qw4990 approved these changes Mar 28, 2024

View reviewed changes

ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Mar 28, 2024

songrijie approved these changes Mar 29, 2024

View reviewed changes

ti-chi-bot bot added the approved label Mar 29, 2024

ti-chi-bot bot merged commit 528bd7e into pingcap:master Mar 29, 2024
23 checks passed

Rustin170506 mentioned this pull request Apr 12, 2024

tidb: update tidb_auto_analyze_ratio range pingcap/docs-cn#17108

Merged

18 tasks

Oreoxmt mentioned this pull request Apr 16, 2024

tidb: update tidb_auto_analyze_ratio range pingcap/docs#17219

Merged

18 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sysvar: Do not allow set tidb_auto_analyze_ratio to 0 #52190

sysvar: Do not allow set tidb_auto_analyze_ratio to 0 #52190

Rustin170506 commented Mar 28, 2024 •

edited

Loading

Rustin170506 left a comment

hawkingrei commented Mar 28, 2024

This comment was marked as outdated.

codecov bot commented Mar 28, 2024

codecov bot commented Mar 28, 2024 •

edited

Loading

Rustin170506 left a comment

Rustin170506 commented Mar 28, 2024

Rustin170506 Mar 28, 2024

Rustin170506 commented Mar 28, 2024

ti-chi-bot bot commented Mar 28, 2024

ti-chi-bot bot commented Mar 29, 2024

easonn7 commented Mar 29, 2024

ti-chi-bot bot commented Mar 29, 2024

sysvar: Do not allow set tidb_auto_analyze_ratio to 0 #52190

sysvar: Do not allow set tidb_auto_analyze_ratio to 0 #52190

Conversation

Rustin170506 commented Mar 28, 2024 • edited Loading

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

Rustin170506 left a comment

Choose a reason for hiding this comment

hawkingrei commented Mar 28, 2024

This comment was marked as outdated.

codecov bot commented Mar 28, 2024

Codecov Report

codecov bot commented Mar 28, 2024 • edited Loading

Codecov Report

Rustin170506 left a comment

Choose a reason for hiding this comment

Rustin170506 commented Mar 28, 2024

Rustin170506 Mar 28, 2024

Choose a reason for hiding this comment

Rustin170506 commented Mar 28, 2024

ti-chi-bot bot commented Mar 28, 2024

[LGTM Timeline notifier]

ti-chi-bot bot commented Mar 29, 2024

easonn7 commented Mar 29, 2024

ti-chi-bot bot commented Mar 29, 2024

Rustin170506 commented Mar 28, 2024 •

edited

Loading

codecov bot commented Mar 28, 2024 •

edited

Loading