client: recognize v2 keyspace-level GC by pingyu · Pull Request #11100 · tikv/pd

pingyu · 2026-08-03T15:24:54Z

What problem does this PR solve?

Issue Number: close #11108

TiDB Cloud Essential v1 keyspaces use safe_point_version=v2 to indicate keyspace-level GC. Existing downstream code already relies on IsKeyspaceUsingKeyspaceLevelGC, but that predicate only recognizes gc_management_type=keyspace_level.

What is changed and how does it work?

Extend IsKeyspaceUsingKeyspaceLevelGC to recognize either native keyspace-level GC metadata or the exact safe_point_version=v2 setting. This keeps PD terminology deployment-agnostic and lets existing client-go and TiDB callers select the correct keyspace-scoped metadata without component-specific helpers.

client: recognize v2 keyspace-level GC

Check List

Tests

Unit test: make basic-test in client/
Focused test: make gotest GOTEST_ARGS='./... -run TestIsKeyspaceUsingKeyspaceLevelGC -count=1' in client/
Manual test: imported the PD client from a standalone driver and verified native metadata returns true, safe_point_version=v2 returns true, and malformed V2 returns false

Related changes

tikv: support CSE keyspace-level transaction safepoint client-go#2040 and metaservice: support CSE keyspace-level GC pingcap/tidb#70322 were closed as superseded because their existing API v3 bases already call this predicate.
*: update kvproto to apiv3 client-go#2024, *: update kvproto to apiv3 pingcap/tidb#69942, and *: fix CSE keyspace GC safepoint compatibility pingcap/ticdc#5870 remain the API v3 dependency stack.

Release note

Recognize version 2 safe-point metadata as keyspace-level GC.

Summary by CodeRabbit

Bug Fixes
- Improved keyspace garbage-collection detection for missing or empty configurations.
- Added support for uppercase and padded configuration values.
- Ensured explicit garbage-collection management settings take precedence over fallback safe-point version settings.
- Added validation for supported safe-point versions and invalid configurations.

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> Signed-off-by: Ping Yu <yuping@pingcap.com>

coderabbitai · 2026-08-03T15:25:27Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8230a39a-28a3-4fbd-9f13-d27c2b1b1311

📥 Commits

Reviewing files that changed from the base of the PR and between 31378cb and a90222b.

📒 Files selected for processing (2)

client/client_test.go
client/keyspace_client.go

🚧 Files skipped from review as they are similar to previous changes (1)

client/client_test.go

📝 Walkthrough

Walkthrough

The client now prioritizes gc_management_type and checks safe_point_version=v2 only when that setting is absent. Table-driven tests cover nil, empty, native, unified, invalid, formatted, and overridden configurations.

Changes

CES keyspace-level GC detection

Layer / File(s)	Summary
Safe-point version detection and validation `client/keyspace_client.go`, `client/client_test.go`	The client defines private safe-point constants, prioritizes explicit GC management types, and falls back to the exact `v2` value. Tests cover supported, invalid, formatted, and overridden configurations.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

tikv/pd#11054: Modifies keyspace-level GC mode detection.

Suggested reviewers: rleungx, disksing

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the client change to recognize v2 keyspace-level GC.
Description check	✅ Passed	The description includes the issue, problem, implementation, tests, related changes, and release note.
Linked Issues check	✅ Passed	The implementation preserves native detection and recognizes exact v2 metadata with the required nil, case, padding, and precedence behavior for `#11108`.
Out of Scope Changes check	✅ Passed	The code and test changes are limited to keyspace-level GC detection and its required coverage.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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

codecov · 2026-08-03T15:41:10Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.41%. Comparing base (e000290) to head (b11cde1).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #11100      +/-   ##
==========================================
+ Coverage   79.35%   79.41%   +0.06%     
==========================================
  Files         542      542              
  Lines       76993    77006      +13     
==========================================
+ Hits        61097    61156      +59     
+ Misses      11594    11557      -37     
+ Partials     4302     4293       -9

Flag	Coverage Δ
unittests	`79.41% <100.00%> (+0.06%)`	⬆️

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

pingyu · 2026-08-04T15:10:08Z

/retest

D3Hunter · 2026-08-05T01:58:44Z

