Added @retryable to avoid deadlocks.

oharsta · oharsta · commit 01f9785527c7 · 2025-06-03T13:47:35.000+02:00
Multiple threads trying to create role's with the
same subset of applications under heavy load
causes SQLTransactionRollbackException errors.

To avoid this we retry the updateRole for max 3
times and also use additional index to prevent
full-table scans.
diff --git a/server/pom.xml b/server/pom.xml
@@ -89,6 +89,11 @@
             <artifactId>mariadb-java-client</artifactId>
             <version>3.5.3</version>
         </dependency>
+        <dependency>
+            <groupId>org.springframework.retry</groupId>
+            <artifactId>spring-retry</artifactId>
+            <version>2.0.12</version>
+        </dependency>
         <dependency>
             <groupId>com.github.spullara.mustache.java</groupId>
             <artifactId>compiler</artifactId>
diff --git a/server/src/main/java/access/api/RoleController.java b/server/src/main/java/access/api/RoleController.java
@@ -28,11 +28,14 @@
 import org.springframework.data.domain.Sort;
 import org.springframework.http.MediaType;
 import org.springframework.http.ResponseEntity;
+import org.springframework.retry.annotation.Backoff;
+import org.springframework.retry.annotation.Retryable;
 import org.springframework.transaction.annotation.Transactional;
 import org.springframework.util.StringUtils;
 import org.springframework.validation.annotation.Validated;
 import org.springframework.web.bind.annotation.*;
 
+import java.sql.SQLTransactionRollbackException;
 import java.util.*;
 import java.util.stream.Collectors;
 
@@ -164,6 +167,11 @@ public ResponseEntity<Role> newRole(@Validated @RequestBody Role role,
     }
 
     @PutMapping("")
