Skip to content

Conversation

@wbingli
Copy link

@wbingli wbingli commented Nov 13, 2019

Issue #, if available: N/A

Description of changes: The 16 times retry is way too much for cloudformation API. The max time will over 3 minutes, which will cause lambda function timeout.

SDK Default retry policy: https://sdk.amazonaws.com/java/api/2.0.0/software/amazon/awssdk/core/internal/retry/SdkDefaultRetrySetting.html

  • Strategy: FullJitterBackoffStrategy
  • Base delay: Duration.ofMillis(100L);
  • max backoff: Duration.ofMillis(20000L);
  • Num retries: 3

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@johnttompkins
Copy link
Contributor

Can we go ahead and remove them from the other providers as well?

@rjlohan
Copy link
Contributor

rjlohan commented Nov 13, 2019

Can you link to the SDK docs on default retry strategy for reference? Maybe file an Issue to track this change so we don't revert it by mistake later. We can't have any SDK retry past the underlying handler runtime or they are just bound to hard fail at some point. We should probably have limited retries everywhere that turns into a re-invoke. This can come later but long retries with a runtime limit is not gonna fly at scale.

@wbingli
Copy link
Author

wbingli commented Nov 13, 2019

Can you link to the SDK docs on default retry strategy for reference? Maybe file an Issue to track this change so we don't revert it by mistake later. We can't have any SDK retry past the underlying handler runtime or they are just bound to hard fail at some point. We should probably have limited retries everywhere that turns into a re-invoke. This can come later but long retries with a runtime limit is not gonna fly at scale.

Added the link and the default retry policy.

I will create other issue to follow up with other retry policy.

@rjlohan rjlohan merged commit c4dc8d0 into aws-cloudformation:master Nov 13, 2019
@rjlohan rjlohan deleted the standard_retry branch November 13, 2019 23:55
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.

4 participants