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.

Sign up for GitHub

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

Jump to bottom

Refactor SDK's request retry, and error handling behavior #487

Merged
merged 11 commits into from
Mar 11, 2020
Merged

Refactor SDK's request retry, and error handling behavior #487

merged 11 commits into from
Mar 11, 2020

Conversation

jasdel
Copy link
Contributor

@jasdel jasdel commented Feb 6, 2020 •
edited
Loading

This update refactors the SDK's request retry behavior by updating the SDK's implementation to a model that will be the standard across the SDKs. This model simplifies retry behavior, and will retry all connection errors. Not just write connection reset errors.

This update also refactors the SDK's error wrapping and handling. The SDK's error types such as awserr.Error implement the Unwrap method allowing the nested errors to be more easily extracted using the Go 1.13 errors.As, Is, and Unwrap functions.

This update also provides more information about why a request failed, and was retried, but exhausted attempts. The aws.MaxAttemptsError will wrap the connection or service API error.

TODO:

  • Update SDK's retry and error tests
  • Add changelog
  • Add missing/commented out tests to retry package

@jasdel jasdel added the pr/work-in-progress This PR is a draft and needs further work. label Feb 6, 2020
@jasdel jasdel changed the title Introduces new retry model and transport client to the SDK Refactor SDK's request retry behavior adding rate limiting Feb 27, 2020
@jasdel jasdel force-pushed the proto/StandardRetries branch 2 times, most recently from 778842e to 3fb5834 Compare March 4, 2020 20:41
@jasdel
Refactor SDK's request retry, and error handling behavior
843b02a 
This update refactors the SDK's request retry behavior by updating the
SDK's implementation to a model that will be the standard across the
SDKs. This model simplifies retry behavior, and will retry all
connection errors. Not just write connection reset errors.

This update also refactors the SDK's error wrapping and handling. The
SDK's error types such as awserr.Error implement the Unwrap method
allowing the nested errors to be more easily extracted using the Go 1.13
errors.As, Is, and Unwrap functions.

This update also provides more information about why a request failed,
and was retried, but exhausted attempts. The aws.MaxAttemptsError will wrap
the connection or service API error.
@jasdel jasdel changed the title Refactor SDK's request retry behavior adding rate limiting Refactor SDK's request retry, and error handling behavior Mar 7, 2020
@jasdel jasdel marked this pull request as ready for review March 7, 2020 23:28
@jasdel jasdel removed the pr/work-in-progress This PR is a draft and needs further work. label Mar 7, 2020
@jasdel
Copy link
Contributor Author

jasdel commented Mar 7, 2020 •
edited
Loading

Two commits, first includes the hand written code changes, second commit is all code generation.

@jasdel
Copy link
Contributor Author

jasdel commented Mar 8, 2020

Expect Go 1.11 and Go 1.12 builds to fail due to errors.As and other functions added in Go 1.13

