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

[4.x]: Remote temp upload directories recreated on every asset field render #11427

Closed
mattgrayisok opened this issue Jun 13, 2022 · 3 comments
Closed

Comments

@mattgrayisok
Copy link

mattgrayisok commented Jun 13, 2022

What happened?

Description

Debugging some slow CP requests in a client project we found that whenever an Asset Field is rendered within a template, and the 'temp' directory for assets is set to a volume, Craft will try to recreate the directory structure for the temp path on every page load.

This was causing significant slowdowns for this particular project because the temp directory was set to a remote S3 FS and it had multiple path parts resulting in several sequential API calls to the remote storage to create directories, each taking a non negligible amount of time.

I profiled and tracked it down to this line:

$volume->getFs()->createDirectory($path);

Being called from:

return $this->ensureFolderByFullPathAndVolume($path, $tempVolume, false);

This is a particularly pronounced issue because most remote object storage platforms don't even need the directories to be created. Maybe it's worth allowing filesystems to define whether they need directories to be pre-created? Then object-key stores can opt out and save a load of API calls.

Steps to reproduce

  1. Use a remote object storage volume
  2. Set the temp upload path to use this volume
  3. Add an asset field to a control panel page
  4. Pages will now load slowly

Expected behavior

Humm, I guess it'd be nice if Craft didn't try to recreate the directories during field render, but instead during the actual upload which would use the temp directory. Alternatively allow remote filesystems to opt-out of directory pre-creation (maybe this is something flysystem should be doing?)

Craft CMS version

4.0.4

PHP version

8.0

Operating system and version

No response

Database type and version

No response

Image driver and version

No response

Installed plugins and versions

@mattgrayisok
Copy link
Author

I should add that we fixed the issue for the time being by simply changing the temp upload directory to the local temp dir instead of the volume

@brandonkelly
Copy link
Member

This should be resolved for the next Craft 3 and 4 releases. Thanks for reporting!

@brandonkelly
Copy link
Member

Craft 3.7.45 and 4.0.5 are out now with this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants