statistics: fix wrong NDV in the global stats when to disable async-merge-global-stats #53762
Conversation
ti-chi-bot
bot
added
do-not-merge/needs-linked-issue
do-not-merge/needs-tests-checked
release-note-none
…erge-global-stats Signed-off-by: Weizhen Wang <[email protected]>
ti-chi-bot
added
needs-cherry-pick-release-8.1
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #53762 +/- ##
=================================================
- Coverage 72.4954% 56.2464% -16.2490%
=================================================
Files 1506 1626 +120
Lines 431172 607432 +176260
=================================================
+ Hits 312580 341659 +29079
- Misses 99191 242521 +143330
- Partials 19401 23252 +3851
Flags with carried forward coverage won't be shown. Click here to find out more.
|
…erge-global-stats Signed-off-by: Weizhen Wang <[email protected]>
…erge-global-stats Signed-off-by: Weizhen Wang <[email protected]>
| @@ -959,3 +959,75 @@ func TestGlobalStatsAndSQLBindingWithConcurrency(t *testing.T) { | |||
| tk.MustExec("set global tidb_merge_partition_stats_concurrency=2") | |||
| testGlobalStatsAndSQLBinding(tk) | |||
| } | |||
|
|
|||
| func TestBlockCheckFMSketch(t *testing.T) { | |||
There was a problem hiding this comment.
It seems that even if I revert your change I can still pass the tests. So maybe the test case doesn't cover the change?
There was a problem hiding this comment.
You should disable the tidb_enable_async_merge_global_stats.
There was a problem hiding this comment.
Yes, I have. I've run the TestBlockCheckFMSketch.
- I revert your change.
- I ran the TestBlockCheckFMSketch, and it still passed.
There was a problem hiding this comment.
I find this problem, only it is reproduced in the real tikv.
Co-authored-by: 二手掉包工程师 <[email protected]>
Co-authored-by: 二手掉包工程师 <[email protected]>
Co-authored-by: 二手掉包工程师 <[email protected]>
Signed-off-by: Weizhen Wang <[email protected]>
Signed-off-by: Weizhen Wang <[email protected]>
ti-chi-bot
bot
added
approved
needs-1-more-lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hi-rustin, winoros 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:
Approvers can indicate their approval by writing |
ti-chi-bot
bot
added
lgtm
and removed
needs-1-more-lgtm
|
/retest |
1 similar comment
|
/retest |
|
In response to a cherrypick label: new pull request created to branch |
Signed-off-by: ti-chi-bot <[email protected]>
Signed-off-by: ti-chi-bot <[email protected]>
|
In response to a cherrypick label: new pull request created to branch |
|
In response to a cherrypick label: new pull request created to branch |
|
In response to a cherrypick label: new pull request created to branch |
…erge-global-stats (pingcap#53762) (pingcap#53803) close pingcap#53752
…erge-global-stats (pingcap#53762) (pingcap#53803) close pingcap#53752
What problem does this PR solve?
Issue Number: close #53752
Problem Summary:
What changed and how does it work?
blockMergeGlobalStats doesn't notice that FMSketch contains empty objectives in all partition's stats. so we need to find nil to init global rightly.
Check List
Tests
run as the PR's description.
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.