Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add retry with exponential backoff to S3ObjectClient.PutObject() #4732

Conversation

JordanRushing
Copy link
Contributor

@JordanRushing JordanRushing commented Nov 10, 2021

Signed-off-by: Jordan Rushing jordan.rushing@grafana.com

What this PR does / why we need it:
This PR adds grafana/dskit retry with exponential backoff to S3ObjectClient.PutObject(). This is part of the recommended request behavior that AWS provides via its official documentation.

We added this to GetObject() in #4453

Which issue(s) this PR fixes:
N/A

Special notes for your reviewer:
N/A

Checklist

  • Documentation added
  • Tests updated
  • Add an entry in the CHANGELOG.md about the changes.

Signed-off-by: Jordan Rushing <jordan.rushing@grafana.com>
@JordanRushing JordanRushing requested a review from a team as a code owner November 10, 2021 21:04
@JordanRushing JordanRushing self-assigned this Nov 10, 2021
Signed-off-by: Jordan Rushing <jordan.rushing@grafana.com>
Signed-off-by: Jordan Rushing <jordan.rushing@grafana.com>
@owen-d
Copy link
Member

owen-d commented Nov 15, 2021

Hey, this is a great PR, but I don't think we should merge it. None of our other object clients do this for PutObject and we already have backoffs via the flushloop (30s) and the flushBackoff (1s) for retrying immediate flushes. Instead, we may wish to make the flushBackoff use an exponential backoff strategy so that it applies once for every backend rather than sequencing (flushBackoff + objectClientBackoff) for specific backends.

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

Successfully merging this pull request may close these issues.

None yet

3 participants