Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

backup: debounce fine-grained backup #1036

Merged
merged 8 commits into from
Apr 20, 2021

Conversation

YuJuncen
Copy link
Collaborator

@YuJuncen YuJuncen commented Apr 19, 2021

What problem does this PR solve?

Sometimes, TiKV make empty response stream from backing up request. (This seems buggy, but not sure.)

In this condition, fine-grained backup would crazily send backup RPC, if there are enough resource, thing wouldn't be so bad: when PD make progress, real leader can be found, and everything should back to normal, but for our CI, the busiest node of our community, un, ya see.

Note the timestamp. There are about 100k+ or even more logs like this.

image

What is changed and how it works?

Now, if fine-grained backup make no progress, it would backoff for 10 seconds.
The backoff time is an experience value, by analyzing the local log. Let's change it if necessary.

Check List

Tests

  • Manual test (add detailed scripts or steps below)

Release Note

  • Fix a bug that caused BR send too many useless RPCs to TiKV.

@YuJuncen YuJuncen marked this pull request as draft April 19, 2021 15:14
@YuJuncen YuJuncen marked this pull request as ready for review April 19, 2021 15:35
// If no progress, backoff 10s for debouncing.
// 10s is the default interval of stores sending a heartbeat to the PD.
// And is the average new leader election timeout, which would be a reasonable back off time.
if !hasProgress && backoffMill < 10000 {
Copy link
Collaborator

@3pointer 3pointer Apr 20, 2021

Choose a reason for hiding this comment

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

if !hasProgress is enough?

@3pointer
Copy link
Collaborator

/lgtm

@ti-chi-bot ti-chi-bot added the status/LGT1 LGTM1 label Apr 20, 2021
@glorv
Copy link
Collaborator

glorv commented Apr 20, 2021

/lgtm

@ti-chi-bot
Copy link
Member

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • 3pointer
  • glorv

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 writing /lgtm in a comment.
Reviewer can cancel approval by writing /lgtm cancel in a comment.

@ti-chi-bot ti-chi-bot added status/LGT2 LGTM2 and removed status/LGT1 LGTM1 labels Apr 20, 2021
@YuJuncen
Copy link
Collaborator Author

/merge

@ti-chi-bot
Copy link
Member

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

Commit hash: faffdbc

@ti-chi-bot ti-chi-bot merged commit 55f1843 into pingcap:master Apr 20, 2021
ti-srebot pushed a commit to ti-srebot/br that referenced this pull request Apr 20, 2021
@ti-srebot
Copy link
Contributor

cherry pick to release-5.0 in PR #1037

ti-srebot pushed a commit to ti-srebot/br that referenced this pull request Apr 28, 2021
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #1067

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants