From e46c879cdecb5510477e4d3b676d9c93ea1b8ff7 Mon Sep 17 00:00:00 2001 From: Alexander Schwartz Date: Tue, 2 Sep 2025 15:43:03 +0200 Subject: [PATCH] Retry duplicate exceptions to handle concurrent client sessions Closes #42278 Signed-off-by: Alexander Schwartz --- .../PersistentSessionsChangelogBasedTransaction.java | 6 +----- .../model/session/UserSessionConcurrencyTest.java | 10 ++++------ 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/changes/PersistentSessionsChangelogBasedTransaction.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/changes/PersistentSessionsChangelogBasedTransaction.java index 7efa3efc533..af9a1329b94 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/changes/PersistentSessionsChangelogBasedTransaction.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/changes/PersistentSessionsChangelogBasedTransaction.java @@ -23,7 +23,6 @@ import org.jboss.logging.Logger; import org.keycloak.common.util.Retry; import org.keycloak.models.AbstractKeycloakTransaction; import org.keycloak.models.KeycloakSession; -import org.keycloak.models.ModelDuplicateException; import org.keycloak.models.RealmModel; import org.keycloak.models.UserSessionModel; import org.keycloak.models.sessions.infinispan.SessionFunction; @@ -150,10 +149,7 @@ abstract public class PersistentSessionsChangelogBasedTransaction KeycloakModelUtils.runJobInTransaction(kcSession.getKeycloakSessionFactory(), super::applyChangesSynchronously), (iteration, t) -> { - if (t instanceof ModelDuplicateException ex) { - // duplicate exceptions are unlikely to succeed on a retry, - throw ex; - } else if (iteration > 20) { + if (iteration > 20) { // never retry more than 20 times throw new RuntimeException("Maximum number of retries reached", t); } diff --git a/testsuite/model/src/test/java/org/keycloak/testsuite/model/session/UserSessionConcurrencyTest.java b/testsuite/model/src/test/java/org/keycloak/testsuite/model/session/UserSessionConcurrencyTest.java index dddb7ec9d1a..97f8d883943 100644 --- a/testsuite/model/src/test/java/org/keycloak/testsuite/model/session/UserSessionConcurrencyTest.java +++ b/testsuite/model/src/test/java/org/keycloak/testsuite/model/session/UserSessionConcurrencyTest.java @@ -31,11 +31,11 @@ import org.keycloak.testsuite.model.RequireProvider; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; -import java.util.function.Consumer; import java.util.stream.IntStream; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.aMapWithSize; +import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.startsWith; @RequireProvider(UserSessionProvider.class) @@ -44,8 +44,6 @@ public class UserSessionConcurrencyTest extends KeycloakModelTest { private String realmId; private static final int CLIENTS_COUNT = 10; - private static final ThreadLocal wasWriting = ThreadLocal.withInitial(() -> false); - @Override public void createEnvironment(KeycloakSession s) { RealmModel realm = createRealm(s, "test"); @@ -92,7 +90,6 @@ public class UserSessionConcurrencyTest extends KeycloakModelTest { UserSessionModel uSession = session.sessions().getUserSession(realm, uId); AuthenticatedClientSessionModel cSession = uSession.getAuthenticatedClientSessionByClient(client.getId()); if (cSession == null) { - wasWriting.set(true); cSession = session.sessions().createClientSession(realm, client, uSession); } @@ -103,7 +100,8 @@ public class UserSessionConcurrencyTest extends KeycloakModelTest { cdl.countDown(); }})); - cdl.await(10, TimeUnit.SECONDS); + assertThat(cdl.await(10, TimeUnit.SECONDS), is(true)); + withRealm(this.realmId, (session, realm) -> { UserSessionModel uSession = session.sessions().getUserSession(realm, uId); assertThat(uSession.getAuthenticatedClientSessions(), aMapWithSize(CLIENTS_COUNT)); @@ -118,7 +116,7 @@ public class UserSessionConcurrencyTest extends KeycloakModelTest { return null; }); - inComittedTransaction((Consumer) session -> { + inComittedTransaction(session -> { RealmModel realm = session.realms().getRealm(realmId); session.getContext().setRealm(realm); session.realms().removeRealm(realmId);