Repository navigation
Conversation
…ra queries Closes keycloak#54886 Signed-off-by: Alexander Schwartz <alexander.schwartz@ibm.com>
2 tasks done
Closes keycloak#54886 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Alexander Schwartz <alexander.schwartz@ibm.com>
…ty realms Closes keycloak#54886 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Alexander Schwartz <alexander.schwartz@ibm.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The test’s getUserSession() null check does not prove the expired database row was deleted; it should use UserSessionPersisterProvider.loadUserSession(..., false) to verify persistence cleanup.
1 open issue
What changed in this PR
Reduces persistent-session expiration overhead for empty realms, addressing #54886 without schema changes or state carried between runs.
Changes:
- Adds a one-row existence query scoped by realm and session type.
- Skips bucketed expiration queries when no matching sessions exist, using separate short transactions for probes.
- Adds integration tests for empty-realm query behavior and expiration after a realm gains a session.
| File | Description |
|---|---|
| tests/base/src/test/java/org/keycloak/tests/model/UserSessionExpirationQueryCountTest.java | Adds query-behavior and session-expiration regression tests. |
| model/jpa/src/main/java/org/keycloak/models/jpa/session/PersistentUserSessionEntity.java | Defines the session-existence named query. |
| model/jpa/src/main/java/org/keycloak/models/jpa/session/JpaUserSessionPersisterProvider.java | Probes for sessions before running bucketed expiration. |
Review effort: Balanced
ahus1
requested review from
ruchikajha95
and removed request for
ruchikajha95
November 12, 2026 11:47
Updated user session retrieval method in test. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Alexander Schwartz <alexander.schwartz@gmx.net>
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.
Closes #54886
Each expiration run first looks for a single session of the realm and type with a one-row index lookup, and only runs the per-bucket queries if one exists. The lookup is a prefix seek on the existing expiration index, so no schema change is needed and the fix can be backported.
Decision: nothing is remembered between runs. An earlier version recorded the time a realm was found empty and skipped it until the idle timeout passed. That saved the probe query, but it depended on the wall clock, on node-local state in a shared singleton, and on the assumption that new sessions never carry a timestamp older than the observation. It was fragile under time offsets and clock changes, and it needed extra guards and tests. The probe costs one query per realm and type per run instead of about 259, which resolves the issue without that risk. A per-bucket high-water mark is tracked in #55012.
Decision: the probe runs in its own short transaction. The expiration runs in an outer transaction while the deletes use inner transactions, and a read in the outer one could hold shared locks under strict isolation levels that the deletes then wait for.
Assumption: for offline sessions the probe does not filter by
REMEMBER_ME, as that column can be true, false or NULL there. The skipped migration of the remember-me flag only concerns regular sessions, which are probed separately.Assumption: the integration test uses Hibernate statistics, which are shared with the periodic expiration task, so it asserts on query kinds and retries up to three times. It fails without the fix.