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

Fix an issue where mountpoint-s3-client could interpret a HTTP 206 Partial success response as an error #917

Merged
merged 1 commit into from
Jun 17, 2024

Conversation

passaro
Copy link
Contributor

@passaro passaro commented Jun 17, 2024

Description of change

We are removing a workaround in mountpoint-s3-client that reduced the number of requests to S3 and is no longer needed. When introduced in #285, the workaround used a default CRT meta-request instead of an auto-ranged-get for small requests, which avoided a redundant HeadObject request that the CRT performed on every auto-ranged-get. Since then, the CRT has been updated to avoid the extra requests when a range is specified, so we can always use auto-ranged-get.

Does this change impact existing behavior?

No.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and I agree to the terms of the Developer Certificate of Origin (DCO).

…rtial success response as an error

We are removing a workaround in mountpoint-s3-client that reduced the number of requests to S3 and is no longer needed. When introduced in awslabs#285, the workaround used a default CRT meta-request instead of an auto-ranged-get for small requests, which avoided a redundant HeadObject request that the CRT performed on every auto-ranged-get. Since then, the CRT has been updated to avoid the extra requests when a range is specified, so we can always use auto-ranged-get.

Signed-off-by: Alessandro Passaro <alexpax@amazon.co.uk>
@passaro passaro temporarily deployed to PR integration tests June 17, 2024 18:17 — with GitHub Actions Inactive
@passaro passaro temporarily deployed to PR integration tests June 17, 2024 18:17 — with GitHub Actions Inactive
@passaro passaro temporarily deployed to PR integration tests June 17, 2024 18:17 — with GitHub Actions Inactive
@passaro passaro temporarily deployed to PR integration tests June 17, 2024 18:17 — with GitHub Actions Inactive
@passaro passaro temporarily deployed to PR integration tests June 17, 2024 18:17 — with GitHub Actions Inactive
@passaro passaro temporarily deployed to PR integration tests June 17, 2024 18:17 — with GitHub Actions Inactive
@passaro passaro temporarily deployed to PR integration tests June 17, 2024 18:17 — with GitHub Actions Inactive
Copy link

@fuatbasik fuatbasik left a comment

Choose a reason for hiding this comment

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

LGTM!

@passaro passaro enabled auto-merge June 17, 2024 18:22
@passaro passaro added this pull request to the merge queue Jun 17, 2024
Merged via the queue into awslabs:main with commit e299e2b Jun 17, 2024
23 checks passed
@passaro passaro deleted the small-requests-revert branch June 17, 2024 18:48
mhnap pushed a commit to mhnap/mountpoint-s3 that referenced this pull request Aug 1, 2024
…rtial success response as an error (awslabs#917)

We are removing a workaround in mountpoint-s3-client that reduced the number of requests to S3 and is no longer needed. When introduced in awslabs#285, the workaround used a default CRT meta-request instead of an auto-ranged-get for small requests, which avoided a redundant HeadObject request that the CRT performed on every auto-ranged-get. Since then, the CRT has been updated to avoid the extra requests when a range is specified, so we can always use auto-ranged-get.

Signed-off-by: Alessandro Passaro <alexpax@amazon.co.uk>
Co-authored-by: Alessandro Passaro <alexpax@amazon.com>
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