Support service mode switching for tso and change the value type of tso primary to tsopb.Participant #6160

binshi-bing · 2023-03-14T03:25:37Z

Support service mode switching for and change the value type of tso primary

What problem does this PR solve?

Issue Number: Ref #5836

What is changed and how does it work?

Support tso service switched between PD mode and API mode;
Add server/client implemenation for the GetClusterInfo rpc
Add tsopb.Participant as the value type of tso primary

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
Performance test

Conclusion: no performance regression.

w/ change
Total:
count: 28949005, max: 6.1310ms, min: 0.0410ms, avg: 2.0541ms
<1ms: 479588, >1ms: 11500091, >2ms: 16969184, >5ms: 142, >10ms: 0, >30ms: 0, >50ms: 0, >100ms: 0, >200ms: 0, >400ms: 0, >800ms: 0, >1s: 0
count: 28949005, <1ms: 1.66%, >1ms: 39.73%, >2ms: 58.62%, >5ms: 0.00%, >10ms: 0.00%, >30ms: 0.00%, >50ms: 0.00%, >100ms: 0.00%, >200ms: 0.00%, >400ms: 0.00%, >800ms: 0.00%, >1s: 0.00%
P0.5: 2.0470ms, P0.8: 2.2553ms, P0.9: 2.4288ms, P0.99: 3.1310ms

w/o change
Total:
count: 28181983, max: 9.9771ms, min: 0.0616ms, avg: 2.0791ms
<1ms: 367685, >1ms: 10210926, >2ms: 17602085, >5ms: 1287, >10ms: 0, >30ms: 0, >50ms: 0, >100ms: 0, >200ms: 0, >400ms: 0, >800ms: 0, >1s: 0
count: 28181983, <1ms: 1.30%, >1ms: 36.23%, >2ms: 62.46%, >5ms: 0.00%, >10ms: 0.00%, >30ms: 0.00%, >50ms: 0.00%, >100ms: 0.00%, >200ms: 0.00%, >400ms: 0.00%, >800ms: 0.00%, >1s: 0.00%
P0.5: 2.0682ms, P0.8: 2.2742ms, P0.9: 2.4449ms, P0.99: 3.1436ms

Release note

None.

ti-chi-bot · 2023-03-14T03:25:39Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

lhy1024
rleungx

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 · 2023-03-14T03:25:46Z

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.

client/pd_service_discovery.go

pkg/mcs/meta_storage/server/grpc_service.go

pkg/mcs/tso/server/server.go

lhy1024 · 2023-03-15T03:21:01Z

/ok-to-test

client/client.go

client/pd_service_discovery.go

codecov · 2023-03-15T23:40:35Z

Codecov Report

Patch coverage: 76.21% and project coverage change: -0.06 ⚠️

Comparison is base (6eb4410) 74.54% compared to head (2f5422f) 74.48%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6160      +/-   ##
==========================================
- Coverage   74.54%   74.48%   -0.06%     
==========================================
  Files         393      393              
  Lines       38527    38627     +100     
==========================================
+ Hits        28719    28772      +53     
- Misses       7268     7300      +32     
- Partials     2540     2555      +15

Flag	Coverage Δ
unittests	`74.48% <76.21%> (-0.06%)`	⬇️

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

Impacted Files	Coverage Δ
client/errs/errno.go	`100.00% <ø> (ø)`
pkg/mcs/meta_storage/server/grpc_service.go	`59.61% <0.00%> (ø)`
pkg/mcs/resource_manager/server/grpc_service.go	`67.77% <0.00%> (ø)`
pkg/storage/endpoint/key_path.go	`93.33% <ø> (ø)`
server/api/server.go	`100.00% <ø> (ø)`
client/tso_stream.go	`73.49% <31.25%> (-5.38%)`	⬇️
client/keyspace_client.go	`63.63% <33.33%> (ø)`
client/meta_storage_client.go	`68.47% <50.00%> (ø)`
client/resource_manager_client.go	`70.29% <50.00%> (ø)`
server/server.go	`74.33% <62.50%> (-0.58%)`	⬇️
... and 14 more

... and 17 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.

rleungx · 2023-03-17T03:19:50Z

CI failed.

client/go.mod

… as the value type of tso primary Support tso service switched between PD mode and API mode Add server/client implemenation for the GetClusterInfo rpc and support service mode in the client Signed-off-by: Bin Shi <binshi.bing@gmail.com>

