search

OrchardCMS / Orchard

Orchard is a free, open source, community-focused Content Management System built on the ASP.NET MVC platform.
https://orchardproject.net
BSD 3-Clause "New" or "Revised" License
2.37k stars 1.12k forks source link

Added sequential upload parameter to fileupload() parameters when importing media #8650

Closed AndreaPiovanelli closed 1 year ago

AndreaPiovanelli commented 1 year ago

To solve issue #8648 , added "sequentialUploads" parameter to fileupload options. This forces the files to be uploaded sequentially, avoiding the "progress bar stuck" problem and shortening upload times when importing multiple files.

AndreaPiovanelli commented 1 year ago

@sebastienros I added a single line to the script inside the view, but this disables parallel upload of files: do you have any other suggestions? I may add a flag in admin settings (Admin/Settings/Media) to switch the behaviour of media uploads. Browser freezes / slowness seem to heavily depend on user bandwidth, so it's not really something I can predict and properly handle with a setting. There are ways to check client bandwidth via javascript but those aren't very reliable and "cost" bandwidth too (they basically "simulate" the upload of a file, uploading bytes to the server) so, in my opinion, the "sequentialUploads" option can't be conditionally set at runtime client side.

AndreaPiovanelli commented 1 year ago

After adding the sequential upload parameter, we realized we didn't want to remove such a critical feature from MediaLibrary uploads. To solve the speed and freeze issues, we tried to implement chunked upload of files. This proved to be challenging and, after some tests, we abandoned (at least for the moment) the implementation of that feature, because it has a impact of IStorageProvider and its implementations, requiring a lot of work to properly manage chunks.

After some more tests, we found a good solution was to limit (via a tenant setting) the number of concurrent uploads, using the "limitConcurrentUploads" option of fileupload. The (administrative) setting should be based on server capabilities and settings.

sebastienros commented 1 year ago

Should there be a safe limit by default if it's known to be an issue when there are too many?

AndreaPiovanelli commented 1 year ago

Should there be a safe limit by default if it's known to be an issue when there are too many?

We thought about that but decided to leave it undefined to avoid limiting uploads by default where the issue has never been experienced (as it highly depends on client and server bandwidth). In fact, uploading 20 very small files is probably going fine, while uploading 3 large ones may cause the "freeze" on the import iframe.