Skip to content

statistics: ignore system tables in stats cache#64097

Merged
ti-chi-bot[bot] merged 5 commits intopingcap:masterfrom
0xPoe:poe-patch-ignore-sys-tables
Nov 21, 2025
Merged

statistics: ignore system tables in stats cache#64097
ti-chi-bot[bot] merged 5 commits intopingcap:masterfrom
0xPoe:poe-patch-ignore-sys-tables

Conversation

@0xPoe
Copy link
Copy Markdown
Member

@0xPoe 0xPoe commented Oct 22, 2025

What problem does this PR solve?

Issue Number: close #64080, close #57176

Problem Summary:

What changed and how does it work?

  1. Avoid putting system tables into the stats cache, as it would pollute the cache.
  2. Do not always place small tables in the “unneeded” range; otherwise, it may confuse users into thinking the table hasn’t been analyzed when it actually has.
  3. Added a pseudo label to separate it from the unneeded label.

After these changes: total tables = pseudo tables + unneeded analyze tables + tables in healthy buckets.

Check List

Tests

  • Unit test
  • Integration test
  • Manual test
  • 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 the issue where system tables could pollute the stats health metrics

@ti-chi-bot ti-chi-bot Bot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. sig/planner SIG: Planner size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 22, 2025
@0xPoe 0xPoe force-pushed the poe-patch-ignore-sys-tables branch from 657ec9b to 491112f Compare October 22, 2025 09:53
@0xPoe 0xPoe changed the title WIP: statistics: remove global max table id variable WIP: statistics: ignore system tables in stats cache Oct 22, 2025
@ti-chi-bot ti-chi-bot Bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. needs-cherry-pick-release-8.5 Should cherry pick this PR to release-8.5 branch. needs-cherry-pick-release-7.5 Should cherry pick this PR to release-7.5 branch. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 22, 2025
@0xPoe 0xPoe force-pushed the poe-patch-ignore-sys-tables branch from 93b7a77 to 2c31275 Compare October 22, 2025 09:59
@ti-chi-bot 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 Oct 22, 2025
@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 22, 2025

Codecov Report

❌ Patch coverage is 88.88889% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.7222%. Comparing base (82e2c6f) to head (cf127b0).
⚠️ Report is 45 commits behind head on master.

Additional details and impacted files
@@               Coverage Diff                @@
##             master     #64097        +/-   ##
================================================
+ Coverage   72.6962%   74.7222%   +2.0259%     
================================================
  Files          1865       1889        +24     
  Lines        506195     523002     +16807     
================================================
+ Hits         367985     390799     +22814     
+ Misses       115838     108211      -7627     
- Partials      22372      23992      +1620     
Flag Coverage Δ
integration 48.1535% <47.8632%> (?)
unit 72.5433% <88.8888%> (+0.2772%) ⬆️

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

