Closes Taskana#2374: Deadlock when creating HistoryEvents from many connections simultaneously

-Write operations are now performed with the regular TaskanEngine and its' SqlSession and TransactionFactory which provides the needed transactionality and doesn't open multiple connections
-Read operations are still performed by the TaskanaHistoryEngine

Author: Jörg Heffner

history/taskana-simplehistory-provider/src/main/java/pro/taskana/simplehistory/impl/SimpleHistoryServiceImpl.java

@@ -1,14 +1,18 @@
 package pro.taskana.simplehistory.impl;
 
+import java.lang.reflect.Field;
 import java.sql.SQLException;
 import java.time.Instant;
 import java.util.List;
+import org.apache.ibatis.session.SqlSession;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import pro.taskana.common.api.TaskanaEngine;
 import pro.taskana.common.api.TaskanaRole;
 import pro.taskana.common.api.exceptions.InvalidArgumentException;
 import pro.taskana.common.api.exceptions.NotAuthorizedException;
+import pro.taskana.common.api.exceptions.SystemException;
+import pro.taskana.common.internal.TaskanaEngineImpl;
 import pro.taskana.simplehistory.impl.classification.ClassificationHistoryEventMapper;
 import pro.taskana.simplehistory.impl.classification.ClassificationHistoryQuery;
 import pro.taskana.simplehistory.impl.task.TaskHistoryEventMapper;
@@ -43,61 +47,74 @@ public void initialize(TaskanaEngine taskanaEngine) {
           taskanaEngine.getConfiguration().getSchemaName());
     }
 
