client: add backoff for `member loop` #6995

HuSharp · 2023-08-28T10:58:59Z

What problem does this PR solve?

Issue Number: Ref #6949 #6556

What is changed and how does it work?

Check List

Tests

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

PR Summary

~~1. 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.~~
2. 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.

ti-chi-bot · 2023-08-28T10:59:01Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

lhy1024
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-28T10:59:01Z

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

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

client/pd_service_discovery.go

client/retry/backoff.go

lhy1024 · 2023-08-29T03:17:48Z

client/retry/backoff.go

+}
+
+// ExponentialBackoff Get the exponential backoff duration.
+func (rs *BackOffer) exponentialBackoff() time.Duration {


Will we use exponential backoff by default? When network partition a long time, it may need more time to recover.

limited by max backoff time...

how long is it?

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

client/retry/backoff.go

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

HuSharp · 2023-08-29T07:20:18Z

client/pd_service_discovery.go

@@ -239,17 +241,19 @@ func (c *pdServiceDiscovery) updateMemberLoop() {
 	ticker := time.NewTicker(memberUpdateInterval)
 	defer ticker.Stop()

+	bo := retry.InitialBackOffer(updateMemberBackOffBaseTime, updateMemberTimeout)


@lhy1024 for member loop max backup time time is 1 second which is updateMemberTimeout, PTAL, thx!

codecov · 2023-08-29T07:31:29Z

Codecov Report

Merging #6995 (2af0f29) into master (50368e5) will increase coverage by 0.01%.
Report is 7 commits behind head on master.
The diff coverage is 100.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6995      +/-   ##
==========================================
+ Coverage   74.18%   74.19%   +0.01%     
==========================================
  Files         433      433              
  Lines       46133    46097      -36     
==========================================
- Hits        34225    34203      -22     
+ Misses       8887     8879       -8     
+ Partials     3021     3015       -6

Flag	Coverage Δ
unittests	`74.19% <100.00%> (+0.01%)`	⬆️

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

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

nolouch

lgtm! good job.

nolouch · 2023-08-29T10:50:40Z

/merge

ti-chi-bot · 2023-08-29T10:50:42Z

@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-29T10:50:44Z

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

Commit hash: 2af0f29

nolouch · 2023-08-31T08:09:13Z

/run-cherry-picker

nolouch · 2023-08-31T08:12:49Z

/run-cherry-picker

ti-chi-bot · 2023-08-31T08:13:23Z

In response to a cherrypick label: new pull request created to branch release-7.1: #7020.

ref tikv#6949 Signed-off-by: ti-chi-bot <[email protected]>

ref #6949 Signed-off-by: husharp <[email protected]> Co-authored-by: husharp <[email protected]>

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

ti-chi-bot bot requested review from lhy1024 and rleungx August 28, 2023 10:59

ti-chi-bot bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 28, 2023

HuSharp force-pushed the add_back_off branch from 70608c5 to c005f2a Compare August 28, 2023 11:05

refine backoff

2f08813

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

HuSharp force-pushed the add_back_off branch from c005f2a to 2f08813 Compare August 28, 2023 11:06

address comment

21a9820

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

HuSharp marked this pull request as ready for review August 29, 2023 03:04

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 29, 2023

lhy1024 reviewed Aug 29, 2023

View reviewed changes

address comment

b9deebc

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

lhy1024 reviewed Aug 29, 2023

View reviewed changes

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

address comment

6c9c6f0

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

lhy1024 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

HuSharp commented Aug 29, 2023

View reviewed changes

HuSharp force-pushed the add_back_off branch from 863d1a4 to 9b984bf Compare August 29, 2023 08:21

rename backoff

2af0f29

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

HuSharp force-pushed the add_back_off branch from 9b984bf to 2af0f29 Compare August 29, 2023 08:27

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 added 2 commits August 29, 2023 10:50

Merge branch 'master' into add_back_off

d023333

Merge branch 'master' into add_back_off

a0562c4

nolouch approved these changes Aug 29, 2023

View reviewed changes

ti-chi-bot bot merged commit 9a574ed into tikv:master Aug 29, 2023
19 checks passed

HuSharp deleted the add_back_off branch August 29, 2023 13:28

nolouch mentioned this pull request Aug 30, 2023

reduce pd api calls when fails #6949

Closed

7 tasks

niubell mentioned this pull request Aug 31, 2023

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

Closed

nolouch added the affects-7.1 label Aug 31, 2023

nolouch added the needs-cherry-pick-release-7.1 Should cherry pick this PR to release-7.1 branch. label Aug 31, 2023

ti-chi-bot mentioned this pull request Aug 31, 2023

client: add backoff for member loop (#6995) #7020

Merged

ti-chi-bot pushed a commit to ti-chi-bot/pd that referenced this pull request Aug 31, 2023

This is an automated cherry-pick of tikv#6995

6437dd8

ref tikv#6949 Signed-off-by: ti-chi-bot <[email protected]>

ti-chi-bot bot pushed a commit that referenced this pull request Sep 1, 2023

client: add backoff for member loop (#6995) (#7020)

931012e

ref #6949 Signed-off-by: husharp <[email protected]> Co-authored-by: husharp <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

client: add backoff for `member loop` #6995

client: add backoff for `member loop` #6995

HuSharp commented Aug 28, 2023 •

edited

Loading

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

edited

Loading

ti-chi-bot bot commented Aug 28, 2023

lhy1024 Aug 29, 2023

HuSharp Aug 29, 2023

lhy1024 Aug 29, 2023

HuSharp Aug 29, 2023

lhy1024 Aug 29, 2023

codecov bot commented Aug 29, 2023 •

edited

Loading

nolouch left a comment •

edited

Loading

nolouch commented Aug 29, 2023

ti-chi-bot bot commented Aug 29, 2023

ti-chi-bot bot commented Aug 29, 2023

nolouch commented Aug 31, 2023

nolouch commented Aug 31, 2023

ti-chi-bot commented Aug 31, 2023

client: add backoff for member loop #6995

client: add backoff for member loop #6995

Conversation

HuSharp commented Aug 28, 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 28, 2023 • edited Loading

ti-chi-bot bot commented Aug 28, 2023

lhy1024 Aug 29, 2023

Choose a reason for hiding this comment

HuSharp Aug 29, 2023

Choose a reason for hiding this comment

lhy1024 Aug 29, 2023

Choose a reason for hiding this comment

HuSharp Aug 29, 2023

Choose a reason for hiding this comment

lhy1024 Aug 29, 2023

Choose a reason for hiding this comment

codecov bot commented Aug 29, 2023 • edited Loading

Codecov Report

nolouch left a comment • edited Loading

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

nolouch commented Aug 31, 2023

nolouch commented Aug 31, 2023

ti-chi-bot commented Aug 31, 2023

client: add backoff for `member loop` #6995

client: add backoff for `member loop` #6995

HuSharp commented Aug 28, 2023 •

edited

Loading

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

edited

Loading

codecov bot commented Aug 29, 2023 •

edited

Loading

nolouch left a comment •

edited

Loading