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

S3 Speed Up #16438

Merged
merged 3 commits into from
Oct 2, 2023
Merged

S3 Speed Up #16438

merged 3 commits into from
Oct 2, 2023

Conversation

matdave
Copy link
Contributor

@matdave matdave commented Jun 9, 2023

What does it do?

Removed the functions that process each file individually when listing files, e.g. lastModified, fileSize, mimeType, e.g.

Why is it needed?

Currently, the S3 bucket will time out as you grow the contents of your folders. This dramatically increases the responses and has been tested on folders containing hundreds of files.

How to test

The best way to test is with an S3 bucket with numerous sample files. The file size doesn't matter, just a large number. As the number grows, the response time increases.

Related issue(s)/PR(s)

No related public issues, just personal experience

…in an effort to dramatically speed up S3 response times.
@matdave matdave requested review from opengeek and Mark-H as code owners June 9, 2023 16:40
@cla-bot cla-bot bot added the cla-signed CLA confirmed for contributors to this PR. label Jun 9, 2023
@codecov
Copy link

codecov bot commented Jun 9, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.07 ⚠️

Comparison is base (aed303b) 21.73% compared to head (f1eaf41) 21.67%.

Additional details and impacted files
@@             Coverage Diff              @@
##                3.x   #16438      +/-   ##
============================================
- Coverage     21.73%   21.67%   -0.07%     
- Complexity    10482    20994   +10512     
============================================
  Files           561     1122     +561     
  Lines         31612    63386   +31774     
============================================
+ Hits           6872    13739    +6867     
- Misses        24740    49647   +24907     
Impacted Files Coverage Δ
core/src/Revolution/Sources/modS3MediaSource.php 0.00% <0.00%> (ø)

... and 561 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@JoshuaLuckers
Copy link
Contributor

I don't have access to an S3 bucket, is there another way I can test this?

@matdave
Copy link
Contributor Author

matdave commented Jun 28, 2023

@theboxer can you review this

@matdave
Copy link
Contributor Author

matdave commented Jun 28, 2023

@JoshuaLuckers I believe Amazon offers a free tier https://aws.amazon.com/free/storage/s3/

@rthrash rthrash added this to the v3.0.4 milestone Sep 21, 2023
@rthrash rthrash added the pr/ready-for-merging Pull request reviewed and tested and ready for merging. label Sep 29, 2023
@opengeek opengeek merged commit 6d125ec into modxcms:3.x Oct 2, 2023
opengeek pushed a commit that referenced this pull request Oct 2, 2023
* remove functions that require PHP to interact with objects directly, in an effort to dramatically speed up S3 response times.

* fix PHPCS

* fix type conflict
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed CLA confirmed for contributors to this PR. pr/ready-for-merging Pull request reviewed and tested and ready for merging.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants