Cache sweeper is executed for request to ActiveStorage::DiskController#show

[albiere]

Hey all 👋

I have an application running Rails 8.0.2 and had just added ActiveStorage to my application. In development, I noticed the uploaded images had very slow response times.

I started debugging the problem and noticed the issue has to do with the cache sweeper being executed even for requests to ActiveStorage::DiskController#show, which is responsible for responding with the uploaded file requested.

https://github.com/rails/propshaft/blob/main/lib/propshaft/railtie.rb#L59

That means the cache sweeper is executed multiple times depending on how many uploaded files are being requested on the page. In my case, I have over 2k (I know, I know) .svg icons in my assets folder and the default file watcher takes its time to check every single one of them. Updating the file watcher improves the response time quite a bit though.

The thing is, I can't see a reason why the cache sweeper should be executed when requesting an uploaded file from ActiveStorage. What do you all think about this?

[brenogazzola]

Weird. Propshaft does not add the /storage folder to its paths by default, so it should not trigger a sweep when a file is uploaded. Did you manually add paths anywhere in your app?

Meanwhile, I suggest you switch to evented file checker like suggested in the README.

[albiere]

@brenogazzola the before_action which performs the sweep does not seem to filter out any type of path as long as its controller inherits from ActionController::Base, which is the case for ActiveStorage::DiskController.

Let me know if I can help you with a fix.

[brenogazzola]

@albiere You are correct. However, let me explain what I meant in more detail, because there are two steps to the cache sweeper.

Step 1: Check if files were updated
As you have mentioned, this is triggered on any controller action, no matter what paths Propshaft has been tracking. With the EventedFileUpdateChecker this happens immediately, but with the default watcher (FileUpdateChecker), its slower because the checker has to check the size and update time of every file in the paths to know if something changed.

This means this step can be a bit slow, but from what I remember when I was using the default watcher in a project with 6k assets, it wasn't quite as slow as what you are describing.

Step 2: Clear the cache, load it again
This one is slow no matter what watcher you are using, since Propshaft has to read the content of every file, and calculate its digest.

What I actually mean in my previous message
The kind of slow down you are describing is something I usually associate with step 2, not step 1. This is why I asked if you accidentally added the /storage folder to the paths, which would make step 2 be run on every file upload through ActiveStorage.

However, if you have not, then I think we need to take a step back and consider that you are probably seeing this level of slow down any time your application hits a controller action. In this case, changing Propshaft not to trigger on ActiveStorage's controllers feels like solving a symptom, not the cause. Solving the cause means:

1 - Checking if you are not accidentally using the NullFileWatcher that França introduce a couple months ago, which always sweeps the cache: Rails.application.assets.config.file_watcher
2 - Ensuring you have not added /storage to the paths by accident: Rails.application.assets.config.paths
3 - Switching to the EventedFileUpdateChecker which will solve the problem for the entire application, not just ActiveStorage controller's

[albiere]

@brenogazzola I really appreciate the thorough explanation!

The slowness comes from the FileUpdateChecker, so no, the NullFileChecker is not being used here. More precisely, from the FileUpdateChecker#watched private method.

I've created a new repo - https://github.com/albiere/testing-propshaft - where you can see how slow it is to render the 3 variants of a uploaded image. Just go to /users/new, upload an image and check the logs for the response time of ActiveStorage::DiskController#show. I added the rails_icons gem just so you have the 2k+ files in the assets folder in order to make things slow. Everything else in the app is default rails 8 + ruby 3.3.5.

[albiere]

I've spent some more time debugging the issue and the call to Dir[@glob] in ActiveSupport::FileUpdateChecker#watched is what makes things very very slow. That's because of the way the glob is built. I made a very simple change to the glob and things got much faster. Then I realized that change is already in the main branch of rails here: rails/rails#53680 😅

So, yeah.. I still think there's no need to call the sweeper for active_storage requests, but I understand if we want to keep things simpler. Anyway, that's probably not gonna be a big issue in the future and, of course, changing the cache_sweeper to be a EventedFileUpdateChecker makes things much faster.