Open TFleury opened 4 years ago
//cc @deanmarcussen
When this feature was designed we realized these limitations, and postponed these on purpose for when we would find some time to improve it. So definitely a yes to find solution to these issues.
The IDistributedCache is not a valid solution IMO as it will just move the static files in memory (even if it's stored on redis for instance). The goal here is to use the fastest way to serve the content by using the FS.
I think we talked that we could just define more options in the feature with disk quotas, and a background task to ensure the quotas are respected. And also create an handler interface when files are updated/removed such that this feature could also remove the file from the cache.
This should not be hard to implement.
Yes, also agree that there are more improvements that can be done to reduce the limitations.
Here's a few thoughts on the limitations mentioned, plus a couple of others.
Multiple instances, and purging. This should probably be handled by a change token, see what @jtkech has been working on here https://github.com/OrchardCMS/OrchardCore/pull/4001 and then when the distributed support is completed https://github.com/OrchardCMS/OrchardCore/pull/2247 it should work in the same way as the other cached items.
Disc space management. Yes there is no concept of quotas, or such at this point. We kept it really simple to begin with. A background task (as a feature) or even a workflow could manage this based on file access times, purging files not access for more than x days, or based on a size quote. One of the intentions with the cdn configuration options was that if you want to keep the storage at a minimum on the server you use a cdn to front the media, and are then able to purge the local media cache regularly. (or automatically!). One of the other issues with disc space on the local server is the image sharp cache. Depending on the sizes you resize this may grow much larger than the blob cache. I.e. for each image there will be at least two thumbnails created, plus whatever sizes you resize to in templates. It would be great to apply any disc space management to the is-cache
as well, and we did talk about making it a cache per tenant, and adding a purge ui in for it as well. At the time we were waiting for some configuration changes to happen the image sharp code base, they have now been merged.
Deleted or changed media. The simplest solution here is as Seb suggests, handler / events on the media controller. There are other options I considered, based on blob hashes, but they were all quite expensive, in terms of head requests etc. One thing we particularly want to avoid is the ImageSharp approach, which works really well with local media, but requires a head request before every image is served when using blob storage, to check that the file still exists. There are limitations with using the handler/event approach, i.e. that media uploaded manually and outside of OrchardCore would not be covered, but that instance is a case where the cache should be purged afterwards. We did consider allowing purge of specific items, rather than the whole cache, but this would not work for the ImageSharp cache as the file names are hashed irreversibly. Handler events should probably use the change token arrangement for distributing signals as well.
Cache busting. There is a minor limitation in cache busting blob files currently, in that it will have to have retrieved the item from blob storage before it is available to the local file system to generate a hash. And the hash will be based on the local file system's hash. So not useful for determining media cache validity. I did consider this as another alternative to checking validity, but it is much more complicated than events, and heavier on head requests. Also in terms of using cache busting it is a minor limitation as most media items will be uploaded via the media app, causing them to be retrieved to local cache as soon as the thumbnail is generated.
The one last point I should make is that we kept the purge interfaces quite simple, and limited, so that other storage providers could also implement them without much effort. So whatever we do to improve it should keep the basic implementation as simple as possible, and be opt in to more improved features that target azure blob specifically.
If you're working on this @TFleury happy to help with anything, ping me on gitter or here
When the
DefaultMediaFileStoreCacheFileProvider
is enabled (with Azure Media Storage) all accessed media are stored in 'ms-cache' directory.It's very efficient for performance, but I see some issues here :
Shouldn't we use
IDistributedCache
for that ? Probably with a maxFileSize above which the file is not cached but served directly fromIMediaFileStore
.