Skip to content

Commit

Permalink
Closes #2374: Deadlock when creating HistoryEvents from many connecti…
Browse files Browse the repository at this point in the history
…ons 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
  • Loading branch information
Jörg Heffner committed Nov 2, 2023
1 parent fdbc13e commit 52e2007
Show file tree
Hide file tree
Showing 8 changed files with 106 additions and 64 deletions.
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -43,61 +47,75 @@ 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 SystemException(
"TASKANA engine of Session Manager could not be retrieved. Aborting Startup");
}
}

@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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand All @@ -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();
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
Expand All @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

/**
Expand All @@ -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);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: {}",
Expand Down Expand Up @@ -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);
Expand All @@ -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(),
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 52e2007

Please sign in to comment.