statistics: correct behavior of non-lite InitStats and stats sync load of no stats column #57803

winoros · 2024-11-28T12:30:05Z

What problem does this PR solve?

Issue Number: close #57804

Problem Summary:

What changed and how does it work?

You can regard this pull request as a totally rewrite of #53399 and #53298

This pull reduced the possible stats state after the stats initialization finished. Now we'll get the same memory objects after a non-concurrent or current non-lite load.

Then use the unified status to make the ColumnIsLoadNeeded correct when one column even has no record in mysql.stats_histograms.

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.

None

codecov · 2024-11-28T12:50:44Z

Codecov Report

Attention: Patch coverage is 86.66667% with 8 lines in your changes missing coverage. Please review.

Project coverage is 73.9413%. Comparing base (e0019df) to head (2ecd406).
Report is 3 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #57803        +/-   ##
================================================
+ Coverage   73.1616%   73.9413%   +0.7796%     
================================================
  Files          1671       1701        +30     
  Lines        460714     469452      +8738     
================================================
+ Hits         337066     347119     +10053     
+ Misses       102898     100952      -1946     
- Partials      20750      21381       +631

Flag	Coverage Δ
integration	`46.1774% <26.6666%> (?)`
unit	`72.4219% <86.6666%> (+0.1254%)`	⬆️

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