Components Coverage Δ
dumpling 52.8700% <ø> (ø)
parser ∅ <ø> (∅)
br 62.3775% <ø> (+16.1619%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ti-chi-bot 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 Oct 22, 2025
@0xPoe 0xPoe requested a review from Copilot October 22, 2025 11:42
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR prevents system tables from being cached in the statistics cache to avoid memory overhead and unnecessary cache pollution. The change addresses issues where commands like show stats_meta and show stats_healthy would cause system tables to be added to the stats cache.

Key Changes:

  • Added logic to detect and skip caching pseudo table statistics for system tables
  • Refactored stats healthy metrics calculation with improved code structure
  • Added test coverage to verify system tables are not cached

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pkg/statistics/handle/handle.go Core logic to filter out system tables from stats cache using schema detection
pkg/statistics/handle/handletest/handle_test.go Test case verifying system tables don't pollute the cache after show commands
pkg/statistics/handle/cache/statscache.go Refactored healthy metrics calculation with cleaner bucket indexing logic
pkg/statistics/handle/cache/statscache_test.go New test for UpdateStatsHealthyMetrics with mock infrastructure
pkg/statistics/handle/handletest/BUILD.bazel Incremented shard count for additional test
pkg/statistics/handle/cache/BUILD.bazel Added dependencies for new test utilities
pkg/statistics/handle/BUILD.bazel Added filter package dependency

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread pkg/statistics/handle/handle.go Outdated
Comment thread pkg/statistics/handle/cache/statscache.go
@ti-chi-bot ti-chi-bot Bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 22, 2025
@0xPoe 0xPoe force-pushed the poe-patch-ignore-sys-tables branch from 7a5d152 to 61ab8e0 Compare October 22, 2025 13:09
@0xPoe 0xPoe changed the title WIP: statistics: ignore system tables in stats cache statistics: ignore system tables in stats cache Oct 22, 2025
@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 Oct 22, 2025
@0xPoe 0xPoe requested a review from Copilot October 22, 2025 13:26
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread pkg/statistics/handle/cache/statscache.go Outdated
Copy link
Copy Markdown
Member Author

@0xPoe 0xPoe left a comment

Choose a reason for hiding this comment

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

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

  • Code compiles successfully

  • Unit tests added

  • All tests pass

  • Bazel files updated

  • Comments added where necessary

  • PR title and description updated

  • Documentation PR created (or confirmed not needed)

  • PR size is reasonable

/cc @time-and-fate @winoros

@ti-chi-bot ti-chi-bot Bot requested review from time-and-fate and winoros October 22, 2025 13:29
@ti-chi-bot ti-chi-bot Bot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Oct 22, 2025
@0xPoe
Copy link
Copy Markdown
Member Author

0xPoe commented Nov 12, 2025

/test all

test: fix borken tests

test: fix broken tests

fix: update bazel

fix

fix: update

fix

test: fix broken tests

fix: bazel update

fix: rename

fix: better code

refatcor: better code

fix: better code

test: assert the labels

fix: remove useless file

fix: make lint happy

fix: correct the failpoint name

fix: update comments

feat: add isMemSchemaID function

feat: add system schema cache

refator: use IsMemSchemaID

fix: make lint happy

fix: move to the right place

fix: remove

fix

doc: add comments

test: assert the cache

fix: make lint happy
@0xPoe 0xPoe force-pushed the poe-patch-ignore-sys-tables branch from c6a13a8 to 05db961 Compare November 12, 2025 13:44
@0xPoe
Copy link
Copy Markdown
Member Author

0xPoe commented Nov 12, 2025

Just rebased. Nothing changed.

@0xPoe
Copy link
Copy Markdown
Member Author

0xPoe commented Nov 12, 2025

/test all

@0xPoe
Copy link
Copy Markdown
Member Author

0xPoe commented Nov 12, 2025

/retest

1 similar comment
@0xPoe
Copy link
Copy Markdown
Member Author

0xPoe commented Nov 12, 2025

/retest

@0xPoe
Copy link
Copy Markdown
Member Author

0xPoe commented Nov 13, 2025

/test all

@0xPoe
Copy link
Copy Markdown
Member Author

0xPoe commented Nov 13, 2025

/retest

2 similar comments
@0xPoe
Copy link
Copy Markdown
Member Author

0xPoe commented Nov 13, 2025

/retest

@0xPoe
Copy link
Copy Markdown
Member Author

0xPoe commented Nov 13, 2025

/retest

@0xPoe
Copy link
Copy Markdown
Member Author

0xPoe commented Nov 13, 2025

/test all

@0xPoe
Copy link
Copy Markdown
Member Author

0xPoe commented Nov 13, 2025

/retest

2 similar comments
@0xPoe
Copy link
Copy Markdown
Member Author

0xPoe commented Nov 13, 2025

/retest

@0xPoe
Copy link
Copy Markdown
Member Author

0xPoe commented Nov 13, 2025

/retest

fix: no need to assert type

fix: check if the pool is nil

fix
@0xPoe 0xPoe force-pushed the poe-patch-ignore-sys-tables branch from fd54cdf to cf127b0 Compare November 21, 2025 11:35
@0xPoe
Copy link
Copy Markdown
Member Author

0xPoe commented Nov 21, 2025

/retest

1 similar comment
@0xPoe
Copy link
Copy Markdown
Member Author

0xPoe commented Nov 21, 2025

/retest

@ti-chi-bot ti-chi-bot Bot merged commit 9f3ae48 into pingcap:master Nov 21, 2025
32 checks passed
@ti-chi-bot
Copy link
Copy Markdown
Member

In response to a cherrypick label: new pull request created to branch release-7.5: #64627.
But this PR has conflicts, please resolve them!

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Nov 21, 2025
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
@ti-chi-bot
Copy link
Copy Markdown
Member

In response to a cherrypick label: new pull request created to branch release-8.5: #64628.
But this PR has conflicts, please resolve them!

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Nov 21, 2025
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
ti-chi-bot Bot pushed a commit that referenced this pull request Dec 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved component/statistics lgtm needs-cherry-pick-release-7.5 Should cherry pick this PR to release-7.5 branch. needs-cherry-pick-release-8.5 Should cherry pick this PR to release-8.5 branch. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/planner SIG: Planner size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stats Healthy metric cannot display the stats health correctly The statistics metric cannot correctly display the stats health

6 participants