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

storage: adding maximum_chunk_size_gb storage option (PROJQUAY-2679) #2186

Merged
merged 3 commits into from Aug 30, 2023

Conversation

bcaton85
Copy link
Contributor

@bcaton85 bcaton85 commented Aug 28, 2023

Users are currently using the S3storage option to use the IBM storage solution. The final copy of an image push from the uploads to the sha256 storage directory currently fails for large image layers (10gb). This is due to the 10gb layer being chunked into 5gb ranges that causes delays in the IBM copy operation. This PR adds performance by removing an unnecessary copy and adds the maximum_chunk_size_gb option to reduce the size of the copy.

@codecov
Copy link

codecov bot commented Aug 28, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.01% ⚠️

Comparison is base (0e75ffa) 70.23% compared to head (064e468) 70.23%.
Report is 7 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2186      +/-   ##
==========================================
- Coverage   70.23%   70.23%   -0.01%     
==========================================
  Files         432      432              
  Lines       39305    39304       -1     
  Branches     5088     5088              
==========================================
- Hits        27606    27605       -1     
- Misses      10112    10114       +2     
+ Partials     1587     1585       -2     
Flag Coverage Δ
unit 70.23% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
storage/cloud.py 72.27% <100.00%> (-0.06%) ⬇️

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

syed
syed previously approved these changes Aug 28, 2023
@bcaton85 bcaton85 changed the title draft: removing extra copy call + adding ibm storage option storage: adding IBM storage option (PROJQUAY-2679) Aug 28, 2023
@bcaton85 bcaton85 changed the title storage: adding IBM storage option (PROJQUAY-2679) storage: adding maximum_chunk_size_gb storage option (PROJQUAY-2679) Aug 29, 2023
@bcaton85 bcaton85 marked this pull request as ready for review August 29, 2023 17:15
Copy link
Contributor

@syed syed left a comment

Choose a reason for hiding this comment

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

LGTM. We have to expose somewhere in the docs that maximum_chunk_size_gb is something that can be passed to the S3 storage provider

@bcaton85
Copy link
Contributor Author

Yup that's right, i'll bring in @stevsmit

@bcaton85 bcaton85 merged commit 3e9cff6 into quay:master Aug 30, 2023
17 checks passed
@bcaton85
Copy link
Contributor Author

/cherry-pick redhat-3.9

@openshift-cherrypick-robot

@bcaton85: new pull request created: #2191

In response to this:

/cherry-pick redhat-3.9

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sunandadadi pushed a commit to Sunandadadi/quay that referenced this pull request Aug 30, 2023
…uay#2186)

Adds the `maximum_chunk_size_gb` option to s3 storage to reduce chunk size and increase performance. Also removes redundant storage copy call.
raja-0940 pushed a commit to raja-0940/quay that referenced this pull request Sep 25, 2023
…uay#2186)

Adds the `maximum_chunk_size_gb` option to s3 storage to reduce chunk size and increase performance. Also removes redundant storage copy call.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants