feat(engine): check transaction isolation level on init

Related to #4516
camunda · Aug 29, 2024 · ff92c21 · ff92c21
1 parent fbdd474
commit ff92c21
Show file tree

Hide file tree

Showing 4 changed files with 174 additions and 2 deletions.
diff --git a/engine/src/main/java/org/camunda/bpm/engine/impl/cfg/ConfigurationLogger.java b/engine/src/main/java/org/camunda/bpm/engine/impl/cfg/ConfigurationLogger.java
@@ -137,4 +137,20 @@ public NotValidException logErrorNoTTLConfigured() {
         + "* Set a default historyTimeToLive as a global process engine configuration\n"
         + "* (Not recommended) Deactivate the enforceTTL config to disable this check"));
   }
+
+  public ProcessEngineException invalidTransactionIsolationLevel(String transactionIsolationLevel) {
+    return new ProcessEngineException(exceptionMessage("019",
+        "The transaction isolation level set for the database is '{}' which differs from the recommended value. "
+            + "Please change the isolation level to 'READ_COMMITTED' or set property 'skipIsolationLevelCheck' to true. "
+            + "Please keep in mind that some levels are known to cause deadlocks and other unexpected behaviours.",
+        transactionIsolationLevel));
+
+  }
+
+  public void logSkippedIsolationLevelCheck(String transactionIsolationLevel) {
+    logWarn("020", "The transaction isolation level set for the database is '{}' which differs from the recommended value "
+            + "and property skipIsolationLevelCheck is enabled. "
+            + "Please keep in mind that levels different from 'READ_COMMITTED' are known to cause deadlocks and other unexpected behaviours.",
+        transactionIsolationLevel);
+  }
 }
diff --git a/engine/src/main/java/org/camunda/bpm/engine/impl/cfg/ProcessEngineConfigurationImpl.java b/engine/src/main/java/org/camunda/bpm/engine/impl/cfg/ProcessEngineConfigurationImpl.java
@@ -390,7 +390,7 @@
  */
 public abstract class ProcessEngineConfigurationImpl extends ProcessEngineConfiguration {
 
-  protected final static ConfigurationLogger LOG = ConfigurationLogger.CONFIG_LOGGER;
+  protected static ConfigurationLogger LOG = ConfigurationLogger.CONFIG_LOGGER;
 
   public static final String DB_SCHEMA_UPDATE_CREATE = "create";
   public static final String DB_SCHEMA_UPDATE_DROP_CREATE = "drop-create";
@@ -414,6 +414,12 @@ public abstract class ProcessEngineConfigurationImpl extends ProcessEngineConfig
 
   public static SqlSessionFactory cachedSqlSessionFactory;
 
+  protected static final Map<Integer, String> ISOLATION_LEVEL_CONSTANT_NAMES = Map.of(
+      Connection.TRANSACTION_READ_COMMITTED, "READ_COMMITTED",
+      Connection.TRANSACTION_READ_UNCOMMITTED, "READ_UNCOMMITTED",
+      Connection.TRANSACTION_REPEATABLE_READ, "REPEATABLE_READ",
+      Connection.TRANSACTION_SERIALIZABLE, "SERIALIZABLE");
+
   // SERVICES /////////////////////////////////////////////////////////////////
 
   protected RepositoryService repositoryService = new RepositoryServiceImpl();
@@ -601,6 +607,7 @@ public abstract class ProcessEngineConfigurationImpl extends ProcessEngineConfig
   protected boolean enableScriptEngineLoadExternalResources = false;
   protected boolean enableScriptEngineNashornCompatibility = false;
   protected boolean configureScriptEngineHostAccess = true;
+  protected boolean skipIsolationLevelCheck = false;
 
   /**
    * When set to false, the following behavior changes:
@@ -1681,11 +1688,33 @@ protected void initDataSource() {
       }
     }
 
+    checkTransactionIsolationLevel();
+
     if (databaseType == null) {
       initDatabaseType();
     }
   }
 
+  protected void checkTransactionIsolationLevel() {
+    Integer currentIsolationLevel = getCurrentTransactionIsolationLevel();
+    String isolationLevelName = ISOLATION_LEVEL_CONSTANT_NAMES.get(currentIsolationLevel);
+    if (currentIsolationLevel != Connection.TRANSACTION_READ_COMMITTED) {
+      if (skipIsolationLevelCheck) {
+        LOG.logSkippedIsolationLevelCheck(isolationLevelName);
+      } else {
+        throw LOG.invalidTransactionIsolationLevel(isolationLevelName);
+      }
+    }
+  }
+
+  protected Integer getCurrentTransactionIsolationLevel() {
+    try (Connection connection = dataSource.getConnection()) {
+      return connection.getTransactionIsolation();
+    } catch (SQLException e) {
+      throw LOG.databaseConnectionAccessException(e);
+    }
+  }
+
   protected static Properties databaseTypeMappings = getDefaultDatabaseTypeMappings();
   protected static final String MY_SQL_PRODUCT_NAME = "MySQL";
   protected static final String MARIA_DB_PRODUCT_NAME = "MariaDB";
@@ -3537,6 +3566,15 @@ public void setBeans(Map<Object, Object> beans) {
     this.beans = beans;
   }
 
+  public boolean getSkipIsolationLevelCheck() {
+    return this.skipIsolationLevelCheck;
+  }
+
+  public ProcessEngineConfigurationImpl setSkipIsolationLevelCheck(boolean skipIsolationLevelCheck) {
+    this.skipIsolationLevelCheck = skipIsolationLevelCheck;
+    return this;
+  }
+
   @Override
   public ProcessEngineConfigurationImpl setClassLoader(ClassLoader classLoader) {
     super.setClassLoader(classLoader);

diff --git a/engine/src/test/java/org/camunda/bpm/engine/impl/cfg/ProcessEngineConfigurationTest.java b/engine/src/test/java/org/camunda/bpm/engine/impl/cfg/ProcessEngineConfigurationTest.java
@@ -17,11 +17,42 @@
 package org.camunda.bpm.engine.impl.cfg;
 
 import static org.assertj.core.api.Assertions.*;
+import static org.camunda.bpm.engine.impl.ProcessEngineLogger.CONFIG_LOGGER;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.sql.Connection;
+import org.apache.ibatis.datasource.pooled.PooledDataSource;
 import org.camunda.bpm.engine.ProcessEngineConfiguration;
+import org.camunda.bpm.engine.ProcessEngineException;
+import org.junit.AfterClass;
+import org.junit.Before;
 import org.junit.Test;
 
 public class ProcessEngineConfigurationTest {
 
+  private ProcessEngineConfigurationImpl engineConfiguration;
+  private ConfigurationLogger logger;
+  private static final int SERIALIZABLE_VALUE = Connection.TRANSACTION_SERIALIZABLE;
+  private static final String SERIALIZABLE_NAME = "SERIALIZABLE";
+  public static final ProcessEngineException EXPECTED_EXCEPTION = CONFIG_LOGGER.invalidTransactionIsolationLevel(SERIALIZABLE_NAME);
+
+  @Before
+  public void setUp() {
+    this.engineConfiguration = (ProcessEngineConfigurationImpl) ProcessEngineConfiguration.createProcessEngineConfigurationFromResourceDefault();
+    this.logger = mock(ConfigurationLogger.class);
+    when(logger.invalidTransactionIsolationLevel(SERIALIZABLE_NAME)).thenReturn(EXPECTED_EXCEPTION);
+    engineConfiguration.initDataSource(); // initialize the datasource for the first time so we can modify the level
+    ProcessEngineConfigurationImpl.LOG = logger;
+  }
+
+  @AfterClass
+  public static void cleanUp() {
+    ProcessEngineConfigurationImpl.LOG = CONFIG_LOGGER;
+  }
+
   @Test
   public void shouldEnableStandaloneTasksByDefault() {
     // when
@@ -35,8 +66,46 @@ public void shouldEnableStandaloneTasksByDefault() {
   public void shouldEnableImplicitUpdatesDetectionByDefault() {
     // when
     ProcessEngineConfigurationImpl engineConfiguration = (ProcessEngineConfigurationImpl) ProcessEngineConfiguration.createStandaloneProcessEngineConfiguration();
-
     // then
     assertThat(engineConfiguration.isImplicitVariableUpdateDetectionEnabled()).isTrue();
   }
+
+  @Test
+  public void validIsolationLevel() {
+    // given
+    ((PooledDataSource) engineConfiguration.getDataSource()).setDefaultTransactionIsolationLevel(Connection.TRANSACTION_READ_COMMITTED);
+    // when
+    engineConfiguration.initDataSource();
+    // then no exception
+  }
+
+  @Test
+  public void invalidIsolationLevelWithSkipFlagDisabled() {
+    // given
+    ((PooledDataSource) engineConfiguration.getDataSource()).setDefaultTransactionIsolationLevel(SERIALIZABLE_VALUE);
+    // when then
+    assertThatThrownBy(() -> engineConfiguration.initDataSource())
+        .isInstanceOf(ProcessEngineException.class)
+        .hasMessage(EXPECTED_EXCEPTION.getMessage());
+  }
+
+  @Test
+  public void invalidIsolationLevelWithSkipFlagEnabled() {
+    // given
+    ((PooledDataSource) engineConfiguration.getDataSource()).setDefaultTransactionIsolationLevel(SERIALIZABLE_VALUE);
+    engineConfiguration.setSkipIsolationLevelCheck(true);
+    // when
+    engineConfiguration.initDataSource();
+    // then
+    verify(logger).logSkippedIsolationLevelCheck(SERIALIZABLE_NAME);
+  }
+
+  @Test
+  public void validIsolationLevelPropertyFromFileIsSetCorrectly() {
+    // given
+    ProcessEngineConfigurationImpl engineConfiguration = (ProcessEngineConfigurationImpl) ProcessEngineConfiguration
+        .createProcessEngineConfigurationFromResource("camunda.cfg.skipIsolationLevelCheckEnabled.xml");
+    // then
+    assertTrue(engineConfiguration.skipIsolationLevelCheck);
+  }
 }
diff --git a/engine/src/test/resources/camunda.cfg.skipIsolationLevelCheckEnabled.xml b/engine/src/test/resources/camunda.cfg.skipIsolationLevelCheckEnabled.xml
@@ -0,0 +1,49 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<beans xmlns="http://www.springframework.org/schema/beans" 
+       xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+       xsi:schemaLocation="http://www.springframework.org/schema/beans   http://www.springframework.org/schema/beans/spring-beans.xsd">
+
+  <bean id="processEngineConfiguration" class="org.camunda.bpm.engine.impl.cfg.StandaloneInMemProcessEngineConfiguration">
+
+    <property name="jdbcUrl" value="${database.url}" />
+    <property name="jdbcDriver" value="${database.driver}" />
+    <property name="jdbcUsername" value="${database.username}" />
+    <property name="jdbcPassword" value="${database.password}" />
+
+    <!-- Database configurations -->
+    <property name="databaseSchemaUpdate" value="true" />
+
+    <!-- Empty beans map to for testing purpose -->
+    <property name="beans">
+      <map/>
+    </property>
+
+    <!-- job executor configurations -->
+    <property name="jobExecutorActivate" value="false" />
+
+    <property name="bpmnStacktraceVerbose" value="false" />
+
+    <!-- turn off metrics reporter -->
+    <property name="dbMetricsReporterActivate" value="false" />
+    <property name="taskMetricsEnabled" value="false" />
+
+    <!-- mail server configurations -->
+    <property name="mailServerPort" value="${mail.server.port}" />
+    <property name="history" value="${history.level}" />
+
+    <property name="authorizationCheckRevokes" value="${authorizationCheckRevokes}"/>
+
+    <property name="jdbcBatchProcessing" value="${jdbcBatchProcessing}"/>
+
+    <!--<property name="idGenerator" ref="uuidGenerator" />-->
+
+    <property name="enforceHistoryTimeToLive" value="false" />
+
+    <property name="skipIsolationLevelCheck" value="true" />
+
+  </bean>
+
+  <!--<bean id="uuidGenerator" class="org.camunda.bpm.engine.impl.persistence.StrongUuidGenerator" />-->
+
+</beans>