Skip to content
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.

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

basic implement tso gPRC service #5949

Closed
wants to merge 6 commits into from

Conversation

binshi-bing
Copy link
Contributor

@binshi-bing binshi-bing commented Feb 9, 2023

Signed-off-by: Bin Shi [email protected]

What problem does this PR solve?

Issue Number: Ref #5836

What is changed and how does it work?

Implement TSO gRPC service. We refactor and move tso rpcs' implementation from pd/server to pkg/tso so that both pd server and the independent TSO mcs can share the same tso logic layer and hide the internal data structures and logics from the upper layer. By doing so, we can adding multi-tenant and sharding while keep grpc layer stable.

1. Basic implement TSO gRPC service
2. Update kvproto to reuse pdpb tso messages and add GetMembers() rpc. Adjust to the following two main changes in the gPRC protocol:
    a. Add keyspace id and keyspace_group_id to RequestHeader/Response Header to support multi-tenant.
    b. Define new XXXRequest/XXXResponse messages by reusing the messages in pdpb for better back-compatibility. The tenant tso protocol can also evolve independently by adding new rpcs and new messages.
3.  Implement rpcs GetMembers() and Tso()
4. Implement rpcs SyncMaxTS() and GetDCLocationInfo()
5. Implement rpcs SetExternalTimestamp() and GetExternalTimestamp() 

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

@ti-chi-bot
Copy link
Member

[REVIEW NOTIFICATION]

This pull request has not been approved.

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

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

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

@ti-chi-bot ti-chi-bot added do-not-merge/needs-linked-issue release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Feb 9, 2023
@binshi-bing binshi-bing marked this pull request as draft February 9, 2023 04:38
@ti-chi-bot ti-chi-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-ok-to-test Indicates a PR created by contributors and need ORG member send '/ok-to-test' to start testing. labels Feb 9, 2023
@ti-chi-bot
Copy link
Member

Hi @binshi-bing. Thanks for your PR.

I'm waiting for a tikv member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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/test-infra repository.

@ti-chi-bot ti-chi-bot requested review from lhy1024 and rleungx February 9, 2023 04:38
@ti-chi-bot ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/needs-linked-issue release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Feb 9, 2023
@binshi-bing binshi-bing changed the title implement tso service basic implement tso service Feb 9, 2023
@binshi-bing binshi-bing force-pushed the basic-implement-tso-svr branch from 7a38347 to dd860c4 Compare February 9, 2023 07:32
@binshi-bing binshi-bing changed the title basic implement tso service basic implement tso gPRC service Feb 9, 2023
@binshi-bing binshi-bing force-pushed the basic-implement-tso-svr branch 3 times, most recently from 84bb7b3 to e74aa09 Compare February 11, 2023 00:07
@ti-chi-bot ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 11, 2023
@binshi-bing binshi-bing force-pushed the basic-implement-tso-svr branch from e74aa09 to a0f7ba8 Compare February 11, 2023 02:08
@ti-chi-bot ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 11, 2023
@binshi-bing binshi-bing force-pushed the basic-implement-tso-svr branch 2 times, most recently from 5322919 to c1ffc53 Compare February 11, 2023 17:11
@codecov
Copy link

codecov bot commented Feb 11, 2023

Codecov Report

Patch coverage: 65.73% and project coverage change: +0.10 🎉

Comparison is base (e6086c4) 75.13% compared to head (764b7ec) 75.24%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5949      +/-   ##
==========================================
+ Coverage   75.13%   75.24%   +0.10%     
==========================================
  Files         362      363       +1     
  Lines       36282    36302      +20     
==========================================
+ Hits        27262    27316      +54     
+ Misses       6645     6618      -27     
+ Partials     2375     2368       -7     
Flag Coverage Δ
unittests 75.24% <65.73%> (+0.10%) ⬆️

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

Impacted Files Coverage Δ
server/keyspace_service.go 61.19% <0.00%> (ø)
server/metrics.go 100.00% <ø> (ø)
server/util.go 46.37% <ø> (-5.31%) ⬇️
server/grpc_service.go 45.42% <57.14%> (-3.74%) ⬇️
pkg/tso/grpc_server.go 65.65% <65.65%> (ø)
server/server.go 74.73% <66.66%> (-1.04%) ⬇️
pkg/utils/etcdutil/etcdutil.go 82.17% <69.23%> (-3.27%) ⬇️
pkg/tso/metrics.go 100.00% <100.00%> (ø)
pkg/utils/grpcutil/grpcutil.go 70.68% <100.00%> (+3.38%) ⬆️

