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

Optional exponential backoff #80

Closed
wants to merge 8 commits into from
Closed

Conversation

joshowen
Copy link
Contributor

Amazon recommends using exponential backoff on 5xx errors and on ProvisionedThroughputExceededException responses. I haven't tested this code yet, but wanted to see if there is interest in including it in the project. If so I'll clean it up and get some documentation written.

@joshowen
Copy link
Contributor Author

@jlafon @danielhochman curious on your thoughts

@jlafon
Copy link
Contributor

jlafon commented Oct 13, 2015

@joshowen What about using botocore's implementation?

@danielhochman
Copy link
Contributor

My opinion:

I'm a fan of handling retries at the application layer. As an example, botocore's retry logic is not well understood. When this kind of handling is on by default, it's sure to behave unexpectedly during an outage or throttling, and can mask real issues with the application in the case that it does recover operations. Some may also want more granular retry logic (e.g. per operation), in which case you have to rewrite it yourself anyways.

If we do ship something like this I would vote for it to be off by default.

@joshowen
Copy link
Contributor Author

I actually didn't know botocore had an implementation included. This PR has retrying disabled by default, and I'm currently using it in production without any issues. Is there any interest in including it in the project?

@joshowen
Copy link
Contributor Author

@jlafon any thoughts on including this functionality in the project? Happy to rewrite to use BotoCore's implementation if you want it included.

@jlafon
Copy link
Contributor

jlafon commented Nov 23, 2015

@joshowen Using botocore's sounds like a good idea.

@danielhochman
Copy link
Contributor

superseded by #135

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

Successfully merging this pull request may close these issues.

3 participants