[MAGNOLIA-5994] Retrieving all users or all groups is too slow if there's a lot of users in jcr Created: 17/Nov/14  Updated: 03/Dec/14  Resolved: 20/Nov/14

Status: Closed
Project: Magnolia
Component/s: security
Affects Version/s: 5.3.5
Fix Version/s: 5.3.6

Type: Bug Priority: Critical
Reporter: Lars Fischer Assignee: Daniel Lipp
Resolution: Fixed Votes: 0
Labels: performance, support, uzh
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
relation
is related to MAGNOLIA-5957 Speed up 5-series login and working f... Closed
is related to MGNLUI-3276 Deleting groups or roles is too slow ... Closed
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Date of First Response:

 Description   

Among others this can result in deletion of a single group easily lasting more than 30 seconds.

There are websites in the system with around 60 groups, so the total time for deletion is around 30 minutes.

  • Deleting must be faster
  • It should be possible to do the deletion of group/role folders asynchronously (otherwise the already known server timeout will happpen).


 Comments   
Comment by Daniel Lipp [ 20/Nov/14 ]

Main reason is that MgnlUserManager#findAllUsersInFolder and MgnlGroupManager#findAllGroupsInFolder traverse the node hierarchy in order to find all matches instead of using JCR queries. This doesn't scale well.

Comment by Magnolia International [ 21/Nov/14 ]

As far as I can tell - correct me if I'm wrong - findAllGroupsInFolder() and findAllUsersInFolder() are only used by getAllGroups() and getAllUsers() respectively.

Consequently and since it's using a query rather than iterate and traverse, it seems a bit silly to use an "accumulate" type of method (no return value, but passing a collection into which we accumulate the results). I guess that's to avoid breaking the API? Maybe we're able to simplify this for 5.4 ?

I pushed a tiny change to javadoc and a parameter name that hopefully makes it clearer that we're "accumulating".

Comment by Daniel Lipp [ 21/Nov/14 ]

Yup, passing a collection is ugly but for 5.3.6 we needed backwards compatibility. For now I'll also apply just these fixes for 5.4. IMHO the best new approach would be to offer something like "Iterator<User> getUserIterator()" on UserManager - that would then really scale as the caller could e.g. stop iterating as soon as he found a certain number of matches. I'd recommend tackling this together with adapting the potential callers at a later point in time.

Generated at Mon Feb 12 04:10:19 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.