Skip to content

*: Update kvproto, pd client, client-go to follow GC API changes#68843

Merged
ti-chi-bot[bot] merged 5 commits into
pingcap:masterfrom
MyonKeminta:m/update-gc-state-api
Jun 2, 2026
Merged

*: Update kvproto, pd client, client-go to follow GC API changes#68843
ti-chi-bot[bot] merged 5 commits into
pingcap:masterfrom
MyonKeminta:m/update-gc-state-api

Conversation

@MyonKeminta
Copy link
Copy Markdown
Contributor

@MyonKeminta MyonKeminta commented Jun 1, 2026

What problem does this PR solve?

Issue Number: close #68844, ref tikv/pd#10659

Problem Summary:

What changed and how does it work?

This PR updates dependencies (kvproto, PD client, client-go) and adapt to the latest incompatible changes to GC states API.

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

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

None

Summary by CodeRabbit

  • Chores

    • Bumped pinned Go module versions for upstream dependencies.
  • Tests

    • Updated test utilities and mocks to align with a revised GC-state API, including variadic options handling, explicit barrier retrieval, and stricter assertions to validate GC barrier behavior.

Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
@ti-chi-bot
Copy link
Copy Markdown

ti-chi-bot Bot commented Jun 1, 2026

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

@ti-chi-bot ti-chi-bot Bot added do-not-merge/needs-linked-issue do-not-merge/needs-tests-checked do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesn't merit a release note. labels Jun 1, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 1, 2026

Review Change Stack

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: 9e98f75c-7643-4ee8-a23c-39b246f22608

📥 Commits

Reviewing files that changed from the base of the PR and between 0fabd47 and f502701.

📒 Files selected for processing (1)
  • pkg/store/mockstore/unistore/tikv/mock_pd_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • pkg/store/mockstore/unistore/tikv/mock_pd_test.go

📝 Walkthrough

Walkthrough

Bump kvproto/client-go/pd deps and adapt mock GC client signatures and tests so GCStatesClient methods accept variadic pdgc.GCStatesAPIOption; mock implementations apply options to include/exclude GC barriers and tests use GetGCBarriers()/require checks.

Changes

GC API Options Adaptation

Layer / File(s) Summary
Dependency pins for GC API updates
go.mod, DEPS.bzl
Bump github.com/pingcap/kvproto, github.com/tikv/client-go/v2, and github.com/tikv/pd/client pins to newer revisions that introduce GCStatesAPIOption.
Mock GC client interface and implementation updates
dumpling/export/util_for_test.go, pkg/store/mockstore/unistore/tikv/mock_region.go
Update GetGCState and GetAllKeyspacesGCStates signatures to accept variadic pdgc.GCStatesAPIOption; mock_region applies options and returns GCState with or without barrier info via pdgc.NewGCState* constructors; dumpling test double signatures updated.
GC test adaptation to use new options API
pkg/store/mockstore/unistore/tikv/mock_pd_test.go, br/pkg/gc/mock_test.go
Tests call GetGCState with pdgc.ExcludeGCBarriers(false), fetch barriers via GetGCBarriers() with explicit error handling and require assertions, and assert HasGCBarriers()/barrier contents accordingly.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

"I hopped through deps and mocked states,
Tweaked signatures, kept tests in place,
Barriers fetched with care,
Options applied everywhere,
A rabbit claps for passing race." 🐰

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: updating three key dependencies (kvproto, pd client, client-go) and adapting to their GC API changes.
Description check ✅ Passed The description follows the template structure with Issue Number, Problem Summary, and What Changed sections completed; required sections are populated with relevant information about the dependency updates and API adaptations.
Docstring Coverage ✅ Passed Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ 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 and usage tips.

@ti-chi-bot ti-chi-bot Bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. component/dumpling This is related to Dumpling of TiDB. labels Jun 1, 2026
@tiprow
Copy link
Copy Markdown

tiprow Bot commented Jun 1, 2026

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>
Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
@MyonKeminta MyonKeminta marked this pull request as ready for review June 2, 2026 05:07
@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 Jun 2, 2026
@pantheon-ai
Copy link
Copy Markdown

pantheon-ai Bot commented Jun 2, 2026

@MyonKeminta I've received your pull request and will start the review. I'll conduct a thorough review covering code quality, potential issues, and implementation details.

⏳ This process typically takes 10-30 minutes depending on the complexity of the changes.

ℹ️ Learn more details on Pantheon AI.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/store/mockstore/unistore/tikv/mock_pd_test.go`:
- Around line 93-96: Add an assertion that checks the error returned by
s.GetGCBarriers() before using gcBarriers: after calling s.GetGCBarriers() in
the test block where gcBarriers and err are assigned, call re.NoError(err) so
any error is caught immediately and the assignment isn't silently unused; target
the GetGCBarriers() call and the gcBarriers/err variables in mock_pd_test.go
(the same pattern used at the other GetGCBarriers() sites).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 64d06462-b4a5-407a-88cf-b41e0fd83440

📥 Commits

Reviewing files that changed from the base of the PR and between a9add5c and 0722e3a.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (5)
  • DEPS.bzl
  • dumpling/export/util_for_test.go
  • go.mod
  • pkg/store/mockstore/unistore/tikv/mock_pd_test.go
  • pkg/store/mockstore/unistore/tikv/mock_region.go

Comment thread pkg/store/mockstore/unistore/tikv/mock_pd_test.go
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 2, 2026

Codecov Report

❌ Patch coverage is 44.44444% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.2588%. Comparing base (99e1c67) to head (f502701).
⚠️ Report is 12 commits behind head on master.

Additional details and impacted files
@@               Coverage Diff                @@
##             master     #68843        +/-   ##
================================================
- Coverage   76.3104%   76.2588%   -0.0517%     
================================================
  Files          2041       2044         +3     
  Lines        563452     572193      +8741     
================================================
+ Hits         429973     436348      +6375     
- Misses       132563     134528      +1965     
- Partials        916       1317       +401     
Flag Coverage Δ
integration 44.6697% <44.4444%> (+4.8912%) ⬆️

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

Components Coverage Δ
dumpling 60.4610% <ø> (ø)
parser ∅ <ø> (∅)
br 65.8816% <ø> (+3.0506%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ti-chi-bot ti-chi-bot Bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Jun 2, 2026
@ti-chi-bot ti-chi-bot Bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jun 2, 2026
@ti-chi-bot
Copy link
Copy Markdown

ti-chi-bot Bot commented Jun 2, 2026

[LGTM Timeline notifier]

Timeline:

  • 2026-06-02 05:30:29.530109854 +0000 UTC m=+246730.600427244: ☑️ agreed by zyguan.
  • 2026-06-02 05:30:53.283743928 +0000 UTC m=+246754.354061318: ☑️ agreed by ekexium.

Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
@ti-chi-bot ti-chi-bot Bot removed the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jun 2, 2026
@ti-chi-bot ti-chi-bot Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jun 2, 2026
Copy link
Copy Markdown
Contributor

@3pointer 3pointer left a comment

Choose a reason for hiding this comment

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

BR part LGTM

@ti-chi-bot
Copy link
Copy Markdown

ti-chi-bot Bot commented Jun 2, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 3pointer, cfzjywxk, D3Hunter, ekexium, zyguan

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 the approved label Jun 2, 2026
Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
@MyonKeminta MyonKeminta force-pushed the m/update-gc-state-api branch from 0fabd47 to f502701 Compare June 2, 2026 06:05
@MyonKeminta
Copy link
Copy Markdown
Contributor Author

/retest

@tiprow
Copy link
Copy Markdown

tiprow Bot commented Jun 2, 2026

@MyonKeminta: 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

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
Copy link
Copy Markdown
Contributor

cfzjywxk commented Jun 2, 2026

/ok-to-test

@ti-chi-bot ti-chi-bot Bot added the ok-to-test Indicates a PR is ready to be tested. label Jun 2, 2026
@MyonKeminta
Copy link
Copy Markdown
Contributor Author

/retest

@ti-chi-bot ti-chi-bot Bot merged commit 9f09310 into pingcap:master Jun 2, 2026
44 checks passed
@MyonKeminta MyonKeminta deleted the m/update-gc-state-api branch June 2, 2026 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved component/dumpling This is related to Dumpling of TiDB. lgtm ok-to-test Indicates a PR is ready to be tested. release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Adapt to incompatible changes to GC API.

6 participants