Hydrate group listings with a single query instead of one lookup per group#51126
Open
CrystalDime wants to merge 1 commit into
Open
Hydrate group listings with a single query instead of one lookup per group#51126CrystalDime wants to merge 1 commit into
CrystalDime wants to merge 1 commit into
Conversation
…group getTopLevelGroupsStream, searchForGroupByNameStream and the JPA adapter's getSubGroupsStream selected only group ids and then resolved every row through getGroupById, issuing one extra query per group whenever the group cache is cold. Selecting the entities instead places them in the persistence context, so the existing by-id resolution (which keeps the group cache populated and all adapter semantics unchanged) is served without any additional statements. Closes keycloak#51125 Signed-off-by: Devontay Eluett <deluett1@gmail.com>
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
1 participant
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.
Closes #51125
Problem
Listing groups (
GET /admin/realms/{realm}/groupswith and withoutsearch, andGET .../groups/{id}/children) issues one SQL statement per group whenever the group cache cannot serve it: a cold cache, and — permanently — realms larger than the bounded cache. Reproduction and end-to-end numbers are in the issue.Cause
JpaRealmProvider#getTopLevelGroupsStream,JpaRealmProvider#searchForGroupByNameStreamand the JPAGroupAdapter#getSubGroupsStreamselect only group ids and resolve each row throughgetGroupById(...); every cache miss becomes anem.findround trip — an N+1.Fix
Select the
GroupEntityrows themselves — the query keeps the same predicates, ordering, pagination and fine-grained-authz filters and changes only its SELECT list; the stream then maps each entity to its id before the existing by-id resolution. The selected entities are managed by the persistence context, so the unchanged per-id resolution throughgetGroupById(...)is served without any additional statement per group. The per-id hop is deliberately kept (rather than mapping entities straight to adapters assearchGroupsByAttributesdoes) so the group cache is still populated and all adapter and invalidation semantics stay as they are.Testing
GroupListingQueryCountTest(tests/base, same pattern asCompositeRoleExpansionQueryCountTest): a page of 20 groups must be hydrated by exactly 1 prepared statement for the top-level listing, the name search, and the subgroups listing — previously 21 each. The test forces the cache-miss path deterministically via per-group invalidations and clearing the persistence context after a warmup pass.AI disclosure
Per CONTRIBUTING.md: this change was developed with the assistance of an AI agent (code audit, patch drafting, test drafting, and benchmarking); it was reviewed and is understood and maintained by the submitter.