+    @Retryable(
+            value = { SQLTransactionRollbackException.class },
+            maxAttempts = 3,
+            backoff = @Backoff(delay = 1000)
+    )
     public ResponseEntity<Role> updateRole(@Validated @RequestBody Role role,
                                            @Parameter(hidden = true) User user) {
         LOG.debug(String.format("PUT /roles/ for user %s", user.getEduPersonPrincipalName()));
diff --git a/server/src/main/java/access/api/RoleOperations.java b/server/src/main/java/access/api/RoleOperations.java
@@ -41,13 +41,14 @@ public void syncRoleApplicationUsages(Role role) {
                 .map(applicationUsageFromClient -> {
                     Application application = applicationUsageFromClient.getApplication();
                     Application applicationFromDB = applicationRepository
-                            .findByManageIdAndManageType(application.getManageId(), application.getManageType())
+                            .findByManageIdAndManageTypeOrderById(application.getManageId(), application.getManageType())
                             .orElseGet(() -> applicationRepository.save(application));
-                    ApplicationUsage applicationUsageFromDB = applicationUsageRepository.findByRoleIdAndApplicationManageIdAndApplicationManageType(
-                            role.getId(),
-                            applicationFromDB.getManageId(),
-                            applicationFromDB.getManageType()
-                    ).orElseGet(() -> new ApplicationUsage(applicationFromDB, applicationUsageFromClient.getLandingPage()));
+                    ApplicationUsage applicationUsageFromDB = applicationUsageRepository
+                            .findByRoleIdAndApplicationManageIdAndApplicationManageTypeOrderByApplicationId(
+                                    role.getId(),
+                                    applicationFromDB.getManageId(),
+                                    applicationFromDB.getManageType()
+                            ).orElseGet(() -> new ApplicationUsage(applicationFromDB, applicationUsageFromClient.getLandingPage()));
                     applicationUsageFromDB.setLandingPage(applicationUsageFromClient.getLandingPage());
                     return applicationUsageFromDB;
                 })
diff --git a/server/src/main/java/access/repository/ApplicationRepository.java b/server/src/main/java/access/repository/ApplicationRepository.java
@@ -2,8 +2,6 @@
 
 import access.manage.EntityType;
 import access.model.Application;
-import org.springframework.data.domain.Page;
-import org.springframework.data.domain.Pageable;
 import org.springframework.data.jpa.repository.JpaRepository;
 import org.springframework.data.jpa.repository.Query;
 import org.springframework.stereotype.Repository;
@@ -15,7 +13,7 @@
 @Repository
 public interface ApplicationRepository extends JpaRepository<Application, Long> {
 
-    Optional<Application> findByManageIdAndManageType(String manageId, EntityType manageType);
+    Optional<Application> findByManageIdAndManageTypeOrderById(String manageId, EntityType manageType);
 
     @Query(value = """
             SELECT count(r.id) as role_count, apps.manage_id as manage_id FROM roles r
diff --git a/server/src/main/java/access/repository/ApplicationUsageRepository.java b/server/src/main/java/access/repository/ApplicationUsageRepository.java
@@ -10,7 +10,7 @@
 @Repository
 public interface ApplicationUsageRepository extends JpaRepository<ApplicationUsage, Long> {
 
-    Optional<ApplicationUsage> findByRoleIdAndApplicationManageIdAndApplicationManageType(
+    Optional<ApplicationUsage> findByRoleIdAndApplicationManageIdAndApplicationManageTypeOrderByApplicationId(
             Long roleId, String manageId, EntityType manageType);
 
 }
diff --git a/server/src/main/java/access/seed/PerformanceSeed.java b/server/src/main/java/access/seed/PerformanceSeed.java
@@ -12,7 +12,6 @@
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.stereotype.Component;
 
-import javax.sql.DataSource;
 import java.time.Instant;
 import java.time.temporal.ChronoUnit;
 import java.util.*;
@@ -178,7 +177,7 @@ private Role createRole(List<Map<String, Object>> providers, String institutionG
     private Application application(ManageIdentifier manageIdentifier) {
         String manageId = manageIdentifier.manageId();
         EntityType entityType = manageIdentifier.manageType();
-        return applicationRepository.findByManageIdAndManageType(manageId, entityType).
+        return applicationRepository.findByManageIdAndManageTypeOrderById(manageId, entityType).
                 orElseGet(() -> applicationRepository.save(new Application(manageId, entityType)));
     }
 
diff --git a/server/src/main/java/access/teams/TeamsController.java b/server/src/main/java/access/teams/TeamsController.java
@@ -95,7 +95,7 @@ public ResponseEntity<Map<String, Integer>> migrateTeam(@RequestBody Team team)
         Set<ApplicationUsage> applicationUsages = team.getApplications().stream()
                 .map(applicationFromTeams -> {
                     Optional<Application> optionalApplication = applicationRepository
-                            .findByManageIdAndManageType(applicationFromTeams.getManageId(), applicationFromTeams.getManageType());
+                            .findByManageIdAndManageTypeOrderById(applicationFromTeams.getManageId(), applicationFromTeams.getManageType());
                     Application applicationFromDB = optionalApplication
                             .orElseGet(() -> {
                                 // ID is also serialized from database teams object, need to prevent StaleObjectStateException
diff --git a/server/src/main/resources/db/mysql/migration/V44_0__application_indexes.sql b/server/src/main/resources/db/mysql/migration/V44_0__application_indexes.sql
@@ -0,0 +1,3 @@
+ALTER TABLE `applications`
+    ADD INDEX `manage_id_index` (`manage_id`),
+    ADD INDEX `manage_type_index` (`manage_type`);
diff --git a/server/src/test/java/access/AbstractTest.java b/server/src/test/java/access/AbstractTest.java
@@ -560,7 +560,7 @@ protected UnaryOperator<Map<String, Object>> institutionalAdminEntitlementOperat
     }
 
     protected Set<ApplicationUsage> application(String manageId, EntityType entityType) {
-        Application application = applicationRepository.findByManageIdAndManageType(manageId, entityType).
+        Application application = applicationRepository.findByManageIdAndManageTypeOrderById(manageId, entityType).
                 orElseGet(() -> applicationRepository.save(new Application(manageId, entityType)));
         return Set.of(new ApplicationUsage(application, "http://landingpage.com"));
     }
@@ -608,7 +608,7 @@ private void doSeed() {
                         new Application("3", EntityType.SAML20_SP),
                         new Application("6", EntityType.OIDC10_RP))
                 .stream()
-                .map(app -> applicationRepository.findByManageIdAndManageType(app.getManageId(), app.getManageType()).
+                .map(app -> applicationRepository.findByManageIdAndManageTypeOrderById(app.getManageId(), app.getManageType()).
                         orElseGet(() -> applicationRepository.save(new Application(app.getManageId(), app.getManageType()))))
                 .collect(Collectors.toSet());
         Set<ApplicationUsage> applicationUsages = applications.stream().map(application -> new ApplicationUsage(application, "https://landingpage.com")).collect(Collectors.toSet());
diff --git a/server/src/test/java/access/api/RoleControllerTest.java b/server/src/test/java/access/api/RoleControllerTest.java
@@ -122,7 +122,7 @@ void createInvalidLandingPage() throws Exception {
         stubForManageProvisioning(List.of());
         stubForManageProvidersAllowedByIdP(ORGANISATION_GUID);
 
-        Application application = applicationRepository.findByManageIdAndManageType("1", EntityType.SAML20_SP).
+        Application application = applicationRepository.findByManageIdAndManageTypeOrderById("1", EntityType.SAML20_SP).
                 orElseGet(() -> applicationRepository.save(new Application("1", EntityType.SAML20_SP)));
         Set<ApplicationUsage> applications = Set.of(new ApplicationUsage(application, "bogus"));
         AccessCookieFilter accessCookieFilter = openIDConnectFlow("/api/v1/users/login", INSTITUTION_ADMIN_SUB);
diff --git a/server/src/test/java/access/repository/ApplicationRepositoryTest.java b/server/src/test/java/access/repository/ApplicationRepositoryTest.java
@@ -13,8 +13,8 @@
 class ApplicationRepositoryTest extends AbstractTest {
 
     @Test
-    void findByManageIdAndManageType() {
-        Application application = applicationRepository.findByManageIdAndManageType("2", EntityType.SAML20_SP).get();
+    void findByManageIdAndManageTypeOrderById() {
+        Application application = applicationRepository.findByManageIdAndManageTypeOrderById("2", EntityType.SAML20_SP).get();
         assertEquals("2", application.getManageId());
     }
 
diff --git a/server/src/test/java/access/repository/ApplicationUsageRepositoryTest.java b/server/src/test/java/access/repository/ApplicationUsageRepositoryTest.java
@@ -14,25 +14,25 @@
 class ApplicationUsageRepositoryTest extends AbstractTest {
 
     @Test
-    void findByRoleIdAndApplicationManageIdAndApplicationManageType() {
+    void findByRoleIdAndApplicationManageIdAndApplicationManageTypeOrderByApplicationId() {
         Role role = roleRepository.findByName("wiki").get();
         ApplicationUsage applicationUsage = role.getApplicationUsages().iterator().next();
         Application application = applicationUsage.getApplication();
-        Optional<ApplicationUsage> optionalApplicationUsage = applicationUsageRepository.findByRoleIdAndApplicationManageIdAndApplicationManageType(
+        Optional<ApplicationUsage> optionalApplicationUsage = applicationUsageRepository.findByRoleIdAndApplicationManageIdAndApplicationManageTypeOrderByApplicationId(
                 role.getId(),
                 application.getManageId(),
                 application.getManageType()
         );
         assertTrue(optionalApplicationUsage.isPresent());
 
-        optionalApplicationUsage = applicationUsageRepository.findByRoleIdAndApplicationManageIdAndApplicationManageType(
+        optionalApplicationUsage = applicationUsageRepository.findByRoleIdAndApplicationManageIdAndApplicationManageTypeOrderByApplicationId(
                 role.getId(),
                 UUID.randomUUID().toString(),
                 application.getManageType()
         );
         assertTrue(optionalApplicationUsage.isEmpty());
 
-        optionalApplicationUsage = applicationUsageRepository.findByRoleIdAndApplicationManageIdAndApplicationManageType(
+        optionalApplicationUsage = applicationUsageRepository.findByRoleIdAndApplicationManageIdAndApplicationManageTypeOrderByApplicationId(
                 null,
                 application.getManageId(),
                 application.getManageType()

Original file line number	Diff line number	Diff line change
`@@ -10,7 +10,7 @@`
`10`	`10`	`@Repository`
`11`	`11`	`public interface ApplicationUsageRepository extends JpaRepository<ApplicationUsage, Long> {`
`12`	`12`
`13`		`- Optional<ApplicationUsage> findByRoleIdAndApplicationManageIdAndApplicationManageType(`
	`13`	`+ Optional<ApplicationUsage> findByRoleIdAndApplicationManageIdAndApplicationManageTypeOrderByApplicationId(`
`14`	`14`	`Long roleId, String manageId, EntityType manageType);`
`15`	`15`
`16`	`16`	`}`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	+ALTER TABLE `applications`
	`2`	+ ADD INDEX `manage_id_index` (`manage_id`),
	`3`	+ ADD INDEX `manage_type_index` (`manage_type`);