-    this.taskHistoryEventMapper =
-        this.taskanaHistoryEngine.getSqlSession().getMapper(TaskHistoryEventMapper.class);
-    this.workbasketHistoryEventMapper =
-        this.taskanaHistoryEngine.getSqlSession().getMapper(WorkbasketHistoryEventMapper.class);
-    this.classificationHistoryEventMapper =
-        this.taskanaHistoryEngine.getSqlSession().getMapper(ClassificationHistoryEventMapper.class);
-    this.userMapper = taskanaHistoryEngine.getSqlSession().getMapper(UserMapper.class);
+    Field sessionManager = null;
+    try {
+      sessionManager = TaskanaEngineImpl.class.getDeclaredField("sessionManager");
+      sessionManager.setAccessible(true);
+    } catch (NoSuchFieldException e) {
+      throw new SystemException("SQL Session could not be retrieved. Aborting Startup");
+    }
+    try {
+      SqlSession sqlSession = (SqlSession) sessionManager.get(taskanaEngine);
+      if (!sqlSession
+          .getConfiguration()
+          .getMapperRegistry()
+          .hasMapper(TaskHistoryEventMapper.class)) {
+        sqlSession.getConfiguration().addMapper(TaskHistoryEventMapper.class);
+      }
+      if (!sqlSession
+          .getConfiguration()
+          .getMapperRegistry()
+          .hasMapper(WorkbasketHistoryEventMapper.class)) {
+
+        sqlSession.getConfiguration().addMapper(WorkbasketHistoryEventMapper.class);
+      }
+      if (!sqlSession
+          .getConfiguration()
+          .getMapperRegistry()
+          .hasMapper(ClassificationHistoryEventMapper.class)) {
+
+        sqlSession.getConfiguration().addMapper(ClassificationHistoryEventMapper.class);
+      }
+
+      this.taskHistoryEventMapper = sqlSession.getMapper(TaskHistoryEventMapper.class);
+      this.workbasketHistoryEventMapper = sqlSession.getMapper(WorkbasketHistoryEventMapper.class);
+      this.classificationHistoryEventMapper =
+          sqlSession.getMapper(ClassificationHistoryEventMapper.class);
+      this.userMapper = sqlSession.getMapper(UserMapper.class);
+    } catch (IllegalAccessException e) {
+      throw new RuntimeException(e);
+    }
   }
 
   @Override
   public void create(TaskHistoryEvent event) {
-    try {
-      taskanaHistoryEngine.openConnection();
-      if (event.getCreated() == null) {
-        Instant now = Instant.now();
-        event.setCreated(now);
-      }
-      taskHistoryEventMapper.insert(event);
-    } catch (SQLException e) {
-      LOGGER.error("Error while inserting task history event into database", e);
-    } finally {
-      taskanaHistoryEngine.returnConnection();
+
+    if (event.getCreated() == null) {
+      Instant now = Instant.now();
+      event.setCreated(now);
     }
+    taskHistoryEventMapper.insert(event);
   }
 
   @Override
   public void create(WorkbasketHistoryEvent event) {
-    try {
-      taskanaHistoryEngine.openConnection();
-      if (event.getCreated() == null) {
-        Instant now = Instant.now();
-        event.setCreated(now);
-      }
-      workbasketHistoryEventMapper.insert(event);
-    } catch (SQLException e) {
-      LOGGER.error("Error while inserting workbasket history event into database", e);
-    } finally {
-      taskanaHistoryEngine.returnConnection();
+
+    if (event.getCreated() == null) {
+      Instant now = Instant.now();
+      event.setCreated(now);
     }
+    workbasketHistoryEventMapper.insert(event);
   }
 
   @Override
   public void create(ClassificationHistoryEvent event) {
-    try {
-      taskanaHistoryEngine.openConnection();
-      if (event.getCreated() == null) {
-        Instant now = Instant.now();
-        event.setCreated(now);
-      }
-      classificationHistoryEventMapper.insert(event);
-    } catch (SQLException e) {
-      LOGGER.error("Error while inserting classification history event into database", e);
-    } finally {
-      taskanaHistoryEngine.returnConnection();
+
+    if (event.getCreated() == null) {
+      Instant now = Instant.now();
+      event.setCreated(now);
     }
+    classificationHistoryEventMapper.insert(event);
   }
 
   @Override

history/taskana-simplehistory-provider/src/main/java/pro/taskana/simplehistory/impl/TaskanaHistoryEngineImpl.java

@@ -55,7 +55,6 @@ public class TaskanaHistoryEngineImpl implements TaskanaHistoryEngine {
   protected TaskanaHistoryEngineImpl(TaskanaEngine taskanaEngine) {
     this.taskanaConfiguration = taskanaEngine.getConfiguration();
     this.taskanaEngine = taskanaEngine;
-
     createTransactionFactory(taskanaConfiguration.isUseManagedTransactions());
     sessionManager = createSqlSessionManager();
   }

history/taskana-simplehistory-provider/src/test/java/pro/taskana/simplehistory/impl/SimpleHistoryServiceImplTest.java

@@ -58,9 +58,7 @@ void should_VerifyMethodInvocations_When_CreateTaskHistoryEvent() throws Excepti
             "wbKey1", "taskId1", "type1", "wbKey2", "someUserId", "someDetails");
 
     cutSpy.create(expectedWb);
-    verify(taskanaHistoryEngineMock, times(1)).openConnection();
     verify(taskHistoryEventMapperMock, times(1)).insert(expectedWb);
-    verify(taskanaHistoryEngineMock, times(1)).returnConnection();
     assertThat(expectedWb.getCreated()).isNotNull();
   }
 
@@ -71,9 +69,7 @@ void should_VerifyMethodInvocations_When_CreateWorkbasketHisoryEvent() throws Ex
             "wbKey1", WorkbasketHistoryEventType.CREATED.getName(), "someUserId", "someDetails");
 
     cutSpy.create(expectedEvent);
-    verify(taskanaHistoryEngineMock, times(1)).openConnection();
     verify(workbasketHistoryEventMapperMock, times(1)).insert(expectedEvent);
-    verify(taskanaHistoryEngineMock, times(1)).returnConnection();
     assertThat(expectedEvent.getCreated()).isNotNull();
   }
 

history/taskana-simplehistory-rest-spring/src/main/java/pro/taskana/simplehistory/rest/TaskHistoryEventController.java

@@ -1,7 +1,6 @@
 package pro.taskana.simplehistory.rest;
 
 import java.beans.ConstructorProperties;
-import java.sql.SQLException;
 import java.util.List;
 import java.util.function.BiConsumer;
 import javax.servlet.http.HttpServletRequest;
@@ -14,7 +13,6 @@
 import org.springframework.web.bind.annotation.GetMapping;
 import org.springframework.web.bind.annotation.PathVariable;
 import org.springframework.web.bind.annotation.RestController;
