perf: Optimize user creation and deletion

[jason-p-pickering]

Summary

Fixes N+1 query performance problems when creating/updating users in systems with large numbers of users and user groups originally reported in https://dhis2.atlassian.net/browse/DHIS2-20614

Problem

Glowroot profiling on a production-scale database identified two major performance bottlenecks when creating a single user via the User app:

1. SchemaToDataFetcher loading ALL users.: During metadata import, uniqueness checking loaded all 221,647 users from the database (SELECT code, ldapid, username FROM userinfo - 75.6ms, 221K rows), even when importing just one user.

2. UserGroup member collection loading : Adding/removing users from groups called userGroup.addUser(user) / userGroup.removeUser(user), which triggered lazy initialization of the entire members collection. Similarly, updateUserGroups called userGroup.getMembers().size() to detect changes, also forcing full collection loading. This resulted in SELECT members0_.usergroupid ... FROM usergroupmembers INNER JOIN userinfo executing twice, loading ~12,500 member rows each time (161.4ms total).

Combined, creating a single user took 4,083ms CPU time and allocated 2.7 GB of memory. During production loading, these figures were observed to be much higher.

3. User deletion: UserRole and UserGroup member collection loading - Deleting a user triggered lazy initialization of the UserRole.members collection (110K+ users) for each role the user belonged to, via UserRoleDeletionHandler.deleteUser() calling role.getMembers().remove(user). Similarly, UserGroupDeletionHandler.deleteUser() iterated user.getGroups() and called userGroup.getMembers().remove(user), loading all members of each group.

Both deletion handlers now use direct SQL to manage the join tables via removeAllMemberships(), with updateLastUpdated calls to maintain timestamps and evict L1/L2 caches.

Additionally, the Hibernate mapping ownership for User↔UserRole was inverted to align with User↔UserGroup semantics: User.userRoles is now inverse="true" (non-owning) and UserRole.members is the owning side. Previously, User.userRoles was the owning side with cascade="all", which caused Hibernate to manage the join table through the User entity - triggering cascade-driven chain loading during user deletion.

The @Property(owner = TRUE) annotation override on getUserRoles() ensures the DHIS2 schema/import framework still treats it as an owner property, so that resetNonOwnerProperties() doesn't clear userRoles during PATCH/PUT and collectObjectReferences() still collects role references. The join table is now synced via SQL in UserObjectBundleHook.postCommit().

Solution

SchemaToDataFetcher

Added a filtered fetch(Schema, Collection) method that only queries for records matching unique property values being imported, using WHERE field IN (:values) clauses instead of loading all records. The old fetch(Schema) method was removed.

UserGroupService

Replaced Hibernate collection operations (userGroup.addUser()/removeUser()/getMembers().size()) with direct SQL via new UserGroupStore methods:

• addMemberViaSQL(userGroupId, userId) — INSERT into usergroupmembers directly
• removeMemberViaSQL(userGroupId, userId) — DELETE from usergroupmembers directly
• updateLastUpdatedViaSQL(userGroupId, lastUpdatedBy) — UPDATE usergroup metadata with L1/L2 cache eviction
• getMemberCounts(userGroupIds) — batch COUNT query for member counts without loading member entities

This avoids initializing the lazy members collection entirely.

The updateUserGroups method was rewritten to track membership changes via a changedGroups set rather than comparing getMembers().size() before and after.

canAddOrRemoveMember

Added an overload accepting UserGroup directly to avoid redundant database lookups when the caller already has the entity loaded. The original code called canAddOrRemoveMember(uid) (which fetched the group by UID), then immediately
called getUserGroup(uid) again.

Additional fixes

• Replaced deprecated Class.newInstance() with getDeclaredConstructor().newInstance() in SchemaToDataFetcher
• Reduced cognitive complexity in SchemaToDataFetcher.mapUniqueFields() by extracting helper methods

Results

Glowroot traces for creating a single user with group assignment on a copy of the database where this problem was originally observed.

	Before	After
SchemaToDataFetcher query	75.6ms -> 221,647 rows	Eliminated
Member collection loading	161.4ms ->2 × ~12,500 rows	Eliminated
CPU time	4,083ms	259ms
Allocated memory	2.7 GB	34.7 MB

Gatling simulation test was performed and the feature branch was consistently faster across the board. 2000 users were imported into the Sierra Leone database for this test.

Mean Response Time (ms)

Scenario	Baseline	Feature	Diff	Change
GET Users - userRoles expansion	185	150	-35	⬇️ -18.9%
GET Users - userGroups expansion	187	169	-18	⬇️ -9.6%
GET Users - query filter	208	180	-28	⬇️ -13.5%
GET Users - organisationUnits expansion	230	196	-34	⬇️ -14.8%
GET Users	153	133	-20	⬇️ -13.1%
GET Users - large page size	1263	1015	-248	⬇️ -19.6%
GET Users - combined common fields	341	244	-97	⬇️ -28.4%
PUT User - full update	642	42	-600	⬇️ -93.5%
PATCH User - partial update	684	52	-632	⬇️ -92.4%
GET Users - all fields	836	619	-217	⬇️ -26.0%
DELETE User - delete	2152	156	-1996	⬇️ -92.8%
POST Metadata Import - single user	865	646	-219	⬇️ -25.3%
POST User - create	867	646	-221	⬇️ -25.5%

95th Percentile Response Time (ms)

Scenario	Baseline	Feature	Diff	Change
GET Users - userRoles expansion	261	171	-90	⬇️ -34.5%
GET Users - userGroups expansion	257	215	-42	⬇️ -16.3%
GET Users - query filter	284	221	-63	⬇️ -22.2%
GET Users - organisationUnits expansion	291	227	-64	⬇️ -22.0%
GET Users	214	149	-65	⬇️ -30.4%
GET Users - large page size	1263	1015	-248	⬇️ -19.6%
GET Users - combined common fields	341	244	-97	⬇️ -28.4%
PUT User - full update	642	62	-580	⬇️ -90.3%
PATCH User - partial update	684	98	-586	⬇️ -85.7%
GET Users - all fields	836	619	-217	⬇️ -26.0%
DELETE User - delete	2152	207	-1945	⬇️ -90.4%
POST Metadata Import - single user	865	646	-219	⬇️ -25.3%
POST User - create	867	646	-221	⬇️ -25.5%

⬇️ = faster (improvement), ⬆️ = slower (regression)

Known limitation

The direct SQL operations bypass Hibernate's event lifecycle, which means PostCacheEventPublisher does not fire for these changes. In multi-instance deployments with Redis cache invalidation enabled (redis.cache.invalidation.enabled=ON), other nodes will not receive cache invalidation messages for usergroupm
embers changes. This will be addressed in a follow-up ticket. Single-instance deployments are unaffected — local L1/L2 caches are explicitly evicted.

Testing

All existing tests pass. Tested with Glowroot on a copy of the production database where the problem was originally observed.

AI Disclaimer:
Portions of the PR were developed with the assistance of AI.

[jbee]

Looks like a step in a good direction to me with having service methods that represent a user level action which take UID inputs and then mainly use SQL to perform the operation. This will create a mix of using SQL and manipulating the object model which we know has its risk. I hope we can eliminate more of the object model manipulation code in future improvements.

[david-mackessy]

Overall, the outcome for this scenario seems to be much improved, nice job 👍
Some comments:

• having a performance test included in the new performance test suite would be great to have. It would be nice to build this suite up, especially when fixing/improving specific performance issues.
• Introducing the use of session().evict (outside of the CacheInvalidationListener) is something new in the system. This feels like it has the potential to introduce very subtle issues that might be hard to pin down in future. By doing this, we're trying to take control away from Hibernate, even though almost everything relies on it. Only time will tell.

[jason-p-pickering]

• Introducing the use of session().evict (outside of the CacheInvalidationListener) is something new in the system. This feels like it has the potential to introduce very subtle issues that might be hard to pin down in future. By doing this, we're trying to take control away from Hibernate, even though almost everything relies on it. Only time will tell.

Its a fair point. This is the necessary price of using raw SQL in a Hibernate managed context. The issue of the Redis cache is noted with a comment to deal with with an additional follow up.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
1 Accepted issue

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud