Skip to content

statistics: drop old stats meta after truncate a table - #49664

Merged
ti-chi-bot[bot] merged 5 commits into
pingcap:masterfrom
0xPoe:rustin-patch-truncate-table
Dec 22, 2023
Merged

statistics: drop old stats meta after truncate a table #49664
ti-chi-bot[bot] merged 5 commits into
pingcap:masterfrom
0xPoe:rustin-patch-truncate-table

Conversation

@0xPoe

@0xPoe 0xPoe commented Dec 21, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

What problem does this PR solve?

Issue Number: close #49663

Problem Summary:

What changed and how does it work?

  • Added a test case to cover this problem.
  • Drop the old stats meta after truncating a table.

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

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Changes MySQL compatibility

Release note

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

Fix TiDB can not drop old stats meta after truncating a table issue

0xPoe added 2 commits December 21, 2023 15:09
@0xPoe
test: add test case for truncate a partition
3dc2f83 
Signed-off-by: hi-rustin <rustin.liu@gmail.com>
@0xPoe
fix: drop old stats meta after truncate a table
085b37b 
Signed-off-by: hi-rustin <rustin.liu@gmail.com>
@ti-chi-bot ti-chi-bot Bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. do-not-merge/needs-triage-completed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed do-not-merge/needs-triage-completed labels Dec 21, 2023
0xPoe
0xPoe commented Dec 21, 2023
View reviewed changes

@0xPoe 0xPoe left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔢 Self-check

@0xPoe
0xPoe requested review from hawkingrei and qw4990 December 21, 2023 07:34
@codecov

codecov Bot commented Dec 21, 2023 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #49664 (d77d2a7) into master (da460f1) will increase coverage by 0.3793%.
Report is 26 commits behind head on master.
The diff coverage is 0.0000%.

Additional details and impacted files 
@@               Coverage Diff                @@
##             master     #49664        +/-   ##
================================================
+ Coverage   70.9801%   71.3594%   +0.3793%     
================================================
  Files          1368       1424        +56     
  Lines        398761     420826     +22065     
================================================
+ Hits         283041     300299     +17258     
- Misses        95945     101583      +5638     
+ Partials      19775      18944       -831
Flag Coverage Δ
integration 43.6693% <0.0000%> (?)
unit 70.9796% <ø> (-0.0006%) ⬇️

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

Components Coverage Δ
dumpling 53.9663% <ø> (ø)
parser ∅ <ø> (∅)
br 46.7639% <ø> (-6.1187%) ⬇️

@ti-chi-bot ti-chi-bot Bot added approved needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Dec 21, 2023
@0xPoe

0xPoe commented Dec 21, 2023

Copy link
Copy Markdown
Member Author

/hold

@ti-chi-bot 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 Dec 21, 2023
0xPoe
0xPoe commented Dec 21, 2023
View reviewed changes
Comment thread pkg/statistics/handle/ddl/ddl.go Outdated
}

// Remove the old table stats.
if err := h.statsWriter.ResetTableStats2KVForDrop(droppedTableInfo.ID); err != nil {

@0xPoe 0xPoe Dec 21, 2023

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should drop all partitions as well.

@ti-chi-bot

ti-chi-bot Bot commented Dec 21, 2023

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

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

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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 ti-chi-bot Bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Dec 21, 2023
@ti-chi-bot

ti-chi-bot Bot commented Dec 21, 2023

Copy link
Copy Markdown

[LGTM Timeline notifier]

Timeline:

  • 2023-12-21 07:38:39.162266316 +0000 UTC m=+1119410.199493266: ☑️ agreed by hawkingrei.
  • 2023-12-21 09:52:39.329871298 +0000 UTC m=+1127450.367098240: ☑️ agreed by qw4990.

@0xPoe 0xPoe changed the title statistics: drop old stats meta after truncate a table WIP: statistics: drop old stats meta after truncate a table Dec 21, 2023
@ti-chi-bot ti-chi-bot Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 21, 2023
@0xPoe
0xPoe marked this pull request as draft December 21, 2023 12:10
@ti-chi-bot ti-chi-bot Bot removed the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Dec 22, 2023
@ti-chi-bot ti-chi-bot Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Dec 22, 2023
0xPoe
0xPoe commented Dec 22, 2023
View reviewed changes

@0xPoe 0xPoe left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔢 Self-check

@0xPoe

0xPoe commented Dec 22, 2023

Copy link
Copy Markdown
Member Author

/unhold

@0xPoe
0xPoe marked this pull request as ready for review December 22, 2023 05:11
@ti-chi-bot 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 Dec 22, 2023
@0xPoe 0xPoe changed the title WIP: statistics: drop old stats meta after truncate a table statistics: drop old stats meta after truncate a table Dec 22, 2023
@ti-chi-bot ti-chi-bot Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 22, 2023
@ti-chi-bot
ti-chi-bot Bot merged commit 98236ca into pingcap:master Dec 22, 2023
Reminiscent pushed a commit to Reminiscent/tidb that referenced this pull request May 26, 2026
yanchengpeng
statistics: update old stats meta after truncate table
73db175 
This semantically backports:
pingcap#49664

Original commit:
98236ca statistics: drop old stats meta after truncate a table (pingcap#49664)

The v7.5.6 backport carries the old TableInfo in the existing DDL event
path and uses ResetTableStats2KVForDrop to mark stale stats for GC.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@qw4990 qw4990 qw4990 approved these changes
+1 more reviewer
@hawkingrei hawkingrei hawkingrei approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

approved component/statistics lgtm release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/planner SIG: Planner size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

After truncating a table in TiDB, the old table's stats meta is not deleted

3 participants

@0xPoe @hawkingrei @qw4990