Repository navigation
Perf: Optimize share providers by fetching the shares in the manager #58086
Description
Activity
The impact should be not that big as only deck, circle and talk implement this API and they are released at the same time as server.
Deck and Talk are not strictly released same time as server.
The problem is the "usergroup share" equivalent entries can/must not be created when doing a share. So it's kind of hard for the Share manager to know up front which Talk shares are relevant?
The general idea and decision from a former performance hackweek has been to centralize the share providers into the core/platform so we are able to optimize the db logic in a way due to centralization -> we can remove generic aspects. So the analysis on how to improve this can and should include a look at the existing share providers.
- Beware that this should either happen after Add new share type federated_group and provider for VO app #52992 has been merged or we would need to take care of updating that PR and merge it.
Reacted by Carl SchwanUsual workflow for a provider:
- Get the list of group/room/board that a user as access to.
- Get the list of shares for those entities
- Override the target, permissions, and accepted status.
3.1. Based on the user share
3.2. Based on the board's permissions.
Those three might be hard to centralize.
1Could be a new API to implement on each provider, so that the manager has that info.
2and3.1could probably be merged and centralized.
3.2could be trickier. Or we decide that1should also return the permissions that a user has for a board.I started a proof of concept: #58365 mostly focused on optimizing the reads. This moves some of the work from the providers to the IShareManager
And since I am adding new interface, I am adding some pagination APIs that are not based on offset (inspired by Mastodon). It's not yet working but that seems promising so far trying to implement the new interfaces in the share providers from server.
Reacted by Andy ScherzingerReacted by Andy ScherzingerReacted by Andy ScherzingerReacted by Andy Scherzinger and Jonathan Treffler
Currently when fetching shares, we first fetch the shares from each provider and then merge the results in the manager. This is inefficient and creates for each share provider (default, federated share, mail, deck, circle and talk) a very similar request where we do a
SELECTquery where the only difference is aAND share_type = ?.Instead we could have a
createShareFromRow(array $data): ISharein the IShareProvier and let the manager fetch all shares and then call thecreateShareFromRowcorresponding to the provider. This would be much quicker and would allow use to get rid of some duplication.This comes from #58057 where we are doing an absurd number of SQL request when deleting a group share (around 6 queries per user who had access to the share) and that could be only one. And there is more codepath doing similar queries.
This requires an change in the OCP API. The impact should be not that big as only deck, circle and talk implement this API and they are released at the same time as server.
@AndyScherzinger @nickvergessen @artonge