fix(api): fix performance issue on PROPFIND api#4676
Conversation
|
Hi @provokateurin can you help check from your end if this is good enough? This was able to reduce the query time of PROPFIND from 208 seconds -> 46 seconds. And on another instance from 944 sec to 116 sec |
tpelingan
force-pushed
the
master
branch
2 times, most recently
from
f920420 to
257439c
Compare
provokateurin
left a comment
provokateurin
left a comment
There was a problem hiding this comment.
I get the feeling that you don't understand and test the code changes. This PR is not working in its current state and the optimization you're trying to apply will break, too.
| // a separate SELECT on oc_filecache via parentFolder->get(folderId). | ||
| $basePath = $parentFolder->getInternalPath(); |
There was a problem hiding this comment.
Yes, but this is needed in order to create the folder if it is missing, so you can't remove it.
There was a problem hiding this comment.
interesting. what if we do a query only if the folder is missing?
There was a problem hiding this comment.
and how do you suggest that we fix the N+1 query issue introduced here on version 20?
There was a problem hiding this comment.
But you can't know if the folder exists or not without doing a query.
A solution might be to create these subfolders when the groupfolder is created, but I'm not even sure if that would make it work.
There was a problem hiding this comment.
I see. Can we not make the GET PROPFIND API be purely read only? I'm a bit surprised that these operations need to happen. Is there a specific reason or can we maybe remove the folder creation logic?
This fixes nextcloud#4095 Signed-off-by: tpelingan <theodoro.pelingan@gmail.com>
3 participants
This fixes #4095
Problem
After upgrading groupfolders from v19 to v20, PROPFIND /remote.php/dav/files/{user}/ became significantly slower for instances with many groupfolders (e.g., 600+ folders: 4s → 18s).
Root cause: Commit 2637d7d ("remove assumptions that all groupfolders share a storage id") introduced per-folder node resolution in FolderStorageManager::getBaseStorageForFolderRootJail(). Each call to parentFolder->get((string)$folderId) triggers a separate SELECT on oc_filecache. With N groupfolders, this creates an N+1 query problem — N extra DB queries per PROPFIND request.