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

switch s3 putobject to upload function #2801

Merged

Conversation

loadtheaccumulator
Copy link
Collaborator

@loadtheaccumulator loadtheaccumulator commented Feb 20, 2025

Description

Repo file uploads occasionally timeout--especially on slower upstream connections.
Switching from PutObject() to Upload() resolves the issue.

FIXES: HMS-5534

Type of change

What is it?

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Documentation update
  • Tests update
  • Refactor

@mergify mergify bot added the bug fix Fixes #issue label Feb 20, 2025
Copy link
Collaborator

@lzap lzap left a comment

Choose a reason for hiding this comment

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

Yep, this is exactly what the documentation says. Upload function uses MIME multipart upload which can likely resurrect failed transaction.

https://docs.aws.amazon.com/AmazonS3/latest/userguide/mpu-upload-object.html

@loadtheaccumulator
Copy link
Collaborator Author

Docs are a wonderful thing. :-)

@lzap
Copy link
Collaborator

lzap commented Feb 20, 2025

One remark tho, my guts tells me that Upload will be more resource heavy than PutObject and using it for ALL uploads might slow down upload of small files.

@ezr-ondrej
Copy link
Member

I don't think it will be huge difference, but do we have any telemetry around this to see potential spike once we merge?

@lzap
Copy link
Collaborator

lzap commented Feb 20, 2025

I don't think it will be huge difference, but do we have any telemetry around this to see potential spike once we merge?

This was just a warning, I think it will be okay but it could possibly worsen uploads for many small files (repo upload). Just keep an eye on this @loadtheaccumulator no action is needed at this point.

@ezr-ondrej ezr-ondrej merged commit 505f611 into RedHatInsights:main Feb 20, 2025
11 checks passed
@loadtheaccumulator loadtheaccumulator deleted the fix_upload_timeouts branch February 20, 2025 16:39
@loadtheaccumulator
Copy link
Collaborator Author

NOTE: It can also be tuned via the Uploader.

@mergify mergify bot added the waiting for review Need reviewers label Feb 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug fix Fixes #issue waiting for review Need reviewers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants