gc_worker: Adapt to new GC API and support keyspace level GC #60632

MyonKeminta · 2025-04-17T09:43:31Z

What problem does this PR solve?

Issue Number: ref #58720

Problem Summary:

What changed and how does it work?

This PR updates the implementation of GCWorker to adapt the changes of PD's GC related APIs made in tikv/pd#8978 . This update also makes it able to support keyspace level GC.

Requires:

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

ti-chi-bot · 2025-04-17T09:43:35Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

tiprow · 2025-04-17T09:43:46Z

Hi @MyonKeminta. 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.

Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>

…factor

…actor

MyonKeminta · 2025-06-09T07:06:58Z

/ok-to-test

Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>

MyonKeminta · 2025-06-09T09:38:36Z

/test all

codecov · 2025-06-09T09:55:33Z

Codecov Report

Attention: Patch coverage is 80.74074% with 26 lines in your changes missing coverage. Please review.

Project coverage is 75.0039%. Comparing base (a2203f8) to head (347646e).
Report is 13 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #60632        +/-   ##
================================================
+ Coverage   73.1084%   75.0039%   +1.8955%     
================================================
  Files          1729       1775        +46     
  Lines        481032     490590      +9558     
================================================
+ Hits         351675     367962     +16287     
+ Misses       107827      99680      -8147     
- Partials      21530      22948      +1418

Flag	Coverage Δ
integration	`48.8603% <0.7407%> (?)`
unit	`72.4445% <80.7407%> (+0.0642%)`	⬆️

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

