Removal of retry blocks introduced for CRDB

Closes #24095

Signed-off-by: vramik <vramik@redhat.com>
Signed-off-by: Alexander Schwartz <aschwart@redhat.com>
Co-authored-by: Alexander Schwartz <aschwart@redhat.com>

server-spi-private/src/main/java/org/keycloak/models/utils/KeycloakModelUtils.java

@@ -62,13 +62,11 @@ import java.security.KeyPair;
 import java.security.PrivateKey;
 import java.security.PublicKey;
 import java.security.cert.X509Certificate;
-import java.sql.SQLException;
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
-import java.util.Random;
 import java.util.Set;
 import java.util.UUID;
 import java.util.stream.Collectors;
@@ -390,78 +388,6 @@ public final class KeycloakModelUtils {
         return result;
     }
 
-    /**
-     * Creates a new {@link KeycloakSession} and runs the specified callable in a new transaction. If the transaction fails
-     * with a SQL retriable error, the method re-executes the specified callable until it either succeeds or the maximum number
-     * of attempts is reached, leaving some increasing random delay milliseconds between the invocations. It uses the exponential
-     * backoff + jitter algorithm to compute the delay, which is limited to {@code attemptsCount * retryIntervalMillis}.
-     * More details https://aws.amazon.com/blogs/architecture/exponential-backoff-and-jitter/
-     *
-     * @param factory a reference to the {@link KeycloakSessionFactory}.
-     * @param callable a reference to the {@link KeycloakSessionTaskWithResult} that will be executed in a retriable way.
-     * @param attemptsCount the maximum number of attempts to execute the callable.
-     * @param retryIntervalMillis the base interval value in millis used to compute the delay.
-     * @param <V> the type returned by the callable.
-     * @return the value computed by the callable.
-     */
-    public static <V> V runJobInRetriableTransaction(final KeycloakSessionFactory factory, final KeycloakSessionTaskWithResult<V> callable,
-                                                     final int attemptsCount, final int retryIntervalMillis) {
-        int retryCount = 0;
-        Random rand = new Random();
-        while (true) {
-            try (KeycloakSession session = factory.create()) {
-                session.getTransactionManager().begin();
-                return callable.run(session);
-            } catch (RuntimeException re) {
-                if (isExceptionRetriable(re) && ++retryCount < attemptsCount) {
-                    int delay = Math.min(retryIntervalMillis * attemptsCount, (1 << retryCount) * retryIntervalMillis)
-                            + rand.nextInt(retryIntervalMillis);
-                    logger.debugf("Caught retriable exception, retrying request. Retry count = %s, retry delay = %s", retryCount, delay);
-                    try {
-                        Thread.sleep(delay);
-                    } catch (InterruptedException ie) {
-                        ie.addSuppressed(re);
-                        throw new RuntimeException(ie);
-                    }
-                } else {
-                    if (retryCount == attemptsCount) {
-                        logger.debug("Exhausted all retry attempts for request.");
-                        throw new RuntimeException("retries exceeded", re);
-                    }
-                    throw re;
-                }
-            }
-        }
-    }
-
-    /**
-     * Checks if the specified exception is retriable or not. A retriable exception must be an instance of {@code SQLException}
-     * and must have a 40001 SQL retriable state. This is a standard SQL state as defined in SQL standard, and across the
-     * implementations its meaning boils down to "deadlock" (applies to Postgres, MSSQL, Oracle, MySQL, and others).
-     *
-     * @param exception the exception to be checked.
-     * @return {@code true} if the exception is retriable; {@code false} otherwise.
-     */
-    public static boolean isExceptionRetriable(final Throwable exception) {
-        Objects.requireNonNull(exception);
-        // first find the root cause and check if it is a SQLException
-        Throwable rootCause = exception;
-        while (rootCause.getCause() != null && rootCause.getCause() != rootCause) {
-            rootCause = rootCause.getCause();
-        }
-        // JTA transaction handler might add multiple suppressed exceptions to the root cause, evaluate each of those
-        for (Throwable suppressed : rootCause.getSuppressed()) {
-            if (isExceptionRetriable(suppressed)) {
-                return true;
-            }
-        };
-        if (rootCause instanceof SQLException) {
-            // check if the exception state is a recoverable one (40001)
-            return "40001".equals(((SQLException) rootCause).getSQLState());
-        }
-        return false;
-    }
-
     /**
      * Wrap given runnable job into KeycloakTransaction. Set custom timeout for the JTA transaction (in case we're in the environment with JTA enabled)
      *

services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java

@@ -39,7 +39,6 @@ import org.keycloak.models.UserModel;
 import org.keycloak.models.UserSessionModel;
 import org.keycloak.models.utils.AuthenticationFlowResolver;
 import org.keycloak.models.utils.FormMessage;
-import org.keycloak.models.utils.KeycloakModelUtils;
 import org.keycloak.protocol.LoginProtocol;
 import org.keycloak.protocol.LoginProtocol.Error;
 import org.keycloak.protocol.oidc.TokenManager;
@@ -47,7 +46,6 @@ import org.keycloak.services.ErrorPage;
 import org.keycloak.services.ErrorPageException;
 import org.keycloak.services.ServicesLogger;
 import org.keycloak.services.managers.AuthenticationManager;
-import org.keycloak.services.managers.AuthenticationSessionManager;
 import org.keycloak.services.managers.BruteForceProtector;
 import org.keycloak.services.managers.ClientSessionCode;
 import org.keycloak.services.managers.UserSessionManager;
@@ -829,14 +827,6 @@ public class AuthenticationProcessor {
                 if (e.getResponse() != null) return e.getResponse();
                 return ErrorPage.error(session, authenticationSession, Response.Status.BAD_REQUEST, Messages.INVALID_USER);
             }
-
-        } else if (KeycloakModelUtils.isExceptionRetriable(failure)) {
-            // let calling code decide if whole action should be retried.
-            if (failure instanceof RuntimeException) {
-                throw (RuntimeException) failure;
-            } else {
-                throw new RuntimeException(failure);
-            }
         } else {
             ServicesLogger.LOGGER.failedAuthentication(failure);
             event.error(Errors.INVALID_USER_CREDENTIALS);

services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java

@@ -21,7 +21,6 @@ import org.jboss.logging.Logger;
 import org.keycloak.OAuth2Constants;
 import org.keycloak.authentication.AuthenticationProcessor;
 import org.keycloak.common.Profile;
-import org.keycloak.common.util.ResponseSessionTask;
 import org.keycloak.constants.AdapterConstants;
 import org.keycloak.events.Details;
 import org.keycloak.events.Errors;
@@ -32,7 +31,6 @@ import org.keycloak.models.AuthenticationFlowModel;
 import org.keycloak.models.ClientModel;
 import org.keycloak.models.Constants;
 import org.keycloak.models.KeycloakSession;
-import org.keycloak.models.utils.KeycloakModelUtils;
 import org.keycloak.protocol.AuthorizationEndpointBase;
 import org.keycloak.protocol.oidc.OIDCAdvancedConfigWrapper;
 import org.keycloak.protocol.oidc.OIDCLoginProtocol;
@@ -102,22 +100,17 @@ public class AuthorizationEndpoint extends AuthorizationEndpointBase {
         event.event(EventType.LOGIN);
     }
 
-    private AuthorizationEndpoint(final KeycloakSession session, final EventBuilder event, final Action action) {
-        this(session, event);
-        this.action = action;
-    }
-
     @POST
     @Consumes(MediaType.APPLICATION_FORM_URLENCODED)
     public Response buildPost() {
         logger.trace("Processing @POST request");
-        return processInRetriableTransaction(httpRequest.getDecodedFormParameters());
+        return process(httpRequest.getDecodedFormParameters());
     }
 
     @GET
     public Response buildGet() {
         logger.trace("Processing @GET request");
-        return processInRetriableTransaction(session.getContext().getUri().getQueryParameters());
+        return process(session.getContext().getUri().getQueryParameters());
     }
 
     /**
@@ -131,28 +124,7 @@ public class AuthorizationEndpoint extends AuthorizationEndpointBase {
         return new DeviceEndpoint(session, event);
     }
 
-    /**
+    private Response process(final MultivaluedMap<String, String> params) {
-     * Process the request in a retriable transaction.
-     */
-    private Response processInRetriableTransaction(final MultivaluedMap<String, String> formParameters) {
-        if (Profile.isFeatureEnabled(Profile.Feature.MAP_STORAGE)) {
-            return KeycloakModelUtils.runJobInRetriableTransaction(session.getKeycloakSessionFactory(), new ResponseSessionTask(session) {
-                @Override
-                public Response runInternal(KeycloakSession session) {
-                    session.getContext().getHttpResponse().setWriteCookiesOnTransactionComplete();
-                    // create another instance of the endpoint to isolate each run.
-                    AuthorizationEndpoint other = new AuthorizationEndpoint(session,
-                            new EventBuilder(session.getContext().getRealm(), session, clientConnection), action);
-                    // process the request in the created instance.
-                    return other.process(formParameters);
-                }
-            }, 10, 100);
-        } else {
-            return process(formParameters);
-        }
-    }
-
-    private Response process(MultivaluedMap<String, String> params) {
         String clientId = AuthorizationEndpointRequestParserProcessor.getClientId(event, session, params);
 
         checkSsl();

services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java

@@ -31,7 +31,6 @@ import org.keycloak.common.Profile;
 import org.keycloak.common.VerificationException;
 import org.keycloak.common.constants.ServiceAccountConstants;
 import org.keycloak.common.util.KeycloakUriBuilder;
-import org.keycloak.common.util.ResponseSessionTask;
 import org.keycloak.constants.AdapterConstants;
 import org.keycloak.events.Details;
 import org.keycloak.events.Errors;
@@ -49,7 +48,6 @@ import org.keycloak.models.RealmModel;
 import org.keycloak.models.UserModel;
 import org.keycloak.models.UserSessionModel;
 import org.keycloak.models.utils.AuthenticationFlowResolver;
-import org.keycloak.models.utils.KeycloakModelUtils;
 import org.keycloak.protocol.oidc.OIDCAdvancedConfigWrapper;
 import org.keycloak.protocol.oidc.OIDCLoginProtocol;
 import org.keycloak.protocol.oidc.TokenExchangeContext;
@@ -178,25 +176,6 @@ public class TokenEndpoint {
     @Consumes(MediaType.APPLICATION_FORM_URLENCODED)
     @POST
     public Response processGrantRequest() {
-        if (Profile.isFeatureEnabled(Profile.Feature.MAP_STORAGE)) {
-            // grant request needs to be run in a retriable transaction as concurrent execution of this action can lead to
-            // exceptions on DBs with SERIALIZABLE isolation level.
-            return KeycloakModelUtils.runJobInRetriableTransaction(session.getKeycloakSessionFactory(), new ResponseSessionTask(session) {
-                @Override
-                public Response runInternal(KeycloakSession session) {
-                    // create another instance of the endpoint to isolate each run.
-                    TokenEndpoint other = new TokenEndpoint(session, tokenManager,
-                            new EventBuilder(session.getContext().getRealm(), session, clientConnection));
-                    // process the request in the created instance.
-                    return other.processGrantRequestInternal();
-                }
-            }, 10, 100);
-        } else {
-            return processGrantRequestInternal();
-        }
-    }
-
-    private Response processGrantRequestInternal() {
         cors = Cors.add(request).auth().allowedMethods("POST").auth().exposedHeaders(Cors.ACCESS_CONTROL_ALLOW_METHODS);
 
         MultivaluedMap<String, String> formParameters = request.getDecodedFormParameters();

services/src/main/java/org/keycloak/services/resources/LoginActionsService.java

@@ -17,8 +17,6 @@
 package org.keycloak.services.resources;
 
 import org.jboss.logging.Logger;
-import org.keycloak.common.Profile;
-import org.keycloak.common.util.ResponseSessionTask;
 import org.keycloak.forms.login.LoginFormsProvider;
 import org.keycloak.forms.login.MessageType;
 import org.keycloak.forms.login.freemarker.DetachedInfoStateChecker;
@@ -311,25 +309,7 @@ public class LoginActionsService {
                                  @QueryParam(Constants.EXECUTION) String execution,
                                  @QueryParam(Constants.CLIENT_ID) String clientId,
                                  @QueryParam(Constants.TAB_ID) String tabId) {
-        if (Profile.isFeatureEnabled(Profile.Feature.MAP_STORAGE)) {
-            return KeycloakModelUtils.runJobInRetriableTransaction(session.getKeycloakSessionFactory(), new ResponseSessionTask(session) {
-                @Override
-                public Response runInternal(KeycloakSession session) {
-                    // create another instance of the endpoint to isolate each run.
-                    session.getContext().getHttpResponse().setWriteCookiesOnTransactionComplete();
-                    LoginActionsService other = new LoginActionsService(session, new EventBuilder(session.getContext().getRealm(), session, clientConnection));
-                    // process the request in the created instance.
-                    return other.authenticateInternal(authSessionId, code, execution, clientId, tabId);
-                }
-            }, 10, 100);
-        } else {
-            return authenticateInternal(authSessionId, code, execution, clientId, tabId);
-        }
 
-    }
-
-    private Response authenticateInternal(final String authSessionId, final String code, final String execution,
-                                          final String clientId, final String tabId) {
         event.event(EventType.LOGIN);
 
         SessionCodeChecks checks = checksForCode(authSessionId, code, execution, clientId, tabId, AUTHENTICATE_PATH);

services/src/main/java/org/keycloak/services/resources/admin/ComponentResource.java

@@ -31,7 +31,6 @@ import org.keycloak.events.admin.OperationType;
 import org.keycloak.events.admin.ResourceType;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.RealmModel;
-import org.keycloak.models.utils.KeycloakModelUtils;
 import org.keycloak.models.utils.ModelToRepresentation;
 import org.keycloak.models.utils.RepresentationToModel;
 import org.keycloak.models.utils.StripSecretsUtils;
@@ -44,7 +43,6 @@ import org.keycloak.representations.idm.ConfigPropertyRepresentation;
 import org.keycloak.services.ErrorResponse;
 import org.keycloak.services.resources.KeycloakOpenAPI;
 import org.keycloak.services.resources.admin.permissions.AdminPermissionEvaluator;
-import org.keycloak.utils.LockObjectsForModification;
 
 import jakarta.ws.rs.BadRequestException;
 import jakarta.ws.rs.Consumes;
@@ -136,22 +134,19 @@ public class ComponentResource {
     @Operation()
     public Response create(ComponentRepresentation rep) {
         auth.realm().requireManageRealm();
-        return KeycloakModelUtils.runJobInRetriableTransaction(session.getKeycloakSessionFactory(), kcSession -> {
-            RealmModel realmModel = LockObjectsForModification.lockRealmsForModification(kcSession, () -> kcSession.realms().getRealm(realm.getId()));
         try {
-                ComponentModel model = RepresentationToModel.toModel(kcSession, rep);
+            ComponentModel model = RepresentationToModel.toModel(session, rep);
-                if (model.getParentId() == null) model.setParentId(realmModel.getId());
+            if (model.getParentId() == null) model.setParentId(realm.getId());
 
-                model = realmModel.addComponentModel(model);
+            model = realm.addComponentModel(model);
 
-                adminEvent.operation(OperationType.CREATE).resourcePath(kcSession.getContext().getUri(), model.getId()).representation(StripSecretsUtils.strip(kcSession, rep)).success();
+            adminEvent.operation(OperationType.CREATE).resourcePath(session.getContext().getUri(), model.getId()).representation(StripSecretsUtils.strip(session, rep)).success();
-                return Response.created(kcSession.getContext().getUri().getAbsolutePathBuilder().path(model.getId()).build()).build();
+            return Response.created(session.getContext().getUri().getAbsolutePathBuilder().path(model.getId()).build()).build();
         } catch (ComponentValidationException e) {
             return localizedErrorResponse(e);
         } catch (IllegalArgumentException e) {
             throw new BadRequestException(e);
         }
-        }, 10, 100);
     }
 
     @GET
@@ -177,23 +172,20 @@ public class ComponentResource {
     @Operation()
     public Response updateComponent(@PathParam("id") String id, ComponentRepresentation rep) {
         auth.realm().requireManageRealm();
-        return KeycloakModelUtils.runJobInRetriableTransaction(session.getKeycloakSessionFactory(), kcSession -> {
-            RealmModel realmModel = LockObjectsForModification.lockRealmsForModification(kcSession, () -> kcSession.realms().getRealm(realm.getId()));
         try {
-                ComponentModel model = realmModel.getComponent(id);
+            ComponentModel model = realm.getComponent(id);
             if (model == null) {
                 throw new NotFoundException("Could not find component");
             }
-                RepresentationToModel.updateComponent(kcSession, rep, model, false);
+            RepresentationToModel.updateComponent(session, rep, model, false);
-                adminEvent.operation(OperationType.UPDATE).resourcePath(kcSession.getContext().getUri()).representation(StripSecretsUtils.strip(kcSession, rep)).success();
+            adminEvent.operation(OperationType.UPDATE).resourcePath(session.getContext().getUri()).representation(StripSecretsUtils.strip(session, rep)).success();
-                realmModel.updateComponent(model);
+            realm.updateComponent(model);
             return Response.noContent().build();
         } catch (ComponentValidationException e) {
             return localizedErrorResponse(e);
         } catch (IllegalArgumentException e) {
             throw new BadRequestException();
         }
-        }, 10, 100);
     }
     @DELETE
     @Path("{id}")
@@ -201,17 +193,12 @@ public class ComponentResource {
     @Operation()
     public void removeComponent(@PathParam("id") String id) {
         auth.realm().requireManageRealm();
-        KeycloakModelUtils.runJobInRetriableTransaction(session.getKeycloakSessionFactory(), kcSession -> {
+        ComponentModel model = realm.getComponent(id);
-            RealmModel realmModel = LockObjectsForModification.lockRealmsForModification(kcSession, () -> kcSession.realms().getRealm(realm.getId()));
-
-            ComponentModel model = realmModel.getComponent(id);
         if (model == null) {
             throw new NotFoundException("Could not find component");
         }
-            adminEvent.operation(OperationType.DELETE).resourcePath(kcSession.getContext().getUri()).success();
+        adminEvent.operation(OperationType.DELETE).resourcePath(session.getContext().getUri()).success();
-            realmModel.removeComponent(model);
+        realm.removeComponent(model);
-            return null;
-        }, 10 , 100);
     }
 
     private Response localizedErrorResponse(ComponentValidationException cve) {

testsuite/model/src/test/java/org/keycloak/testsuite/model/KeycloakModelTest.java

@@ -116,8 +116,6 @@ import org.keycloak.models.DeploymentStateProviderFactory;
  * @author hmlnarik
  */
 public abstract class KeycloakModelTest {
-    public static final String KEYCLOAK_MODELTESTS_RETRY_TRANSACTIONS = "keycloak.modeltests.retry-transactions";
-
     private static final Logger LOG = Logger.getLogger(KeycloakModelParameters.class);
     private static final AtomicInteger FACTORY_COUNT = new AtomicInteger();
     protected final Logger log = Logger.getLogger(getClass());
@@ -571,22 +569,6 @@ public abstract class KeycloakModelTest {
     }
 
     protected <T, R> R inComittedTransaction(T parameter, BiFunction<KeycloakSession, T, R> what, BiConsumer<KeycloakSession, T> onCommit, BiConsumer<KeycloakSession, T> onRollback) {
-        if (Boolean.parseBoolean(System.getProperty(KEYCLOAK_MODELTESTS_RETRY_TRANSACTIONS, "false"))) {
-            return KeycloakModelUtils.runJobInRetriableTransaction(getFactory(), session -> {
-                session.getTransactionManager().enlistAfterCompletion(new AbstractKeycloakTransaction() {
-                    @Override
-                    protected void commitImpl() {
-                        if (onCommit != null) { onCommit.accept(session, parameter); }
-                    }
-
-                    @Override
-                    protected void rollbackImpl() {
-                        if (onRollback != null) { onRollback.accept(session, parameter); }
-                    }
-                });
-                return what.apply(session, parameter);
-            }, 5, 100);
-        } else {
         return KeycloakModelUtils.runJobInTransactionWithResult(getFactory(), session -> {
             session.getTransactionManager().enlistAfterCompletion(new AbstractKeycloakTransaction() {
                 @Override
@@ -602,7 +584,6 @@ public abstract class KeycloakModelTest {
             return what.apply(session, parameter);
         });
     }
-    }
 
     /**
      * Convenience method for {@link #inComittedTransaction(java.util.function.Consumer)} that