... and 19 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@binshi-bing binshi-bing force-pushed the basic-implement-tso-svr branch 3 times, most recently from 37f6c44 to 5d8c8f1 Compare February 12, 2023 01:10
@binshi-bing binshi-bing marked this pull request as ready for review February 12, 2023 01:12
@ti-chi-bot ti-chi-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 12, 2023
Basic implement the indepent (Tenant) TSO gPRC service.
The kvproto pr is here pingcap/kvproto#1053

Signed-off-by: Bin Shi <[email protected]>
Adjust to the following two main changes in the gPRC protocol:
1.Add keyspace id and keyspace_group_id to RequestHeader/Response Header to support multi-tenant.
2.Define new XXXRequest/XXXResponse messages by reusing the messages in pdpb for better back-compatibility. The tenant tso protocol can also evolve independently by adding new rpcs and new messages.

Signed-off-by: Bin Shi <[email protected]>
rebase master

Signed-off-by: Bin Shi <[email protected]>
@binshi-bing binshi-bing force-pushed the basic-implement-tso-svr branch from 98f15f5 to 764b7ec Compare February 13, 2023 17:12
binshi-bing added a commit to binshi-bing/pd that referenced this pull request Feb 13, 2023
This change is split from "basic implement tso gPRC service tikv#5949" tikv#5949

Signed-off-by: Bin Shi <[email protected]>
binshi-bing added a commit to binshi-bing/pd that referenced this pull request Feb 13, 2023
This change is split from "basic implement tso gPRC service tikv#5949" tikv#5949

Signed-off-by: Bin Shi <[email protected]>
binshi-bing added a commit to binshi-bing/pd that referenced this pull request Feb 13, 2023
This change is split from "basic implement tso gPRC service tikv#5949" tikv#5949

Signed-off-by: Bin Shi <[email protected]>
binshi-bing added a commit to binshi-bing/pd that referenced this pull request Feb 13, 2023
This change is split from "basic implement tso gPRC service tikv#5949" tikv#5949

Signed-off-by: Bin Shi <[email protected]>
binshi-bing added a commit to binshi-bing/pd that referenced this pull request Feb 13, 2023
This change is split from "basic implement tso gPRC service tikv#5949" tikv#5949

Signed-off-by: Bin Shi <[email protected]>
binshi-bing added a commit to binshi-bing/pd that referenced this pull request Feb 13, 2023
This change is split from "basic implement tso gPRC service tikv#5949" tikv#5949

Signed-off-by: Bin Shi <[email protected]>
binshi-bing added a commit to binshi-bing/pd that referenced this pull request Feb 14, 2023
This change is split from "basic implement tso gPRC service tikv#5949" tikv#5949

Signed-off-by: Bin Shi <[email protected]>
binshi-bing added a commit to binshi-bing/pd that referenced this pull request Feb 14, 2023
This change is split from "basic implement tso gPRC service tikv#5949" tikv#5949

Signed-off-by: Bin Shi <[email protected]>
ti-chi-bot pushed a commit that referenced this pull request Feb 14, 2023
… microservice change (#5985)

ref #5836, ref #5949

Refactor some common utilities which will be used by tso mcs

Signed-off-by: Bin Shi <[email protected]>
@binshi-bing binshi-bing marked this pull request as draft February 14, 2023 06:47
@ti-chi-bot ti-chi-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Feb 14, 2023
@ti-chi-bot
Copy link
Member

@binshi-bing: PR needs rebase.

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/test-infra repository.

ti-chi-bot added a commit that referenced this pull request Feb 16, 2023
…n a basic manner (#5986)

close #5836, ref #5949

Implement the server side of the tso grpc service.

Signed-off-by: Bin Shi <[email protected]>

Co-authored-by: Ti Chi Robot <[email protected]>
nolouch pushed a commit to nolouch/pd that referenced this pull request Feb 24, 2023
… microservice change (tikv#5985)

ref tikv#5836, ref tikv#5949

Refactor some common utilities which will be used by tso mcs

Signed-off-by: Bin Shi <[email protected]>
@rleungx
Copy link
Member

rleungx commented Apr 13, 2023

Shall we close it now?

@binshi-bing
Copy link
Contributor Author

Broken down into separate prs.

@binshi-bing
Copy link
Contributor Author

Shall we close it now?

closed

@binshi-bing binshi-bing deleted the basic-implement-tso-svr branch May 25, 2023 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-ok-to-test Indicates a PR created by contributors and need ORG member send '/ok-to-test' to start testing. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. release-note-none Denotes a PR that doesn't merit a release note.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants