client: support backoff mechanism for memberLoop #6978

HuSharp · 2023-08-23T09:45:52Z

Signed-off-by: husharp [email protected]<!--

Thank you for working on PD! Please read PD's CONTRIBUTING document BEFORE filing this PR.

PR Title Format:

pkg [, pkg2, pkg3]: what's changed
*: what's changed

-->

What problem does this PR solve?

Issue Number: ref #6556

What is changed and how does it work?

Check List

Tests

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

PR Summary

Add ready for resp

Have goroutine reconnectMemberLoop call updateMember periodically. When calling ScheduleCheckMemberChanged channel, we need to wait for the goroutine to update members until ready or timeout.

Add backoff mechanism

When waiting for the goroutine to update, the expo function can be used to backoff to sleep when an error is encountered.

Reproduce Step

enable fail point, like gRPC is throttling, cannot read from etcd.
curl -X PUT -d 'return(10)' http://tc-pd-1.tc-pd-peer.csn-simulator-big-cluster-vd62g.svc:2379/pd/api/v1/fail/github.com/tikv/pd/pkg/etcdutil/SlowEtcdKVGet
simulate pd lost leader
curl -X PUT -d 'return("2346857576170797299")' http://tc-pd-1.tc-pd-peer.csn-simulator-big-cluster-vd62g.svc:2379/pd/api/v1/fail/github.com/tikv/pd/server/exitCampaignLeader

Reproduce Result

Grpc request GetMember keeps high:

TiKV side show

PR Effect

The Grpc GetMember call was reduced from 3.2k to 170, which is relative to the TiDB numbers and client requests for triaging checkLeader.

For 20 * tidb 3 * PD 50 * TiKV
170 = (50 * 3 / 3 / 3[TiKV side] + 20 * 2 [TiDB side]) * 3[PD Num]

And more tests are necessary to ensure that no further issues arise.

Release note

None.

Signed-off-by: husharp <[email protected]>

ti-chi-bot · 2023-08-23T09:45:55Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

JmPotato
nolouch

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-08-23T09:45:55Z

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

codecov · 2023-08-24T09:22:54Z

Codecov Report

Patch coverage: 96.87% and project coverage change: -0.12% ⚠️

Comparison is base (75bb796) 75.75% compared to head (7fc588f) 75.64%.

Additional details and impacted files

@@               Coverage Diff               @@
##           release-6.5    #6978      +/-   ##
===============================================
- Coverage        75.75%   75.64%   -0.12%     
===============================================
  Files              329      330       +1     
  Lines            33600    33627      +27     
===============================================
- Hits             25453    25436      -17     
- Misses            5988     6015      +27     
- Partials          2159     2176      +17

Flag	Coverage Δ
unittests	`75.64% <96.87%> (-0.12%)`	⬇️

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

Files Changed	Coverage Δ
client/keyspace_client.go	`62.22% <ø> (ø)`
client/base_client.go	`83.09% <85.71%> (+0.32%)`	⬆️
client/client.go	`67.49% <100.00%> (+0.03%)`	⬆️
client/retry/backoff.go	`100.00% <100.00%> (ø)`
server/server.go	`74.37% <100.00%> (-1.32%)`	⬇️

... and 18 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

client/retry/backoff.go

client/base_client.go

Signed-off-by: husharp <[email protected]>

client/base_client.go

client/retry/backoff.go

Signed-off-by: husharp <[email protected]>

client/base_client.go

client/retry/backoff.go

Signed-off-by: husharp <[email protected]>

nolouch · 2023-08-29T05:47:57Z

client/retry/backoff.go

+}
+
+// Only used for test.
+var testBackOffExecuteFlag = false


can it be removed?

want to make sure back off executed

client/base_client.go

Signed-off-by: husharp <[email protected]>

HuSharp · 2023-08-29T07:25:12Z

tests/pdctl/scheduler/scheduler_test.go

-	mustExec([]string{"-u", pdAddr, "scheduler", "config", "balance-hot-region-scheduler"}, &conf1)
 	// After upgrading, we should not use query.
-	expected1["read-priorities"] = []interface{}{"query", "byte"}
-	re.NotEqual(expected1, conf1)
-	expected1["read-priorities"] = []interface{}{"key", "byte"}
-	re.Equal(expected1, conf1)
+	mustExec([]string{"-u", pdAddr, "scheduler", "config", "balance-hot-region-scheduler"}, &conf1)
+	re.Equal(conf1["read-priorities"], []interface{}{"key", "byte"})
 	// cannot set qps as write-peer-priorities
 	echo = mustExec([]string{"-u", pdAddr, "scheduler", "config", "balance-hot-region-scheduler", "set", "write-peer-priorities", "query,byte"}, nil)
 	re.Contains(echo, "query is not allowed to be set in priorities for write-peer-priorities")
 	mustExec([]string{"-u", pdAddr, "scheduler", "config", "balance-hot-region-scheduler"}, &conf1)
-	re.Equal(expected1, conf1)
+	re.Equal(conf1["write-peer-priorities"], []interface{}{"byte", "key"})

 	// test remove and add
-	mustExec([]string{"-u", pdAddr, "scheduler", "remove", "balance-hot-region-scheduler"}, nil)
-	mustExec([]string{"-u", pdAddr, "scheduler", "add", "balance-hot-region-scheduler"}, nil)
-	re.Equal(expected1, conf1)
+	echo = mustExec([]string{"-u", pdAddr, "scheduler", "remove", "balance-hot-region-scheduler"}, nil)
+	re.Contains(echo, "Success")
+	echo = mustExec([]string{"-u", pdAddr, "scheduler", "add", "balance-hot-region-scheduler"}, nil)
+	re.Contains(echo, "Success")


cp #5847 to make TestScheduler stable

Signed-off-by: husharp <[email protected]>

nolouch · 2023-08-29T11:10:59Z

/merge

ti-chi-bot · 2023-08-29T11:11:01Z

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

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

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-08-29T11:11:03Z

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

Commit hash: 7fc588f

HuSharp added 2 commits April 11, 2023 16:52

abondon sync

9975663

Signed-off-by: husharp <[email protected]>

Merge remote-tracking branch 'upstream/release-6.5' into release-6.5

3199dfe

ti-chi-bot bot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/cherry-pick-not-approved do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Aug 23, 2023

ti-chi-bot bot requested review from JmPotato and nolouch August 23, 2023 09:46

ti-chi-bot bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Aug 23, 2023

HuSharp force-pushed the release-6.5-add_ready branch 5 times, most recently from 9ad1517 to f4541bf Compare August 24, 2023 08:43

ti-chi-bot bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Aug 24, 2023

HuSharp force-pushed the release-6.5-add_ready branch from f4541bf to 2df1ffd Compare August 24, 2023 09:12

HuSharp marked this pull request as ready for review August 24, 2023 09:16

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 Aug 24, 2023

HuSharp requested a review from rleungx August 24, 2023 09:16

nolouch reviewed Aug 24, 2023

View reviewed changes

client/retry/backoff.go Outdated Show resolved Hide resolved

nolouch reviewed Aug 24, 2023

View reviewed changes

client/base_client.go Outdated Show resolved Hide resolved

HuSharp force-pushed the release-6.5-add_ready branch from 2df1ffd to 9cd68d6 Compare August 25, 2023 00:39

ti-chi-bot bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Aug 25, 2023

HuSharp force-pushed the release-6.5-add_ready branch from 9cd68d6 to 870664c Compare August 25, 2023 01:20

ti-chi-bot bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 25, 2023

HuSharp force-pushed the release-6.5-add_ready branch from 3970df8 to 850e3e9 Compare August 28, 2023 11:01

ti-chi-bot added the cherry-pick-approved Cherry pick PR approved by release team. label Aug 28, 2023

ti-chi-bot bot removed the do-not-merge/cherry-pick-not-approved label Aug 28, 2023

