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

backend/s3: Configure AWS Client MaxRetries and provide enhanced S3 NoSuchBucket error message #19951

Merged
merged 1 commit into from
Jan 11, 2019

Conversation

bflad
Copy link
Contributor

@bflad bflad commented Jan 9, 2019

References:

The AWS Go SDK automatically provides a default request retryer with exponential backoff that is invoked via setting MaxRetries or leaving it nil will default to 3. The terraform-aws-provider config.Client() sets MaxRetries to 0 unless explicitly configured above 0. Previously, we were not overriding this behavior by setting the configuration and therefore not invoking the default request retryer.

The default retryer already handles HTTP error codes above 500, including S3's InternalError response, so the extraneous handling can be removed. This will also start automatically retrying many additional cases, such as temporary networking issues or other retryable AWS service responses.

Changes:

  • s3/backend: Add max_retries argument
  • s3/backend: Enhance S3 NoSuchBucket error to include additional information

Output from acceptance testing (no new failures):

--- PASS: TestBackend_impl (0.00s)
--- PASS: TestBackendConfig (0.61s)
--- PASS: TestBackendConfig_invalidKey (0.00s)
--- PASS: TestBackend (4.53s)
--- PASS: TestBackendLocked (7.05s)
--- FAIL: TestBackendExtraPaths (2.83s)
--- PASS: TestBackendPrefixInWorkspace (1.69s)
--- PASS: TestKeyEnv (9.23s)
--- PASS: TestRemoteClient_impl (0.00s)
--- PASS: TestRemoteClient (1.84s)
--- PASS: TestRemoteClientLocks (9.10s)
--- PASS: TestForceUnlock (9.74s)
--- PASS: TestRemoteClient_clientMD5 (8.80s)
--- PASS: TestRemoteClient_stateChecksum (12.80s)

…oSuchBucket error message

The AWS Go SDK automatically provides a default request retryer with exponential backoff that is invoked via setting `MaxRetries` or leaving it `nil` will default to 3. The terraform-aws-provider `config.Client()` sets `MaxRetries` to 0 unless explicitly configured above 0. Previously, we were not overriding this behavior by setting the configuration and therefore not invoking the default request retryer.

The default retryer already handles HTTP error codes above 500, including S3's InternalError response, so the extraneous handling can be removed. This will also start automatically retrying many additional cases, such as temporary networking issues or other retryable AWS service responses.

Changes:
* s3/backend: Add `max_retries` argument
* s3/backend: Enhance S3 NoSuchBucket error to include additional information
Copy link
Contributor

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@dimisjim
Copy link

dimisjim commented Apr 8, 2019

@bflad Is there an argument to configure the time between each retry, that would compliment max_retries ?

EDIT: nw, found it, uses exp backoff by default: https://docs.aws.amazon.com/sdk-for-go/v1/api/aws.request.Retryer.html

EDIT2: Question remains however: How much time of total retries would that (max retries 5 default) translate to?

@ghost
Copy link

ghost commented Aug 13, 2019

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Aug 13, 2019
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.

3 participants