OrchardCMS / OrchardCore

Orchard Core is an open-source modular and multi-tenant application framework built with ASP.NET Core, and a content management system (CMS) built on top of that framework.
https://orchardcore.net
BSD 3-Clause "New" or "Revised" License
7.36k stars 2.37k forks source link

Media library crash With several subfolders #5001

Open aghili371 opened 4 years ago

aghili371 commented 4 years ago

i have a folder with more than 5000 subfolders. if i click on this folder, the website will stop. next time if i open media files, the selected folder will be that folder and the site crash again.

i think the tree sidebar is not a good solution to show the folders. it's better to show folder inside the content panel and doing pagination to show few folders every time or doing lazy loading to load the folders in the tree panel.

media

dodyg commented 4 years ago

Are you running on SSD?

hishamco commented 4 years ago

/cc @matiasmolleja

aghili371 commented 4 years ago

Are you running on SSD?

yes. the problem in load altogether. should have lazy loading.

Skrypt commented 4 years ago

It should show first 10 folders and a link to "load more"

hishamco commented 4 years ago

Again load more will increase the tree node, perhaps a better solution to change the view layout, but all the suggestions are welcome

deanmarcussen commented 4 years ago

likely this issue is related to the storage system underlying the media app, not the app itself (but the fix would require a load more, as @Skrypt says)

but we should confirm this, knowing that it's more difficult, but not impossible, to limit directory / folder listings, particularly on blob storage.

one only needs to see windows explorer crash for the same reason to understand.

so we need to know if it is related to the storage system failing to list all the folders, or if it is related to the size of the json returned to the vue media app.

@aghili371 please can you check your log files and post the exception from them? If no exceptions in the log files (in App_Data) please see what the chrome console reports.

aghili371 commented 4 years ago

@deanmarcussen this is my log. you can find the error at the end of log orchard-log-20191219.txt

deanmarcussen commented 4 years ago

Looks like it's timing out due to a long running request on azure. (or possibly to big a response).

Think azure used to be limited to 2min, it may have gone up to 4min

Does it work locally?

We will probably need to do paging, but that is not a super easy thing to achieve.

Short term I'd try and move some of those folders into subfolders. 5,000 folders is a lot of one root folder

aghili371 commented 4 years ago

@deanmarcussen i didn't test on local, these folders are not in the root, it has to store all subfolder in the same place, because i create for every user one folder to store him/her documentation.

sebastienros commented 4 years ago

5000 users ?

aghili371 commented 4 years ago

5000 users ?

4943 users :)

deanmarcussen commented 4 years ago

@sebastienros I have done some research on this, and it is possible to add paging to both the media and blob stores.

However it will require much changing of interfaces, on the blobstore, and filestore, to return essentially dto's with the next paging information (for example blob returns a serializable cancellation token, rather than a page number), whereas a normal file store will need a next).

Would need to be applied to both directory listings, and file listings.

Let me know if you're ok with the interface changing

hishamco commented 4 years ago

What if we did a client-side pagination without changing the interfaces? Does it solve the issue?

deanmarcussen commented 4 years ago

What if we did a client-side pagination without changing the interfaces? Does it solve the issue?

No, this will require changes to the client-side, to make it work, but the underlying issue is in the filestores, as they will need changes to support the data required for pagination.

Note While this is possible, it is quite a bit of work, hence why I'm asking for confirmation, that's its really worth doing.

Other cms's also face this issue, and most others that I've seen, simply say don't put too many files or directories in one folder/directory.

It is entirely possible for @aghili371 to resolve this issue by applying prefixing to his directory structure.

sebastienros commented 4 years ago

The bug can be fixed without changing the interfaces by loading all the files/folders in batch, inside the current method, to prevent timeouts from the http calls to the service. The UI will still be slow, but it won't crash.

deanmarcussen commented 4 years ago

Not sure how to keep the http connection alive during the batch load. (2min azure time-out or 4min if you extend it) Only way I know of is with a 201 continuation and a token to fetch results from a backgrounded task.

Any hints would be helpful

aghili371 commented 4 years ago

i think we don't save the files in the database.

so it is not better to save filenames in the JSON file or in database? then it will be easier to call part of list.