Signed-off-by: Bin Shi <binshi.bing@gmail.com>

… keep the current mode instead of swtiching to unknown mode. Signed-off-by: Bin Shi <binshi.bing@gmail.com>

Signed-off-by: Bin Shi <binshi.bing@gmail.com>

Fixed the following cases: 1. Timeout issue. Previously, after starting the API service, the test suite tried to create the PD client before starting TSO servers. PD client will try to connect to TSO service with 30 retries and 30s in total. The fix is to change the retry time, overriden by the option passed to NewClientWithContext, to 1. 2. GetTS Failure. Previously, after staring TSO server, the test immediate sent GetTS() requests, but it won't succeed until the TSO servers complete the primary election and the client refresh the primary address. The fix is to add WaitForPrimaryServing() and WaitForTSOServiceAvailable() and to catch the io.EOF for stream.send()/recv() and refresh primary address immediately. 3. The test suite is too heavy. Turn SetupTest/TeardownTest to SetupSuite/TearDownSuite to avoid duplicate setups of the cluster. After the above fixes, the test suite needs more than 50s and will timeout in vscode but ok when running from commandline. Signed-off-by: Bin Shi <binshi.bing@gmail.com>

Signed-off-by: Bin Shi <binshi.bing@gmail.com>

lhy1024

The rest lgtm

pkg/mcs/resource_manager/server/server.go

pkg/mcs/tso/server/server.go

server/server.go

client/client.go

Signed-off-by: Bin Shi <binshi.bing@gmail.com>

JmPotato · 2023-03-20T05:17:01Z

/merge

ti-chi-bot · 2023-03-20T05:17:02Z

@JmPotato: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

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 · 2023-03-20T05:17:04Z

This pull request has been accepted and is ready to merge.

Commit hash: 2f5422f

ti-chi-bot added release-note-none needs-ok-to-test labels Mar 14, 2023

ti-chi-bot requested review from disksing and JmPotato March 14, 2023 03:25

binshi-bing changed the title ~~Support service mode switching for and change the value type of tso primary~~ Support service mode switching for tso and change the value type of tso primary Mar 14, 2023

binshi-bing changed the title ~~Support service mode switching for tso and change the value type of tso primary~~ Support service mode switching for tso and change the value type of tso primary to tsopb.Participant Mar 14, 2023

binshi-bing force-pushed the service-mode-switching branch 2 times, most recently from 3c35e9e to 1d4759b Compare March 14, 2023 05:02

ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 14, 2023

binshi-bing force-pushed the service-mode-switching branch from 1d4759b to 2fa8284 Compare March 14, 2023 05:21

rleungx reviewed Mar 14, 2023

View reviewed changes

client/pd_service_discovery.go Outdated Show resolved Hide resolved

client/pd_service_discovery.go Outdated Show resolved Hide resolved

pkg/mcs/meta_storage/server/grpc_service.go Outdated Show resolved Hide resolved

pkg/mcs/tso/server/server.go Outdated Show resolved Hide resolved

binshi-bing force-pushed the service-mode-switching branch from 01a04b6 to b9d078e Compare March 14, 2023 20:18

ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 14, 2023

ti-chi-bot added ok-to-test and removed needs-ok-to-test labels Mar 15, 2023

rleungx reviewed Mar 15, 2023

View reviewed changes

client/client.go Outdated Show resolved Hide resolved

client/client.go Outdated Show resolved Hide resolved

rleungx reviewed Mar 15, 2023

View reviewed changes

client/pd_service_discovery.go Outdated Show resolved Hide resolved

binshi-bing force-pushed the service-mode-switching branch from 1aa294d to 4a55a1f Compare March 15, 2023 23:28

rleungx approved these changes Mar 16, 2023

View reviewed changes

ti-chi-bot added status/LGT1 Indicates that a PR has LGTM 1. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Mar 16, 2023

binshi-bing force-pushed the service-mode-switching branch from a19a401 to d876055 Compare March 16, 2023 17:24

ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 16, 2023

binshi-bing force-pushed the service-mode-switching branch from d876055 to 852f666 Compare March 16, 2023 17:47

lhy1024 reviewed Mar 17, 2023

View reviewed changes

client/go.mod Outdated Show resolved Hide resolved

ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 20, 2023

