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

storage: increase aws maxRetries request config from 3 to 6 #583

Merged
merged 1 commit into from
Nov 6, 2020

Conversation

lichunzhu
Copy link
Contributor

What problem does this PR solve?

pingcap/dumpling#180

What is changed and how it works?

increase maxRetries from 3 to 6

Check List

Tests

  • Unit test
  • Integration test

Side effects

  • Possible performance regression

Related changes

  • Need to cherry-pick to the release branch

Release Note

  • No release note

@overvenus
Copy link
Member

Copy link
Member

@overvenus overvenus left a comment

Choose a reason for hiding this comment

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

Please add a test to verify if the maxRetries works as expected. Rest LGTM

@overvenus overvenus added this to the v4.0.9 milestone Nov 4, 2020
@lichunzhu lichunzhu changed the title increase maxRetries from 3 to 6 storage: increase aws maxRetries request config from 3 to 6 Nov 4, 2020
@lichunzhu
Copy link
Contributor Author

Please add a test to verify if the maxRetries works as expected. Rest LGTM

How to test this? My manual debug test can show it works. In other words, how to mock a temporary failure on minio/S3?

Copy link
Collaborator

@kennytm kennytm left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot added the status/LGT1 LGTM1 label Nov 4, 2020
@overvenus
Copy link
Member

LGTM

@ti-srebot ti-srebot removed the status/LGT1 LGTM1 label Nov 6, 2020
@ti-srebot ti-srebot added the status/LGT2 LGTM2 label Nov 6, 2020
@kennytm kennytm merged commit caeba63 into pingcap:master Nov 6, 2020
@lichunzhu lichunzhu deleted the retryForCloudRequest branch November 23, 2020 06:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants