Skip to content

client: Update PD Client interface and supports the new GC API #9203

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
wants to merge 86 commits into
base: master
Choose a base branch
from
Draft

client: Update PD Client interface and supports the new GC API #9203

wants to merge 86 commits into from

Conversation

MyonKeminta
Copy link
Contributor

@MyonKeminta MyonKeminta commented Apr 15, 2025 •
edited
Loading

What problem does this PR solve?

Issue Number: Close #8978

What is changed and how does it work?

client: Update PD Client interface and supports the new GC API

This is part of the GC Refactor work #8978 .

Changes in this PR are made based on #9189 . For diff between this and #9189 , see: MyonKeminta/pd@m/new-gc-grpc-api...m/new-gc-api-client

Check List

Tests

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

Code changes

Side effects

  • Possible performance regression
  • Increased code complexity
  • Breaking backward compatibility

Related changes

Release note

None.

@MyonKeminta
Add chagnes to the path definition
c70bd8e 
Signed-off-by: MyonKeminta <[email protected]>
@MyonKeminta
Add gc_states.go and gc_states_test.go
84b09c4 
Signed-off-by: MyonKeminta <[email protected]>
@MyonKeminta
fix test
70c0ce9 
Signed-off-by: MyonKeminta <[email protected]>
@MyonKeminta
Add more comments
ba36b14 
Signed-off-by: MyonKeminta <[email protected]>
@MyonKeminta
fix lint
26fc18c 
Signed-off-by: MyonKeminta <[email protected]>
@MyonKeminta
Add more comments
2622255 
Signed-off-by: MyonKeminta <[email protected]>
@MyonKeminta
Remove unnecessary commented code
db9fa7b 
Signed-off-by: MyonKeminta <[email protected]>
@MyonKeminta
Add more comments
364cf6d 
Signed-off-by: MyonKeminta <[email protected]>
@MyonKeminta
Address comments
c5653a2 
Signed-off-by: MyonKeminta <[email protected]>
@MyonKeminta
Update file headers
2f0c663 
Signed-off-by: MyonKeminta <[email protected]>
@MyonKeminta
Renaming
c1731dd 
Signed-off-by: MyonKeminta <[email protected]>
@MyonKeminta
Merge path functions
d9b944f 
Signed-off-by: MyonKeminta <[email protected]>
@MyonKeminta
rename keypath/safe_point.go to gc_states.go
f1647bf 
Signed-off-by: MyonKeminta <[email protected]>
@MyonKeminta
Merge branch 'master' of https://github.com/tikv/pd into m/gc-state-s…
b913e1e 
…torage
@MyonKeminta
Add comments explaining keyspace-level GC and unified GC
11a002e 
Signed-off-by: MyonKeminta <[email protected]>
@MyonKeminta
Address comments
8e4bd77 
Signed-off-by: MyonKeminta <[email protected]>
@MyonKeminta
Address comments
0a5acfd 
Signed-off-by: MyonKeminta <[email protected]>
@MyonKeminta
fix lint
42cae00 
Signed-off-by: MyonKeminta <[email protected]>
@MyonKeminta
Add related files
632a99d 
Signed-off-by: MyonKeminta <[email protected]>
@MyonKeminta
update kvproto
66d462b 
Signed-off-by: MyonKeminta <[email protected]>
@MyonKeminta
move code
ed770a8 
Signed-off-by: MyonKeminta <[email protected]>
@MyonKeminta
switch implementations
d96539b 
Signed-off-by: MyonKeminta <[email protected]>
@MyonKeminta
abandon GC API V2
1c72c88 
Signed-off-by: MyonKeminta <[email protected]>
@MyonKeminta
Address comments
397a53f 
Signed-off-by: MyonKeminta <[email protected]>
@MyonKeminta
Add test for GCBarrer.IsExpired
6445fa4 
Signed-off-by: MyonKeminta <[email protected]>
@MyonKeminta
Merge branch 'master' of https://github.com/tikv/pd into m/gc-state-s…
106639f 
…torage
@MyonKeminta
Merge branch 'm/gc-state-storage' into m/gc-state-manager
236ed1c
@MyonKeminta
Merge commit 'c1c681a' into m/gc-state-manager
11080fd
@MyonKeminta
fix lint
1530f53 
Signed-off-by: MyonKeminta <[email protected]>
@MyonKeminta
address comments from tikv#9109
8b9c144 
Signed-off-by: MyonKeminta <[email protected]>
@ti-chi-bot ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Apr 15, 2025

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/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. dco-signoff: yes Indicates the PR's author has signed the dco. labels Apr 15, 2025
@ti-chi-bot ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Apr 15, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign hundundm for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

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 size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Apr 15, 2025
@MyonKeminta MyonKeminta mentioned this pull request Apr 10, 2025
Open
17 tasks
@MyonKeminta MyonKeminta force-pushed the m/new-gc-api-client branch 3 times, most recently from f23e828 to b0031c4 Compare April 17, 2025 03:06
@MyonKeminta
Copy link
Contributor Author

/retest

1 similar comment
@MyonKeminta
Copy link
Contributor Author

/retest

@MyonKeminta MyonKeminta force-pushed the m/new-gc-api-client branch 2 times, most recently from 68c5cb7 to 8874b07 Compare April 17, 2025 09:50
@MyonKeminta
Fix etcd key test
74a5a32 
Signed-off-by: MyonKeminta <[email protected]>
@MyonKeminta MyonKeminta force-pushed the m/new-gc-api-client branch from 8874b07 to 74a5a32 Compare April 17, 2025 09:58
This was referenced Apr 21, 2025
Draft
Draft
tiancaiamao
tiancaiamao reviewed Apr 22, 2025
View reviewed changes
server/server.go Outdated
@@ -469,7 +469,7 @@ func (s *Server) startServer(ctx context.Context) error {
s.tsoProtoFactory = &tsoutil.TSOProtoFactory{}
s.pdProtoFactory = &tsoutil.PDProtoFactory{}
s.tsoAllocator = tso.NewAllocator(s.ctx, constant.DefaultKeyspaceGroupID, s.member, s.storage, s)
s.gcSafePointManager = gc.NewSafePointManager(s.storage, s.cfg.PDServerCfg)
s.gcStateManager = gc.NewGCStateManager(s.storage.GetGCStateProvider(), s.cfg.PDServerCfg, s.keyspaceManager)
Copy link
Contributor

Choose a reason for hiding this comment

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

s.keyspaceManager is not initialized yet here, please move this line to after line 485, otherwise when use the AdvenaceTxnSafePoint, it get a nil pointer panic. @MyonKeminta

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😰sorry

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tiancaiamao tiancaiamao tiancaiamao left review comments

Assignees
No one assigned
Labels
dco-signoff: yes Indicates the PR's author has signed the dco. 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. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Refactor GC related PD APIs
2 participants
@MyonKeminta @tiancaiamao