Skip to content

ddl: fix ALTER TABLE NOCACHE not removing record from mysql.table_cache_meta - #67109

Merged
ti-chi-bot[bot] merged 1 commit into
pingcap:masterfrom
expxiaoli:fix_66042
Mar 24, 2026
Merged

ti-chi-bot[bot] merged 1 commit into
pingcap:masterfrom
expxiaoli:fix_66042

Conversation

@expxiaoli

@expxiaoli expxiaoli commented Mar 18, 2026 •

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: close #66042

Problem Summary:

When executing ALTER TABLE t NOCACHE, TiDB correctly updates the table's TableCacheStatusType to Disable, but does not delete the corresponding row from mysql.table_cache_meta. As a result, after ALTER TABLE NOCACHE, the stale record remains in mysql.table_cache_meta (with lock_type = READ if a SELECT was issued while the table was cached), violating the expected invariant that cached-table metadata only exists for tables currently in cache mode.

What changed and how does it work?

In finishDDLJob (pkg/ddl/job_worker.go), added a new case for ActionAlterNoCacheTable: when the job completes successfully, delete the row for the table from mysql.table_cache_meta. This mirrors the insert done during ALTER TABLE CACHE (in executor.go) and ensures the two operations stay symmetric.

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No need to test

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

Fix `ALTER TABLE NOCACHE` not removing the stale record from `mysql.table_cache_meta`.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed improper removal of table cache metadata when executing ALTER TABLE NOCACHE, ensuring cache entries are properly cleaned up and no orphaned metadata remains.

@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 Mar 18, 2026
@pantheon-ai

pantheon-ai Bot commented Mar 18, 2026 •

Copy link
Copy Markdown

Review Complete

Findings: 0 issues
Posted: 0
Duplicates/Skipped: 0

ℹ️ Learn more details on Pantheon AI.

@ti-chi-bot ti-chi-bot Bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Mar 18, 2026
@tiprow

tiprow Bot commented Mar 18, 2026

Copy link
Copy Markdown

Hi @expxiaoli. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

Details

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-sigs/prow repository.

@coderabbitai

coderabbitai Bot commented Mar 18, 2026 •

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 8184cf3a-93c4-4e57-a656-d31cc9c63f31

📥 Commits

Reviewing files that changed from the base of the PR and between 0f00d92 and c412273.

📒 Files selected for processing (2)
  • pkg/ddl/db_cache_test.go
  • pkg/ddl/job_worker.go

📝 Walkthrough

Walkthrough

The changes implement a fix for ALTER TABLE NOCACHE to properly remove the corresponding entry from the mysql.table_cache_meta table. A new DDL job handler branch executes cleanup SQL to delete the cache metadata record, accompanied by a test verifying the removal behavior.

Changes

Cohort / File(s) Summary
ALTER TABLE NOCACHE cache cleanup
pkg/ddl/job_worker.go, pkg/ddl/db_cache_test.go
Added DDL job handler for ActionAlterNoCacheTable that executes cleanup SQL to remove the table entry from mysql.table_cache_meta, with a corresponding test validating the metadata record is properly deleted after ALTER TABLE NOCACHE.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A cache that fades without a trace,
No orphaned rows cluttering the space,
When NOCACHE is set with gentle might,
Metadata vanishes—squeaky clean and right! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: fixing ALTER TABLE NOCACHE not removing records from mysql.table_cache_meta, which is the primary objective of the PR.
Description check ✅ Passed The description follows the template with clear problem statement, solution explanation, and appropriate checkboxes marked; includes issue reference #66042 and a release note.
Linked Issues check ✅ Passed The code changes fully address issue #66042 by adding a delete operation in finishDDLJob for ActionAlterNoCacheTable and including a unit test to verify the mysql.table_cache_meta row is deleted.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the ALTER TABLE NOCACHE bug: one file adds a unit test and another adds the cleanup logic in DDL job handling.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
📝 Coding Plan
  • Generate coding plan for human review comments

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.11.3)

Command failed


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov

codecov Bot commented Mar 18, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 71.42857% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.5145%. Comparing base (102f7af) to head (c412273).
⚠️ Report is 41 commits behind head on master.

Additional details and impacted files
@@               Coverage Diff                @@
##             master     #67109        +/-   ##
================================================
+ Coverage   77.7098%   80.5145%   +2.8047%     
================================================
  Files          2016       1968        -48     
  Lines        552001     555589      +3588     
================================================
+ Hits         428959     447330     +18371     
+ Misses       121300     106794     -14506     
+ Partials       1742       1465       -277     
Flag Coverage Δ
integration 48.6292% <57.1428%> (+0.5024%) ⬆️
unit 77.6974% <71.4285%> (+1.4460%) ⬆️

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

Components Coverage Δ
dumpling 63.6984% <ø> (+6.6885%) ⬆️
parser ∅ <ø> (∅)
br 68.5750% <ø> (+7.7605%) ⬆️
🚀 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.

@expxiaoli

Copy link
Copy Markdown
Contributor Author

/retest-required

@tiprow

tiprow Bot commented Mar 18, 2026

Copy link
Copy Markdown

@expxiaoli: PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test.

Details

In response to this:

/retest-required

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-sigs/prow repository.

@expxiaoli

Copy link
Copy Markdown
Contributor Author

/retest-required

@tiprow

tiprow Bot commented Mar 18, 2026

Copy link
Copy Markdown

@expxiaoli: PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test.

Details

In response to this:

/retest-required

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-sigs/prow repository.

@pantheon-ai pantheon-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Code looks good. No issues found.

@expxiaoli

Copy link
Copy Markdown
Contributor Author

/retest-required

@tiprow

tiprow Bot commented Mar 18, 2026

Copy link
Copy Markdown

@expxiaoli: PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test.

Details

In response to this:

/retest-required

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-sigs/prow repository.

@ti-chi-bot ti-chi-bot Bot added approved needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Mar 23, 2026
@ti-chi-bot

ti-chi-bot Bot commented Mar 23, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tiancaiamao, wjhuang2016

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 Mar 23, 2026
@ti-chi-bot

ti-chi-bot Bot commented Mar 23, 2026

Copy link
Copy Markdown

[LGTM Timeline notifier]

Timeline:

  • 2026-03-23 06:11:32.977088932 +0000 UTC m=+162289.013159192: ☑️ agreed by wjhuang2016.
  • 2026-03-23 09:28:19.878642652 +0000 UTC m=+174095.914712922: ☑️ agreed by tiancaiamao.

@expxiaoli

Copy link
Copy Markdown
Contributor Author

/retest-required

@tiprow

tiprow Bot commented Mar 24, 2026

Copy link
Copy Markdown

@expxiaoli: PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test.

Details

In response to this:

/retest-required

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-sigs/prow repository.

@expxiaoli

Copy link
Copy Markdown
Contributor Author

/retest-required

@tiprow

tiprow Bot commented Mar 24, 2026

Copy link
Copy Markdown

@expxiaoli: PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test.

Details

In response to this:

/retest-required

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-sigs/prow repository.

@ti-chi-bot
ti-chi-bot Bot merged commit 8e791c9 into pingcap:master Mar 24, 2026
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved lgtm release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ALTER TABLE NOCACHE does not remove record from mysql.table_cache_meta

3 participants