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

SDK's output io.Reader prevents SDK from retrying failed GET. #31

Closed
jasdel opened this issue Nov 7, 2017 · 2 comments
Closed

SDK's output io.Reader prevents SDK from retrying failed GET. #31

jasdel opened this issue Nov 7, 2017 · 2 comments
Labels
needs-design refactor wontfix We have determined that we will not resolve the issue.

Comments

@jasdel
Copy link
Contributor

jasdel commented Nov 7, 2017

SDK API operations such as s3.GetObject use and io.Reader in the response for the customer to read API response data from. This prevents the SDK from retrying failed get operations as the SDK's Send has already returned.

An alternative approach would be for the SDK to take an io.Writer builder/getter as input and write to that stream. This would cause the Send to block until the response is written to the io.Writer...

Example customer code:

var f *os.File

req := svc.GetObjectRequest(params)
resp, err := req.Send(ctx, writeTempFileBuilder(&f))
if err != nil {
    panic(err)
}
if err := f. Sync(); err != nil {
    panic(err)
}

info, err := f.Stat()
if err != nil {
    panic(err)
}

fmt.Println("File Info:", Info.Name(), info.Size())
f.Close()
@fifarafa
Copy link
Contributor

@jasdel What retry strategy would you suggest for this type of operation? Are there any other services in the SDK that use similar approach to the one you suggested?

@jasdel
Copy link
Contributor Author

jasdel commented Dec 30, 2019

Thanks for asking @fifarafa. The idea behind this alternative approach is to allow the SDK to wrap the concept of retrying an S3 GetObject download attempt. The C++ SDK is the only SDK that uses a similar approach to this alternative approach. With a few exceptions (Glacier), S3 is the only API with operations to download byte streams.

The retry strategy for this approach would require the user to provide a io.Writer that can be reset/reinitialized in the case the download failed, and the API does not support byte-range requests. There are two strategies that could occur during a retry. If the API supports byte-range requests, the SDK could resume from the last byte it wrote to the user provided io.Writer. Alternatively if the API does not support byte-range requests the SDK would have to start downloading the content from the beginning, and requiring some means of being able to reinitialize the writer for a fresh write.

Both of these approaches seem fragile, and error prone. After reviewing this alternative approach, I'm not convinced it is a good idea for the SDK to invert the way it exposes downloading a byte stream from a API operation, e.g. S3 GetObject. For the S3 GetObject specific usecase the SDK's s3manager.Downloader provides much more consistent behavior since it was tailor made for S3's GetObject. The Downloader has full support for retrying failed GetObject requests.

@KaibaLopez KaibaLopez added the wontfix We have determined that we will not resolve the issue. label Sep 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-design refactor wontfix We have determined that we will not resolve the issue.
Projects
None yet
Development

No branches or pull requests

3 participants