Components	Coverage Δ
dumpling	`52.7804% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`63.0047% <ø> (+16.0260%)`	⬆️

🚀 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.

Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>

MyonKeminta · 2025-06-09T10:15:30Z

/test all

Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>

MyonKeminta · 2025-06-10T08:28:24Z

/retest

cfzjywxk · 2025-06-10T09:12:11Z

pkg/store/gcworker/gc_worker.go

+	// TODO: Use result of AdvanceTxnSafePoint instead, which makes it unnecessary to manually check the config.
+	const cfgGCManagementType = "gc_management_type"
+	const cfgGCManagementTypeKeyspaceLevel = "keyspace_level"
+	if keyspaceMeta := w.store.GetCodec().GetKeyspaceMeta(); keyspaceMeta != nil && keyspaceMeta.Config[cfgGCManagementType] != cfgGCManagementTypeKeyspaceLevel {


@MyonKeminta How about pritting some logs here to display which keyspace name is configured and information about the mode?

ti-chi-bot · 2025-06-10T09:13:12Z

[LGTM Timeline notifier]

Timeline:

2025-06-10 09:11:44.400208174 +0000 UTC m=+346282.628523433: ☑️ agreed by tangenta.
2025-06-10 09:13:10.926344787 +0000 UTC m=+346369.154660051: ☑️ agreed by cfzjywxk.

ti-chi-bot · 2025-06-10T09:48:49Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cfzjywxk, tangenta, tiancaiamao

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:

~~OWNERS~~ [cfzjywxk,tangenta,tiancaiamao]
~~pkg/ddl/OWNERS~~ [tangenta]

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

MyonKeminta · 2025-06-10T10:22:52Z

/retest

cfzjywxk · 2025-06-10T11:06:05Z

/test check-dev2

tiprow · 2025-06-10T11:06:27Z

@cfzjywxk: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test fast_test_tiprow
/test tidb_parser_test

Use /test all to run all jobs.

Details

In response to this:

/test check-dev2

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.

cfzjywxk · 2025-06-10T11:08:28Z

/retest

tiancaiamao · 2025-06-10T12:06:53Z

/merge

ti-chi-bot · 2025-06-10T12:06:55Z

@tiancaiamao: We have migrated to builtin LGTM and approve plugins for reviewing.

👉 Please use /approve when you want approve this pull request.

The changes announcement: LGTM plugin changes

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 ti-community-infra/tichi repository.

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. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Apr 17, 2025

MyonKeminta mentioned this pull request Apr 17, 2025

Refactor GC related PD APIs and GC procedures #58720

Open

8 tasks

MyonKeminta force-pushed the m/gc-refactor branch from 0d55052 to 01a6f2f Compare April 21, 2025 09:32

ti-chi-bot bot added component/dumpling This is related to Dumpling of TiDB. sig/planner SIG: Planner labels Apr 21, 2025

MyonKeminta changed the base branch from dedicated to feature/next-gen-tidb April 21, 2025 09:32

MyonKeminta added 3 commits April 27, 2025 16:54

gc_worker: Adapt to new GC API and support keyspace level GC

0f16e86

Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>

Update client-go

10e18c5

Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>

update dependencies and fix build

0a4b70b

Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>

MyonKeminta force-pushed the m/gc-refactor branch from 01a6f2f to 0a4b70b Compare April 27, 2025 09:52

MyonKeminta changed the base branch from feature/next-gen-tidb to master April 27, 2025 09:52

Fix incorrect check in gc management type; fix incorrect kvproto version

cf925bc

Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>

MyonKeminta removed the component/dumpling This is related to Dumpling of TiDB. label May 15, 2025

MyonKeminta added 3 commits May 15, 2025 18:13

Merge branch 'master' of https://github.com/pingcap/tidb into m/gc-re…

68eb41d

…factor

Merge commit '6f1f815198818f82fcf3e1010e0dff36dae06c1f' into m/gc-ref…

5c4e267

…actor

Merge remote-tracking branch 'upstream/master' into m/gc-refactor

772cf08

ti-chi-bot bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jun 9, 2025

ti-chi-bot bot added the ok-to-test Indicates a PR is ready to be tested. label Jun 9, 2025

update client-go; fix build error; fix error type conversion

30d421f

Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>

RidRisR mentioned this pull request Jun 9, 2025

br: enhance table existence check #61596

Merged

13 tasks

update bazel

c297755

Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>

Add comments

347646e

Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>

MyonKeminta marked this pull request as ready for review June 10, 2025 09:02

tangenta approved these changes Jun 10, 2025

View reviewed changes

ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Jun 10, 2025

cfzjywxk approved these changes Jun 10, 2025

View reviewed changes

ti-chi-bot bot added approved lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jun 10, 2025

MyonKeminta added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Jun 10, 2025

tiancaiamao approved these changes Jun 10, 2025

View reviewed changes

tiancaiamao added sig/transaction SIG:Transaction and removed sig/planner SIG: Planner labels Jun 10, 2025

ti-chi-bot bot merged commit 5ea7f4e into pingcap:master Jun 10, 2025
27 checks passed

MyonKeminta deleted the m/gc-refactor branch June 11, 2025 02:21

MyonKeminta mentioned this pull request Jun 20, 2025

[DNM]GC: Support keyspace level GC #51300

Closed

6 tasks

owlsome2501 mentioned this pull request Jun 30, 2025

Lightning encountered error PD server timeout and remote checksum failed #61833

Open

tiancaiamao mentioned this pull request Jul 7, 2025

GC is not blocked when query is running #61011

Closed

gc_worker: Adapt to new GC API and support keyspace level GC #60632

gc_worker: Adapt to new GC API and support keyspace level GC #60632

Uh oh!

Conversation

MyonKeminta commented Apr 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

Uh oh!

ti-chi-bot bot commented Apr 17, 2025

Uh oh!

tiprow bot commented Apr 17, 2025

Uh oh!

MyonKeminta commented Jun 9, 2025

Uh oh!

MyonKeminta commented Jun 9, 2025

Uh oh!

codecov bot commented Jun 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

MyonKeminta commented Jun 9, 2025

Uh oh!

MyonKeminta commented Jun 10, 2025

Uh oh!

cfzjywxk Jun 10, 2025

Choose a reason for hiding this comment

Uh oh!

ti-chi-bot bot commented Jun 10, 2025

[LGTM Timeline notifier]

Uh oh!

ti-chi-bot bot commented Jun 10, 2025

Uh oh!

MyonKeminta commented Jun 10, 2025

Uh oh!

cfzjywxk commented Jun 10, 2025

Uh oh!

tiprow bot commented Jun 10, 2025

Uh oh!

cfzjywxk commented Jun 10, 2025

Uh oh!

tiancaiamao commented Jun 10, 2025

Uh oh!

ti-chi-bot bot commented Jun 10, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

MyonKeminta commented Apr 17, 2025 •

edited

Loading

codecov bot commented Jun 9, 2025 •

edited

Loading