Components	Coverage Δ
dumpling	`52.6910% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`46.0314% <ø> (+0.0330%)`	⬆️

Rustin170506

Could you please summarize the changes made in your PR description? It's a bit difficult to understand what is happening in the PR.

pkg/statistics/handle/bootstrap.go

pkg/statistics/table.go

Co-authored-by: Rustin <tech@rustin.me>

Rustin170506

rest LGTM
Thank you!

pkg/statistics/handle/bootstrap.go

Rustin170506 · 2024-12-03T05:49:54Z

pkg/statistics/handle/syncload/stats_syncload_test.go

+	require.True(t, statsTbl.GetCol(tblInfo.Columns[3].ID).IsFullLoad())
+	require.Nil(t, statsTbl.GetCol(tblInfo.Columns[2].ID))
+	_, loadNeeded, analyzed := statsTbl.ColumnIsLoadNeeded(tblInfo.Columns[2].ID, false)
+	// After the sync load. The column without any thing in storage should not be marked as loadNeeded any more.


If we analyze column c next time, I assume we can load it correctly? Could you please also add a test step to include it?

Yes, if we analyze it. After the sync load. It should return loadNeeded=false, analyzed=true

Rustin170506

Thanks!

ti-chi-bot · 2024-12-03T08:45:01Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hawkingrei, Rustin170506

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:

~~pkg/statistics/OWNERS~~ [Rustin170506,hawkingrei]

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

ti-chi-bot · 2024-12-03T08:45:04Z

[LGTM Timeline notifier]

Timeline:

2024-11-29 01:19:10.185011604 +0000 UTC m=+772137.804666119: ☑️ agreed by hawkingrei.
2024-12-03 08:45:03.359478405 +0000 UTC m=+1144490.979132906: ☑️ agreed by Rustin170506.

winoros · 2024-12-03T09:46:20Z

/retest

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot · 2024-12-03T12:01:19Z

In response to a cherrypick label: new pull request created to branch release-8.5: #57943.

…d of no stats column (#57803) (#57943) close #57804

ti-chi-bot bot added do-not-merge/needs-linked-issue do-not-merge/needs-tests-checked release-note-none Denotes a PR that doesn't merit a release note. sig/planner SIG: Planner size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 28, 2024

winoros force-pushed the loadneeded-fix branch from 296b172 to d201740 Compare November 28, 2024 12:34

ti-chi-bot bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 28, 2024

statistics: try to eliminate the unnecessary log for sync load

16936a5

winoros force-pushed the loadneeded-fix branch from d201740 to 16936a5 Compare November 28, 2024 14:40

ti-chi-bot bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 28, 2024

winoros changed the title ~~statistics: try to eliminate the unnecessary log for sync load~~ statistics: try to eliminate the unnecessary log for sync load | tidb-test=pr/2435 Nov 28, 2024

ti-chi-bot bot removed do-not-merge/needs-linked-issue do-not-merge/needs-tests-checked labels Nov 28, 2024

winoros force-pushed the loadneeded-fix branch from 07b9bcf to 4434c2a Compare November 28, 2024 16:09

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 Nov 28, 2024

fix tests

0494095

winoros force-pushed the loadneeded-fix branch from 4434c2a to 0494095 Compare November 28, 2024 16:24

winoros changed the title ~~statistics: try to eliminate the unnecessary log for sync load | tidb-test=pr/2435~~ statistics: try to eliminate the unnecessary log for sync load Nov 28, 2024

winoros added 2 commits November 29, 2024 03:36

make it more compatible than before

c1aa991

undo unnecessary test changes

f9f7ac1

winoros force-pushed the loadneeded-fix branch from fca25f1 to f9f7ac1 Compare November 28, 2024 19:36

winoros changed the title ~~statistics: try to eliminate the unnecessary log for sync load~~ statistics: correct some behavior of non-lite InitStats and stats sync load Nov 28, 2024

winoros changed the title ~~statistics: correct some behavior of non-lite InitStats and stats sync load~~ statistics: correct behavior of non-lite InitStats and stats sync load of no stats column Nov 28, 2024

winoros added 3 commits November 29, 2024 03:56

remove unused arg

e7a78b7

reduce changed lines

07cf605

add comments

b09bbbc

ti-chi-bot bot added approved needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Nov 29, 2024

add one test

ccceaa1

Rustin170506 reviewed Dec 2, 2024

View reviewed changes

pkg/statistics/handle/bootstrap.go Show resolved Hide resolved

pkg/statistics/handle/bootstrap.go Show resolved Hide resolved

pkg/statistics/handle/bootstrap.go Show resolved Hide resolved

Rustin170506 reviewed Dec 2, 2024

View reviewed changes

pkg/statistics/table.go Outdated Show resolved Hide resolved

winoros and others added 4 commits December 2, 2024 19:19

Update pkg/statistics/table.go

efc7643

Co-authored-by: Rustin <tech@rustin.me>

add some comments

9d940ad

Merge remote-tracking branch 'yiding/loadneeded-fix' into loadneeded-fix

1fdc4f3

fix bazel_prepare and fix typo

6d003ca

ti-chi-bot bot added the needs-cherry-pick-release-8.5 Should cherry pick this PR to release-8.5 branch. label Dec 3, 2024

Rustin170506 reviewed Dec 3, 2024

View reviewed changes

winoros added 3 commits December 3, 2024 16:24

address comments

4e05b9f

Merge remote-tracking branch 'origin/master' into loadneeded-fix

6183473

address comment

2ecd406

winoros force-pushed the loadneeded-fix branch from 86b446d to 2ecd406 Compare December 3, 2024 08:44

Rustin170506 approved these changes Dec 3, 2024

View reviewed changes

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

ti-chi-bot bot merged commit d021648 into pingcap:master Dec 3, 2024
25 checks passed

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Dec 3, 2024

This is an automated cherry-pick of pingcap#57803

18f9c6b

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot mentioned this pull request Dec 3, 2024

statistics: correct behavior of non-lite InitStats and stats sync load of no stats column (#57803) #57943

Merged

13 tasks

winoros deleted the loadneeded-fix branch December 3, 2024 12:02

ti-chi-bot bot pushed a commit that referenced this pull request Dec 3, 2024

statistics: correct behavior of non-lite InitStats and stats sync loa…

0eb3c0b

…d of no stats column (#57803) (#57943) close #57804

qw4990 mentioned this pull request Dec 6, 2024

TiKV crash loop: "Unknown analyze version" after upgrade from 7.5.1 to nightly tikv/tikv#17944

Closed

Rustin170506 mentioned this pull request Dec 19, 2024

statistics: remove dead code #58412

Merged

13 tasks

ti-chi-bot mentioned this pull request Dec 20, 2024

statistics: remove dead code (#58412) #58442

Merged

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

statistics: correct behavior of non-lite InitStats and stats sync load of no stats column #57803

statistics: correct behavior of non-lite InitStats and stats sync load of no stats column #57803

winoros commented Nov 28, 2024 •

edited

Loading

codecov bot commented Nov 28, 2024 •

edited

Loading

Rustin170506 left a comment

Rustin170506 left a comment

Rustin170506 Dec 3, 2024

winoros Dec 3, 2024

Rustin170506 left a comment

ti-chi-bot bot commented Dec 3, 2024

ti-chi-bot bot commented Dec 3, 2024

winoros commented Dec 3, 2024

ti-chi-bot commented Dec 3, 2024

statistics: correct behavior of non-lite InitStats and stats sync load of no stats column #57803

statistics: correct behavior of non-lite InitStats and stats sync load of no stats column #57803

Conversation

winoros commented Nov 28, 2024 • edited Loading

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

codecov bot commented Nov 28, 2024 • edited Loading

Codecov Report

Rustin170506 left a comment

Choose a reason for hiding this comment

Rustin170506 left a comment

Choose a reason for hiding this comment

Rustin170506 Dec 3, 2024

Choose a reason for hiding this comment

winoros Dec 3, 2024

Choose a reason for hiding this comment

Rustin170506 left a comment

Choose a reason for hiding this comment

ti-chi-bot bot commented Dec 3, 2024

ti-chi-bot bot commented Dec 3, 2024

[LGTM Timeline notifier]

winoros commented Dec 3, 2024

ti-chi-bot commented Dec 3, 2024

winoros commented Nov 28, 2024 •

edited

Loading

codecov bot commented Nov 28, 2024 •

edited

Loading