perf: Fix N+1 query problems when creating users with group membership#23103
Draft
jason-p-pickering wants to merge 3 commits intomasterfrom
Draft
perf: Fix N+1 query problems when creating users with group membership#23103jason-p-pickering wants to merge 3 commits intomasterfrom
jason-p-pickering wants to merge 3 commits intomasterfrom
Conversation
5e49b5d to
cbada3e
Compare
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Fix N+1 query problems when creating users with group membership
Background
Creating a new user in a large DHIS2 instance was significantly slower than expected due to two independent N+1 query problems, both triggered within the same request. This PR addresses both. Parts of the PR have been extracted from a previous attempt to fix this problem, but after review, it was decided to add performance tests for each step of the process (creation, update, deletion). This PR addresses the first part, namely creation of new users.
Problem 1: SchemaToDataFetcher loads all users for uniqueness checking
During preheat (which runs before every metadata import, including API-driven user creation), SchemaToDataFetcher.fetch() was issuing an unbounded query to load all records of a given type in order to check for uniqueness conflicts:
SELECT username, code FROM userinfo -- returns ALL 221K usersThis is unnecessary because uniqueness only needs to be checked against the specific values being imported. The fix scopes the query to only records whose unique property values match what is being imported:
SELECT username, code FROM userinfo WHERE username IN (:usernameValues)For a database with 221K users, this eliminates a 221K-row full table scan on every user creation request.
Not only does it load the full table for users, but the preheat also collects referenced types transitively.
With the old fetch(schema):
With the new fetch(schema, objectsBeingImported):
This eliminates full-table scans for classes which are not actually being imported and it filters the query down to only relevant rows for classes are being imported. The fact that CategoryCombo appears at all in a user import is a consequence of how
deeply the preheat traverses the object graph - it's being defensive about uniqueness checking across the whole schema. However, this is entirely unnecessary since there is no uniqueness constraint on category combos with respect to users. Glowroot traces showed that trace entities were reduced from 106 to 5 during the import of a single user.
Problem 2: Adding a user to a group loads the entire group membership
DefaultUserGroupService.addUserToGroupswas using Hibernate to add users to groups by callinguserGroup.addUser(user)followed byuserGroupStore.updateNoAcl(userGroup). Because UserGroup.members is the owning side of theusergroupmembersjoin table, Hibernate had to load the entire members collection before it could detect what changed and write the join table, even when only adding a single user.For a group with 42K members this results in a 42K-row query per group the new user is assigned to.
The fix adds a direct SQL INSERT to
HibernateUserGroupStorethat writes tousergroupmemberswithout loading the members collection, along with targeted cache eviction for both User.groups and the UserGroup entity.Performance results (Gatling, 3 iterations, group with ~42K members)
On master, 100% of requests exceeded 1.2 seconds. On this branch, 100% completed under 800ms. The test was conducted on a copy of the database where this issue was originally observed.
Changes
Notes
addMemberSQL bypass manually evicts the Hibernate L2 cache for User.groups and UserGroup.members. In a clustered deployment, other nodes will not receive a cache invalidation signal for these entries until TTL expires. This is a known limitation documented in code comments and will need to be addressed separately.