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

service/s3/s3manager: Add retry download object part body #843

Merged
merged 1 commit into from
Sep 20, 2016

Conversation

jasdel
Copy link
Contributor

@jasdel jasdel commented Sep 20, 2016

Adds the ability to automatically retried S3 object parts that fail
after the initial request response is provided. This includes scenarios
such as the network connection is terminated, or broken.

The body parts will be retried based on the S3 service clients Max
Retries configuration. For example setting the aws.Config.MaxRetries to
3 will allow the S3 Downloader to retry part body failures up to 3
times.

This feature is enabled automatically. No additional work is needed to
take advantage of it.

Fix #466

Copy link
Contributor

@xibz xibz left a comment

Choose a reason for hiding this comment

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

A couple comments

err := req.Send()
retry := 0
var n int64
for {
Copy link
Contributor

Choose a reason for hiding this comment

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

for retry := 0; retry < d.partBodyMaxRetries; retry++ {
}

if err != nil {
d.setErr(err)
retry++
Copy link
Contributor

Choose a reason for hiding this comment

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

With the logic moved into the for expression, perhaps breaking early makes sense?

if err == nil {
     break
}

break
}

d.incrWritten(n)
Copy link
Contributor

Choose a reason for hiding this comment

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

Then here upon for exit

return err

retry++

// Attempt to retry failed chunk retries
if retry > d.partBodyMaxRetries {
Copy link
Contributor

@xibz xibz Sep 20, 2016

Choose a reason for hiding this comment

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

Can get rid of this if block

Adds the ability to automatically retried S3 object parts that fail
after the initial request response is provided. This includes scenarios
such as the network connection is terminated, or broken.

The body parts will be retried based on the S3 service clients Max
Retries configuration. For example setting the aws.Config.MaxRetries to
3 will allow the S3 Downloader to retry part body failures up to 3
times.

This feature is enabled automatically. No additional work is needed to
take advantage of it.

Fix aws#466
@jasdel
Copy link
Contributor Author

jasdel commented Sep 20, 2016

Updated to switch to for loop to include retry in statement.

@jasdel jasdel merged commit c23c22d into aws:master Sep 20, 2016
@jasdel jasdel deleted the feature/s3managerDownRetry branch September 20, 2016 22:55
jasdel added a commit that referenced this pull request Sep 20, 2016
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.

2 participants