binshi-bing added 13 commits March 19, 2023 21:24

reimplement the client logic

efd21d8

Signed-off-by: Bin Shi <binshi.bing@gmail.com>

fix a bug which occured when switching service mode from api to pd

e76396c

Signed-off-by: Bin Shi <binshi.bing@gmail.com>

handle feedback

d99087d

Signed-off-by: Bin Shi <binshi.bing@gmail.com>

Use atomic.Value instead of read lock of RWMutext to guard tsoClient

c2dcb9b

Signed-off-by: Bin Shi <binshi.bing@gmail.com>

Handle feedback -- if the client fails to switch to new service mode,…

8b47aa2

… keep the current mode instead of swtiching to unknown mode. Signed-off-by: Bin Shi <binshi.bing@gmail.com>

handle more feedback

79950c2

Signed-off-by: Bin Shi <binshi.bing@gmail.com>

fix test failure

a21f849

Signed-off-by: Bin Shi <binshi.bing@gmail.com>

rebase master

79e743f

Signed-off-by: Bin Shi <binshi.bing@gmail.com>

go get github.com/pingcap/kvproto@master

bda035f

Signed-off-by: Bin Shi <binshi.bing@gmail.com>

Handle feedback

b87de96

Signed-off-by: Bin Shi <binshi.bing@gmail.com>

Handle more Hanyang's feedback

6478b39

Signed-off-by: Bin Shi <binshi.bing@gmail.com>

binshi-bing force-pushed the service-mode-switching branch from af5df68 to 57b2e40 Compare March 20, 2023 04:29

ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 20, 2023

lhy1024 reviewed Mar 20, 2023

View reviewed changes

pkg/mcs/resource_manager/server/server.go Outdated Show resolved Hide resolved

pkg/mcs/tso/server/server.go Show resolved Hide resolved

server/server.go Show resolved Hide resolved

client/client.go Show resolved Hide resolved

rebase master

2f5422f

Signed-off-by: Bin Shi <binshi.bing@gmail.com>

binshi-bing force-pushed the service-mode-switching branch from 57b2e40 to 2f5422f Compare March 20, 2023 04:57

lhy1024 approved these changes Mar 20, 2023

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Mar 20, 2023

JmPotato approved these changes Mar 20, 2023

View reviewed changes

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 20, 2023

ti-chi-bot merged commit 8d1d091 into tikv:master Mar 20, 2023

rleungx mentioned this pull request Mar 29, 2023

Failed to create PD client for PD v6.5/v6.6 #6243

Closed

lance6716 mentioned this pull request Aug 31, 2023

client: backport ErrClientTSOStreamClosed #7021

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support service mode switching for tso and change the value type of tso primary to tsopb.Participant #6160

Support service mode switching for tso and change the value type of tso primary to tsopb.Participant #6160

binshi-bing commented Mar 14, 2023 •

edited

Loading

ti-chi-bot commented Mar 14, 2023 •

edited

Loading

ti-chi-bot commented Mar 14, 2023

lhy1024 commented Mar 15, 2023

codecov bot commented Mar 15, 2023 •

edited

Loading

rleungx commented Mar 17, 2023

lhy1024 left a comment

JmPotato commented Mar 20, 2023

ti-chi-bot commented Mar 20, 2023

ti-chi-bot commented Mar 20, 2023

Support service mode switching for tso and change the value type of tso primary to tsopb.Participant #6160

Support service mode switching for tso and change the value type of tso primary to tsopb.Participant #6160

Conversation

binshi-bing commented Mar 14, 2023 • edited Loading

What problem does this PR solve?

What is changed and how does it work?

Check List

Release note

ti-chi-bot commented Mar 14, 2023 • edited Loading

ti-chi-bot commented Mar 14, 2023

lhy1024 commented Mar 15, 2023

codecov bot commented Mar 15, 2023 • edited Loading

Codecov Report

rleungx commented Mar 17, 2023

lhy1024 left a comment

Choose a reason for hiding this comment

JmPotato commented Mar 20, 2023

ti-chi-bot commented Mar 20, 2023

ti-chi-bot commented Mar 20, 2023

binshi-bing commented Mar 14, 2023 •

edited

Loading

ti-chi-bot commented Mar 14, 2023 •

edited

Loading

codecov bot commented Mar 15, 2023 •

edited

Loading