+// IsCESKeyspaceLevelGC reports whether a keyspace uses the CES keyspace-level GC metadata format.
+func IsCESKeyspaceLevelGC(keyspaceMeta *keyspacepb.KeyspaceMeta) bool {


Suggested change

// IsCESKeyspaceLevelGC reports whether a keyspace uses the CES keyspace-level GC metadata format.

func IsCESKeyspaceLevelGC(keyspaceMeta *keyspacepb.KeyspaceMeta) bool {

// IsCSEKeyspaceLevelGC reports whether a keyspace uses the CSE keyspace-level GC metadata format.

func IsCSEKeyspaceLevelGC(keyspaceMeta *keyspacepb.KeyspaceMeta) bool {

D3Hunter · 2026-08-05T01:59:51Z

 	re.False(IsKeyspaceUsingKeyspaceLevelGC(meta))
 }
+
+func TestIsCESKeyspaceLevelGC(t *testing.T) {


Signed-off-by: Ping Yu <yuping@pingcap.com>

rleungx · 2026-08-05T02:20:30Z

 }
+
+// IsCSEKeyspaceLevelGC reports whether a keyspace uses the CSE keyspace-level GC metadata format.
+func IsCSEKeyspaceLevelGC(keyspaceMeta *keyspacepb.KeyspaceMeta) bool {


Can we extend the existing IsKeyspaceUsingKeyspaceLevelGC? PD doesn't know what CSE is.

yes， just rename as v2

bufferflies

rest lgtm

Apply review feedback by extending the existing keyspace-level GC predicate instead of exposing a CSE-specific API. Signed-off-by: Ping Yu <yuping@pingcap.com>

rleungx · 2026-08-05T05:44:43Z

+	if keyspaceMeta == nil || keyspaceMeta.Config == nil {
+		return false
+	}
+	return keyspaceMeta.Config[KeyspaceConfigGCManagementType] == KeyspaceConfigGCManagementTypeKeyspaceLevel ||


@ystaticy PTAL

pingyu · 2026-08-05T06:10:44Z

/retest

pingyu · 2026-08-05T06:34:53Z

/retest

pingyu · 2026-08-05T07:51:33Z

/retest

ti-chi-bot · 2026-08-05T08:10:40Z

[LGTM Timeline notifier]

Timeline:

2026-08-05 03:31:26.448960265 +0000 UTC m=+2585272.485055320: ☑️ agreed by bufferflies.
2026-08-05 08:10:39.718194153 +0000 UTC m=+2602025.754289219: ☑️ agreed by ystaticy.

pingyu · 2026-08-05T08:14:52Z

/retest

wfxr · 2026-08-05T09:00:52Z

+	if keyspaceMeta == nil || keyspaceMeta.Config == nil {
+		return false
+	}
+	return keyspaceMeta.Config[KeyspaceConfigGCManagementType] == KeyspaceConfigGCManagementTypeKeyspaceLevel ||


gpt-5.6-sol:

This condition treats safe_point_version == "v2" as authoritative even when gc_management_type is explicitly set. Because keyspace config updates use PATCH semantics, an upgraded keyspace may retain the legacy field:

Config: map[string]string{ "gc_management_type": "unified", "safe_point_version": "v2", }

The current || expression returns true for this configuration, causing callers to use the keyspace-level GC/safepoint path even though the canonical field selects unified.

Consider checking gc_management_type first and falling back to safe_point_version only when the canonical field is absent. Please also add a regression test for this conflicting-fields case.

Signed-off-by: Ping Yu <yuping@pingcap.com>

…v1-gc Signed-off-by: Ping Yu <yuping@pingcap.com>

pingyu · 2026-08-05T10:02:16Z

/retest

ti-chi-bot · 2026-08-05T10:04:08Z

@wfxr: adding LGTM is restricted to approvers and reviewers in OWNERS files.

Details

In response to this:

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 · 2026-08-05T10:04:09Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bufferflies, wfxr, ystaticy

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~~ [bufferflies]

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

pingyu · 2026-08-05T10:12:28Z

/retest

rleungx · 2026-08-05T10:20:23Z

/retest

client: identify CES keyspace-level GC

3b527fd

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> Signed-off-by: Ping Yu <yuping@pingcap.com>

ti-chi-bot Bot added do-not-merge/needs-triage-completed and removed do-not-merge/needs-linked-issue labels Aug 4, 2026

ti-chi-bot Bot removed the do-not-merge/needs-triage-completed label Aug 4, 2026

D3Hunter reviewed Aug 5, 2026

View reviewed changes

fix typo

ed4d591

Signed-off-by: Ping Yu <yuping@pingcap.com>

pingyu changed the title ~~client: identify CES keyspace-level GC~~ client: identify CSE keyspace-level GC Aug 5, 2026

rleungx reviewed Aug 5, 2026

View reviewed changes

bufferflies approved these changes Aug 5, 2026

View reviewed changes

ti-chi-bot Bot added needs-1-more-lgtm Indicates a PR needs 1 more LGTM. approved labels Aug 5, 2026

client: recognize v2 keyspace-level GC

31378cb

Apply review feedback by extending the existing keyspace-level GC predicate instead of exposing a CSE-specific API. Signed-off-by: Ping Yu <yuping@pingcap.com>

This was referenced Aug 5, 2026

metaservice: support CSE keyspace-level GC pingcap/tidb#70322

Closed

metaservice: dedicated metadata group is rejected for CSE keyspace-level GC pingcap/tidb#70332

Closed

pingyu changed the title ~~client: identify CSE keyspace-level GC~~ client: recognize v2 keyspace-level GC Aug 5, 2026

rleungx reviewed Aug 5, 2026

View reviewed changes

ystaticy approved these changes Aug 5, 2026

View reviewed changes

ti-chi-bot Bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Aug 5, 2026

wfxr reviewed Aug 5, 2026

View reviewed changes

pingyu added 2 commits August 5, 2026 17:29

treat gc_management_type as authoritative

a90222b

Signed-off-by: Ping Yu <yuping@pingcap.com>

Merge remote-tracking branch 'upstream/master' into compat-essential-…

b11cde1

…v1-gc Signed-off-by: Ping Yu <yuping@pingcap.com>

wfxr approved these changes Aug 5, 2026

View reviewed changes

ti-chi-bot Bot merged commit afa4311 into tikv:master Aug 5, 2026
32 checks passed

pingyu mentioned this pull request Aug 5, 2026

TestMicroserviceTSOClientSuite/TestGetMinTS is flaky #11113

Open

disksing mentioned this pull request Aug 5, 2026

*: update kvproto to apiv3 tikv/client-go#2024

Merged

		// IsCESKeyspaceLevelGC reports whether a keyspace uses the CES keyspace-level GC metadata format.
		func IsCESKeyspaceLevelGC(keyspaceMeta *keyspacepb.KeyspaceMeta) bool {

Uh oh!

Conversation

pingyu commented Aug 3, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What is changed and how does it work?

Check List

Release note

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

codecov Bot commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

pingyu commented Aug 4, 2026

Uh oh!

D3Hunter Aug 5, 2026

Choose a reason for hiding this comment

Uh oh!

D3Hunter Aug 5, 2026

Choose a reason for hiding this comment

Uh oh!

rleungx Aug 5, 2026

Choose a reason for hiding this comment

Uh oh!

bufferflies Aug 5, 2026

Choose a reason for hiding this comment

Uh oh!

bufferflies left a comment

Choose a reason for hiding this comment

Uh oh!

rleungx Aug 5, 2026

Choose a reason for hiding this comment

Uh oh!

ystaticy Aug 5, 2026

Choose a reason for hiding this comment

Uh oh!

pingyu commented Aug 5, 2026

Uh oh!

pingyu commented Aug 5, 2026

Uh oh!

pingyu commented Aug 5, 2026

Uh oh!

ti-chi-bot Bot commented Aug 5, 2026

[LGTM Timeline notifier]

Uh oh!

pingyu commented Aug 5, 2026

Uh oh!

wfxr Aug 5, 2026

Choose a reason for hiding this comment

Uh oh!

pingyu commented Aug 5, 2026

Uh oh!

ti-chi-bot Bot commented Aug 5, 2026

Uh oh!

ti-chi-bot Bot commented Aug 5, 2026

Uh oh!

pingyu commented Aug 5, 2026

Uh oh!

rleungx commented Aug 5, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

pingyu commented Aug 3, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Aug 3, 2026 •

edited

Loading

codecov Bot commented Aug 3, 2026 •

edited

Loading