@skotambkar skotambkar self-requested a review March 9, 2020 16:25
skmcgrail
skmcgrail reviewed Mar 9, 2020
View reviewed changes
aws/retry/retry.go Outdated Show resolved Hide resolved
@jasdel jasdel merged commit dc3c876 into aws:master Mar 11, 2020
@jasdel jasdel deleted the proto/StandardRetries branch March 11, 2020 16:25
jasdel added a commit to jasdel/aws-sdk-go-v2 that referenced this pull request Mar 16, 2020
@jasdel
service/s3/s3manager: Fix broken unit test
c3eae86 
Fixes a broken unit test missed when implementing aws#487.
@jasdel jasdel mentioned this pull request Mar 16, 2020
Merged
jasdel added a commit to jasdel/aws-sdk-go-v2 that referenced this pull request Mar 16, 2020
@jasdel
service/s3/s3manager: Fix broken unit test
57cc328 
Fixes a broken unit test missed when implementing aws#487.
skmcgrail pushed a commit that referenced this pull request Mar 17, 2020
@jasdel
aws: Fix BuildableHTTPClient datarace bug (#504)
7e732f1 
Fixes a broken unit test missed when implementing #487.
skmcgrail added a commit to skmcgrail/aws-sdk-go-v2 that referenced this pull request Mar 17, 2020
@skmcgrail
Release v0.20.0 (2020-03-17)
3fb24bf 
Breaking Change
---
* Update SDK retry behavior
  * Significant updates were made the SDK's retry behavior. The SDK will now retry all connections error. In addition, to changing what errors are retried the SDK's retry behavior not distinguish the difference between throttling errors, and regular retryable errors. All errors will be retried with the same backoff jitter delay scaling.
  * The SDK will attempt an operation request 3 times by default. This is one less than the previous initial request with 3 retires.
  * New helper functions in the new `aws/retry` package allow wrapping a `Retrier` with custom behavior, overriding the base `Retrier`, (e.g. `AddWithErrorCodes`, and `AddWithMaxAttempts`)
* Update SDK error handling
  * Updates the SDK's handling of errors to take advantage of Go 1.13's new `errors.As`, `Is`, and `Unwrap`. The SDK's errors were updated to satisfy the `Unwrap` interface, returning the underlying error.
  * With this update, you can now more easily access the SDK's layered errors, and meaningful state such as, `Timeout`, `Temporary`, and other states added to the SDK such as `CanceledError`.
* Bump SDK minimum supported version from Go 1.12 to Go 1.13
  * The SDK's minimum supported version is bumped to take advantage of Go 1.13's updated `errors` package.

Services
---
* Synced the V2 SDK with latest AWS service API definitions.

SDK Features
---
* `aws`: Add Support for additional credential providers and credential configuration chaining ([aws#488](aws#488))
  * `aws/processcreds`: Adds Support for the Process Credential Provider
    * Fixes [aws#249](aws#249)
  * `aws/stscreds`: Adds Support for the Web Identity Credential Provider
    * Fixes [aws#475](aws#475)
    * Fixes [aws#338](aws#338)
  * Adds Support for `credential_source`
    * Fixes [aws#274](aws#274)
* `aws/awserr`: Adds support for Go 1.13's `errors.Unwrap` ([aws#487](aws#487))
* `aws`: Updates SDK retry behavior ([aws#487](aws#487))
  * `aws/retry`: New package defining logic to determine if a request should be retried, and backoff.
  * `aws/ratelimit`: New package defining rate limit logic such as token bucket used by the `retry.Standard` retrier.

SDK Enhancements
---
* `aws`: Add grouping of concurrent refresh of credentials ([aws#503](aws#503)
  * Concurrent calls to `Retrieve` are now grouped in order to prevent numerous synchronous calls to refresh the credentials. Replacing the mutex with a singleflight reduces the overall amount of time request signatures need to wait while retrieving credentials. This is improvement becomes pronounced when many requests are made concurrently.
* `service/s3/s3manager`: Improve memory allocation behavior by replacing sync.Pool with custom pool implementation
  * Improves memory allocations that occur when the provided `io.Reader` to upload does not satisfy both the `io.ReaderAt` and `io.ReadSeeker` interfaces.

SDK Bugs
---
* `service/s3/s3manager`: Fix resource leaks when the following occurred:
  * Failed CreateMultipartUpload call
  * Failed UploadPart call
@skmcgrail skmcgrail mentioned this pull request Mar 17, 2020
Merged
skmcgrail added a commit to skmcgrail/aws-sdk-go-v2 that referenced this pull request Mar 17, 2020
@skmcgrail
Release v0.20.0 (2020-03-17)
dd25caf 
Breaking Change
---
* Update SDK retry behavior
  * Significant updates were made the SDK's retry behavior. The SDK will now retry all connections error. In addition, to changing what errors are retried the SDK's retry behavior not distinguish the difference between throttling errors, and regular retryable errors. All errors will be retried with the same backoff jitter delay scaling.
  * The SDK will attempt an operation request 3 times by default. This is one less than the previous initial request with 3 retires.
  * New helper functions in the new `aws/retry` package allow wrapping a `Retrier` with custom behavior, overriding the base `Retrier`, (e.g. `AddWithErrorCodes`, and `AddWithMaxAttempts`)
* Update SDK error handling
  * Updates the SDK's handling of errors to take advantage of Go 1.13's new `errors.As`, `Is`, and `Unwrap`. The SDK's errors were updated to satisfy the `Unwrap` interface, returning the underlying error.
  * With this update, you can now more easily access the SDK's layered errors, and meaningful state such as, `Timeout`, `Temporary`, and other states added to the SDK such as `CanceledError`.
* Bump SDK minimum supported version from Go 1.12 to Go 1.13
  * The SDK's minimum supported version is bumped to take advantage of Go 1.13's updated `errors` package.

Services
---
* Synced the V2 SDK with latest AWS service API definitions.

SDK Features
---
* `aws`: Add Support for additional credential providers and credential configuration chaining ([aws#488](aws#488))
  * `aws/processcreds`: Adds Support for the Process Credential Provider
    * Fixes [aws#249](aws#249)
  * `aws/stscreds`: Adds Support for the Web Identity Credential Provider
    * Fixes [aws#475](aws#475)
    * Fixes [aws#338](aws#338)
  * Adds Support for `credential_source`
    * Fixes [aws#274](aws#274)
* `aws/awserr`: Adds support for Go 1.13's `errors.Unwrap` ([aws#487](aws#487))
* `aws`: Updates SDK retry behavior ([aws#487](aws#487))
  * `aws/retry`: New package defining logic to determine if a request should be retried, and backoff.
  * `aws/ratelimit`: New package defining rate limit logic such as token bucket used by the `retry.Standard` retrier.

SDK Enhancements
---
* `aws`: Add grouping of concurrent refresh of credentials ([aws#503](aws#503))
  * Concurrent calls to `Retrieve` are now grouped in order to prevent numerous synchronous calls to refresh the credentials. Replacing the mutex with a singleflight reduces the overall amount of time request signatures need to wait while retrieving credentials. This is improvement becomes pronounced when many requests are made concurrently.
* `service/s3/s3manager`: Improve memory allocation behavior by replacing sync.Pool with custom pool implementation
  * Improves memory allocations that occur when the provided `io.Reader` to upload does not satisfy both the `io.ReaderAt` and `io.ReadSeeker` interfaces.

SDK Bugs
---
* `service/s3/s3manager`: Fix resource leaks when the following occurred:
  * Failed CreateMultipartUpload call
  * Failed UploadPart call
skmcgrail added a commit that referenced this pull request Mar 17, 2020
@skmcgrail
Release v0.20.0 (2020-03-17) (#507)
2930532 
Breaking Change
---
* Update SDK retry behavior
  * Significant updates were made the SDK's retry behavior. The SDK will now retry all connections error. In addition, to changing what errors are retried the SDK's retry behavior not distinguish the difference between throttling errors, and regular retryable errors. All errors will be retried with the same backoff jitter delay scaling.
  * The SDK will attempt an operation request 3 times by default. This is one less than the previous initial request with 3 retires.
  * New helper functions in the new `aws/retry` package allow wrapping a `Retrier` with custom behavior, overriding the base `Retrier`, (e.g. `AddWithErrorCodes`, and `AddWithMaxAttempts`)
* Update SDK error handling
  * Updates the SDK's handling of errors to take advantage of Go 1.13's new `errors.As`, `Is`, and `Unwrap`. The SDK's errors were updated to satisfy the `Unwrap` interface, returning the underlying error.
  * With this update, you can now more easily access the SDK's layered errors, and meaningful state such as, `Timeout`, `Temporary`, and other states added to the SDK such as `CanceledError`.
* Bump SDK minimum supported version from Go 1.12 to Go 1.13
  * The SDK's minimum supported version is bumped to take advantage of Go 1.13's updated `errors` package.

Services
---
* Synced the V2 SDK with latest AWS service API definitions.

SDK Features
---
* `aws`: Add Support for additional credential providers and credential configuration chaining ([#488](#488))
  * `aws/processcreds`: Adds Support for the Process Credential Provider
    * Fixes [#249](#249)
  * `aws/stscreds`: Adds Support for the Web Identity Credential Provider
    * Fixes [#475](#475)
    * Fixes [#338](#338)
  * Adds Support for `credential_source`
    * Fixes [#274](#274)
* `aws/awserr`: Adds support for Go 1.13's `errors.Unwrap` ([#487](#487))
* `aws`: Updates SDK retry behavior ([#487](#487))
  * `aws/retry`: New package defining logic to determine if a request should be retried, and backoff.
  * `aws/ratelimit`: New package defining rate limit logic such as token bucket used by the `retry.Standard` retrier.

SDK Enhancements
---
* `aws`: Add grouping of concurrent refresh of credentials ([#503](#503))
  * Concurrent calls to `Retrieve` are now grouped in order to prevent numerous synchronous calls to refresh the credentials. Replacing the mutex with a singleflight reduces the overall amount of time request signatures need to wait while retrieving credentials. This is improvement becomes pronounced when many requests are made concurrently.
* `service/s3/s3manager`: Improve memory allocation behavior by replacing sync.Pool with custom pool implementation
  * Improves memory allocations that occur when the provided `io.Reader` to upload does not satisfy both the `io.ReaderAt` and `io.ReadSeeker` interfaces.

SDK Bugs
---
* `service/s3/s3manager`: Fix resource leaks when the following occurred:
  * Failed CreateMultipartUpload call
  * Failed UploadPart call
This was referenced Mar 18, 2020
Merged
Closed
spezam pushed a commit to spezam/eventbridge-cli that referenced this pull request Mar 19, 2020
fixed SQS tests to support retry behaviour - sdk PR aws/aws-sdk-go-v2…
f4507a7 
…#487
@sb2nov sb2nov mentioned this pull request Apr 17, 2020
Closed
3 tasks
@glumia glumia mentioned this pull request Aug 26, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skotambkar skotambkar skotambkar left review comments

@skmcgrail skmcgrail skmcgrail approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@jasdel @skmcgrail @skotambkar