-import pro.taskana.TaskanaConfiguration;
 import pro.taskana.common.api.BaseQuery.SortDirection;
 import pro.taskana.common.api.TaskanaEngine;
 import pro.taskana.common.api.exceptions.InvalidArgumentException;
@@ -35,19 +33,17 @@
 @RestController
 @EnableHypermediaSupport(type = EnableHypermediaSupport.HypermediaType.HAL)
 public class TaskHistoryEventController {
-
   private final SimpleHistoryServiceImpl simpleHistoryService;
   private final TaskHistoryEventRepresentationModelAssembler assembler;
 
   @Autowired
   public TaskHistoryEventController(
-      TaskanaConfiguration taskanaConfiguration,
+      TaskanaEngine taskanaEngine,
       SimpleHistoryServiceImpl simpleHistoryServiceImpl,
-      TaskHistoryEventRepresentationModelAssembler assembler)
-      throws SQLException {
+      TaskHistoryEventRepresentationModelAssembler assembler) {
 
     this.simpleHistoryService = simpleHistoryServiceImpl;
-    this.simpleHistoryService.initialize(TaskanaEngine.buildTaskanaEngine(taskanaConfiguration));
+    this.simpleHistoryService.initialize(taskanaEngine);
     this.assembler = assembler;
   }
 

lib/taskana-core/src/main/java/pro/taskana/common/api/TaskanaEngine.java

@@ -2,6 +2,7 @@
 
 import java.sql.SQLException;
 import java.util.function.Supplier;
+import org.apache.ibatis.transaction.TransactionFactory;
 import pro.taskana.TaskanaConfiguration;
 import pro.taskana.classification.api.ClassificationService;
 import pro.taskana.common.api.exceptions.NotAuthorizedException;
@@ -93,7 +94,7 @@ public interface TaskanaEngine {
    */
   @SuppressWarnings("checkstyle:JavadocMethod")
   static TaskanaEngine buildTaskanaEngine(TaskanaConfiguration configuration) throws SQLException {
-    return buildTaskanaEngine(configuration, ConnectionManagementMode.PARTICIPATE);
+    return buildTaskanaEngine(configuration, ConnectionManagementMode.PARTICIPATE, null);
   }
 
   /**
@@ -108,7 +109,26 @@ static TaskanaEngine buildTaskanaEngine(TaskanaConfiguration configuration) thro
   static TaskanaEngine buildTaskanaEngine(
       TaskanaConfiguration configuration, ConnectionManagementMode connectionManagementMode)
       throws SQLException {
-    return TaskanaEngineImpl.createTaskanaEngine(configuration, connectionManagementMode);
+    return buildTaskanaEngine(configuration, connectionManagementMode, null);
+  }
+
+  /**
+   * Builds an {@linkplain TaskanaEngine} based on {@linkplain TaskanaConfiguration},
+   * SqlConnectionMode and TransactionFactory.
+   *
+   * @param configuration complete taskanaConfig to build the engine
+   * @param connectionManagementMode connectionMode for the SqlSession
+   * @param transactionFactory the TransactionFactory
+   * @return a {@linkplain TaskanaEngineImpl}
+   * @throws SQLException when the db schema could not be initialized
+   */
+  static TaskanaEngine buildTaskanaEngine(
+      TaskanaConfiguration configuration,
+      ConnectionManagementMode connectionManagementMode,
+      TransactionFactory transactionFactory)
+      throws SQLException {
+    return TaskanaEngineImpl.createTaskanaEngine(
+        configuration, connectionManagementMode, transactionFactory);
   }
 
   /**

lib/taskana-core/src/main/java/pro/taskana/common/internal/TaskanaEngineImpl.java

@@ -112,7 +112,9 @@ public class TaskanaEngineImpl implements TaskanaEngine {
   protected Connection connection;
 
   protected TaskanaEngineImpl(
-      TaskanaConfiguration taskanaConfiguration, ConnectionManagementMode connectionManagementMode)
+      TaskanaConfiguration taskanaConfiguration,
+      ConnectionManagementMode connectionManagementMode,
+      TransactionFactory transactionFactory)
       throws SQLException {
     LOGGER.info(
         "initializing TASKANA with this configuration: {} and this mode: {}",
@@ -146,7 +148,11 @@ protected TaskanaEngineImpl(
 
     currentUserContext =
         new CurrentUserContextImpl(TaskanaConfiguration.shouldUseLowerCaseForAccessIds());
-    createTransactionFactory(taskanaConfiguration.isUseManagedTransactions());
+    if (transactionFactory == null) {
+      createTransactionFactory(taskanaConfiguration.isUseManagedTransactions());
+    } else {
+      this.transactionFactory = transactionFactory;
+    }
     sessionManager = createSqlSessionManager();
 
     initializeDbSchema(taskanaConfiguration);
@@ -156,7 +162,8 @@ protected TaskanaEngineImpl(
           new TaskanaConfiguration.Builder(this.taskanaConfiguration)
               .jobSchedulerEnabled(false)
               .build();
-      TaskanaEngine taskanaEngine = TaskanaEngine.buildTaskanaEngine(configuration, EXPLICIT);
+      TaskanaEngine taskanaEngine =
+          TaskanaEngine.buildTaskanaEngine(configuration, EXPLICIT, transactionFactory);
       RealClock clock =
           new RealClock(
               this.taskanaConfiguration.getJobSchedulerInitialStartDelay(),
@@ -186,9 +193,12 @@ protected TaskanaEngineImpl(
   }
 
   public static TaskanaEngine createTaskanaEngine(
-      TaskanaConfiguration taskanaConfiguration, ConnectionManagementMode connectionManagementMode)
+      TaskanaConfiguration taskanaConfiguration,
+      ConnectionManagementMode connectionManagementMode,
+      TransactionFactory transactionFactory)
       throws SQLException {
-    return new TaskanaEngineImpl(taskanaConfiguration, connectionManagementMode);
+    return new TaskanaEngineImpl(
+        taskanaConfiguration, connectionManagementMode, transactionFactory);
   }
 
   @Override
@@ -246,7 +256,11 @@ public void setConnection(Connection connection) throws SQLException {
       connection.setAutoCommit(false);
       connection.setSchema(taskanaConfiguration.getSchemaName());
       mode = EXPLICIT;
-      sessionManager.startManagedSession(connection);
+      if (transactionFactory.getClass().getSimpleName().equals("SpringManagedTransactionFactory")) {
+        sessionManager.startManagedSession();
+      } else {
+        sessionManager.startManagedSession(connection);
+      }
     } else if (this.connection != null) {
       closeConnection();
     }

lib/taskana-spring/src/main/java/pro/taskana/common/internal/SpringTaskanaEngineImpl.java

@@ -10,9 +10,7 @@ public class SpringTaskanaEngineImpl extends TaskanaEngineImpl implements Spring
   public SpringTaskanaEngineImpl(
       TaskanaConfiguration taskanaConfiguration, ConnectionManagementMode mode)
       throws SQLException {
-    super(taskanaConfiguration, mode);
-    this.transactionFactory = new SpringManagedTransactionFactory();
-    this.sessionManager = createSqlSessionManager();
+    super(taskanaConfiguration, mode, new SpringManagedTransactionFactory());
   }
 
   public static SpringTaskanaEngine createTaskanaEngine(

rest/taskana-rest-spring-example-boot/src/main/java/pro/taskana/example/boot/ExampleRestConfiguration.java

@@ -11,6 +11,7 @@
 import org.springframework.transaction.PlatformTransactionManager;
 import pro.taskana.TaskanaConfiguration;
 import pro.taskana.common.api.TaskanaEngine;
+import pro.taskana.common.internal.SpringTaskanaEngine;
 import pro.taskana.common.internal.configuration.DbSchemaCreator;
 import pro.taskana.sampledata.SampleDataGenerator;
 
@@ -44,7 +45,7 @@ public SampleDataGenerator generateSampleData(
   @DependsOn("generateSampleData")
   public TaskanaEngine getTaskanaEngine(TaskanaConfiguration taskanaConfiguration)
       throws SQLException {
-    return TaskanaEngine.buildTaskanaEngine(taskanaConfiguration);
+    return SpringTaskanaEngine.buildTaskanaEngine(taskanaConfiguration);
   }
 
   // only required to let the adapter example connect to the same database