Closes #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
This commit is contained in:
Jörg Heffner 2023-10-24 14:47:45 +02:00
parent fdbc13e395
commit 52e20070de
8 changed files with 106 additions and 64 deletions

View File

@ -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,75 @@ public class SimpleHistoryServiceImpl implements TaskanaHistory {
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

View File

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

View File

@ -58,9 +58,7 @@ class SimpleHistoryServiceImplTest {
"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 @@ class SimpleHistoryServiceImplTest {
"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();
}

View File

@ -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.transaction.annotation.Transactional;
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 @@ import pro.taskana.spi.history.api.exceptions.TaskanaHistoryEventNotFoundExcepti
@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;
}

View File

@ -2,6 +2,7 @@ package pro.taskana.common.api;
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 @@ public interface TaskanaEngine {
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);
}
/**

View File

@ -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 @@ public class TaskanaEngineImpl implements TaskanaEngine {
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 @@ public class TaskanaEngineImpl implements TaskanaEngine {
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 @@ public class TaskanaEngineImpl implements TaskanaEngine {
}
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 class TaskanaEngineImpl implements TaskanaEngine {
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();
}

View File

@ -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(

View File

@ -11,6 +11,7 @@ import org.springframework.jdbc.datasource.DataSourceTransactionManager;
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 class ExampleRestConfiguration {
@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