refine backoff

23318b6

Signed-off-by: husharp <[email protected]>

HuSharp force-pushed the release-6.5-add_ready branch from 850e3e9 to 23318b6 Compare August 28, 2023 11:07

rleungx reviewed Aug 29, 2023

View reviewed changes

client/base_client.go Outdated Show resolved Hide resolved

client/retry/backoff.go Outdated Show resolved Hide resolved

HuSharp force-pushed the release-6.5-add_ready branch from c82d267 to 70f016d Compare August 29, 2023 02:34

address comment

6405f53

Signed-off-by: husharp <[email protected]>

HuSharp force-pushed the release-6.5-add_ready branch from 70f016d to 6405f53 Compare August 29, 2023 02:41

JmPotato reviewed Aug 29, 2023

View reviewed changes

client/base_client.go Outdated Show resolved Hide resolved

client/retry/backoff.go Outdated Show resolved Hide resolved

address comment

83ddbf5

Signed-off-by: husharp <[email protected]>

JmPotato approved these changes Aug 29, 2023

View reviewed changes

ti-chi-bot bot added the status/LGT1 Indicates that a PR has LGTM 1. label Aug 29, 2023

nolouch reviewed Aug 29, 2023

View reviewed changes

client/base_client.go Outdated Show resolved Hide resolved

HuSharp added 2 commits August 29, 2023 15:06

address comment

c700180

Signed-off-by: husharp <[email protected]>

make TestScheduler stable

dda1bb4

Signed-off-by: husharp <[email protected]>

HuSharp commented Aug 29, 2023

View reviewed changes

rename backoff

7fc588f

Signed-off-by: husharp <[email protected]>

HuSharp force-pushed the release-6.5-add_ready branch from 39b2043 to 7fc588f Compare August 29, 2023 08:25

nolouch approved these changes Aug 29, 2023

View reviewed changes

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

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

ti-chi-bot bot merged commit 71e8929 into tikv:release-6.5 Aug 29, 2023
16 checks passed

HuSharp deleted the release-6.5-add_ready branch August 29, 2023 14:02

niubell mentioned this pull request Aug 31, 2023

client: support add ready for resp and add backoff mechanism #6974

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

client: support backoff mechanism for memberLoop #6978

client: support backoff mechanism for memberLoop #6978

HuSharp commented Aug 23, 2023 •

edited

Loading

ti-chi-bot bot commented Aug 23, 2023 •

edited

Loading

ti-chi-bot bot commented Aug 23, 2023

codecov bot commented Aug 24, 2023 •

edited

Loading

nolouch Aug 29, 2023

HuSharp Aug 29, 2023

HuSharp Aug 29, 2023

nolouch commented Aug 29, 2023

ti-chi-bot bot commented Aug 29, 2023

ti-chi-bot bot commented Aug 29, 2023

client: support backoff mechanism for memberLoop #6978

client: support backoff mechanism for memberLoop #6978

Conversation

HuSharp commented Aug 23, 2023 • edited Loading

What problem does this PR solve?

What is changed and how does it work?

Check List

PR Summary

Reproduce Step

Reproduce Result

PR Effect

Release note

ti-chi-bot bot commented Aug 23, 2023 • edited Loading

ti-chi-bot bot commented Aug 23, 2023

codecov bot commented Aug 24, 2023 • edited Loading

Codecov Report

nolouch Aug 29, 2023

Choose a reason for hiding this comment

HuSharp Aug 29, 2023

Choose a reason for hiding this comment

HuSharp Aug 29, 2023

Choose a reason for hiding this comment

nolouch commented Aug 29, 2023

ti-chi-bot bot commented Aug 29, 2023

ti-chi-bot bot commented Aug 29, 2023

HuSharp commented Aug 23, 2023 •

edited

Loading

ti-chi-bot bot commented Aug 23, 2023 •

edited

Loading

codecov bot commented Aug 24, 2023 •

edited

Loading