Skip to content

Commit

Permalink
chore(crdb): remove usages of isOptimisticLockingExceptionSuppressible
Browse files Browse the repository at this point in the history
  • Loading branch information
joaquinfelici committed Jun 27, 2024
1 parent a6d1c8b commit 8f1e4cd
Show file tree
Hide file tree
Showing 18 changed files with 50 additions and 109 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,7 @@ public void testJobAcquisitionMetricReporting() {

long acquiredJobs = managementService.createMetricsQuery()
.name(Metrics.JOB_ACQUIRED_SUCCESS).sum();
if (testRule.isOptimisticLockingExceptionSuppressible()) {
assertEquals(3, acquiredJobs);
}
assertEquals(3, acquiredJobs);
}

@Deployment(resources = "org/camunda/bpm/engine/test/api/mgmt/metrics/asyncServiceTaskProcess.bpmn20.xml")
Expand Down Expand Up @@ -128,9 +126,7 @@ public void testCompetingJobAcquisitionMetricReporting() {
long acquiredJobsFailure = managementService.createMetricsQuery()
.name(Metrics.JOB_ACQUIRED_FAILURE).sum();

if (testRule.isOptimisticLockingExceptionSuppressible()) {
assertEquals(3, acquiredJobsFailure);
}
assertEquals(3, acquiredJobsFailure);

// cleanup
jobExecutor1.shutdown();
Expand Down Expand Up @@ -187,13 +183,11 @@ public void testJobExecutionMetricExclusiveFollowUp() {
.name(Metrics.JOB_LOCKED_EXCLUSIVE).sum();

assertEquals(6, jobsSuccessful);
if (testRule.isOptimisticLockingExceptionSuppressible()) {
assertEquals(0, jobsFailed);
// the respective follow-up jobs are exclusive and have been executed right away without
// acquisition
assertEquals(3, jobCandidatesForAcquisition);
assertEquals(3, exclusiveFollowupJobs);
}
assertEquals(0, jobsFailed);
// the respective follow-up jobs are exclusive and have been executed right away without
// acquisition
assertEquals(3, jobCandidatesForAcquisition);
assertEquals(3, exclusiveFollowupJobs);
}

@Deployment(resources = "org/camunda/bpm/engine/test/api/mgmt/metrics/asyncServiceTaskProcess.bpmn20.xml")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,7 @@ public void shouldDetectConcurrentDeletionOfExecutionForTaskInsert() {

// then
assertThat(thread1.exception).isNotNull();
if (getTestRule().isOptimisticLockingExceptionSuppressible()) {
assertThat(thread1.exception).isInstanceOf(OptimisticLockingException.class);
}
assertThat(thread1.exception).isInstanceOf(OptimisticLockingException.class);
}

@Deployment(resources = "org/camunda/bpm/engine/test/concurrency/AbstractCompetingTransactionsOptimisticLockingTest.shouldDetectConcurrentDeletionOfExecutionForTaskInsert.bpmn20.xml")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,6 @@ public void testCompetingExternalTaskFetching() {
thread2.proceedAndWaitTillDone();
assertEquals(0, thread2.fetchedTasks.size());
// but does not fail with an OptimisticLockingException
if (testRule.isOptimisticLockingExceptionSuppressible()) {
assertNull(thread2.exception);
}
assertNull(thread2.exception);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -98,12 +98,10 @@ public void testCompetingJobAcquisitions() {
LOG.debug("test thread notifies thread 2");
threadTwo.proceedAndWaitTillDone();

if (testRule.isOptimisticLockingExceptionSuppressible()) {
// the acquisition did NOT fail
assertNull(threadTwo.exception);
// but the job was not acquired
assertEquals(0, threadTwo.jobs.size());
}
// the acquisition did NOT fail
assertNull(threadTwo.exception);
// but the job was not acquired
assertEquals(0, threadTwo.jobs.size());
}

public class JobAcquisitionThread extends ControllableThread {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,9 @@ public void testVersioning() throws InterruptedException {
.asc()
.list();

if (testRule.isOptimisticLockingExceptionSuppressible()) {
assertThat(processDefinitions.size()).isEqualTo(2);
assertThat(processDefinitions.get(0).getVersion()).isEqualTo(1);
assertThat(processDefinitions.get(1).getVersion()).isEqualTo(2);
}
assertThat(processDefinitions.size()).isEqualTo(2);
assertThat(processDefinitions.get(0).getVersion()).isEqualTo(1);
assertThat(processDefinitions.get(1).getVersion()).isEqualTo(2);
}

protected DeploymentBuilder createDeploymentBuilder() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,7 @@ public void testReconfigureCleanupJobs() {

// there were no failures for the first job reconfiguration
assertThat(engineOne.getException()).isNull();
if (testRule.isOptimisticLockingExceptionSuppressible()) {
assertThat(engineTwo.getException()).isNull();
}
assertThat(engineTwo.getException()).isNull();
}

public class EngineOne extends ControllableCommand<Void> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,7 @@ public void testRunTwoHistoryCleanups() throws InterruptedException {
assertEquals(1, historyCleanupJobs.size());

assertNull(thread1.getException());
if (testRule.isOptimisticLockingExceptionSuppressible()) {
assertNull(thread2.getException());
} else {
assertThat(thread2.getException()).isInstanceOf(OptimisticLockingException.class);
}
assertNull(thread2.getException());

}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,7 @@ public void test() throws InterruptedException {

assertNull(thread1.getException());
Throwable thread2Exception = thread2.getException();
if (testRule.isOptimisticLockingExceptionSuppressible()) {
assertNull(thread2Exception);
}
assertNull(thread2Exception);
HistoryLevel historyLevel = processEngineConfiguration.getHistoryLevel();
assertEquals("full", historyLevel.getName());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,7 @@ public void test() throws InterruptedException {

assertNull(thread1.getException());
Throwable thread2Exception = thread2.getException();
if (testRule.isOptimisticLockingExceptionSuppressible()) {
assertNull(thread2Exception);
}
assertNull(thread2Exception);

String id = processEngineConfiguration.getInstallationId();
assertThat(id).isNotEmpty();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -323,11 +323,9 @@ public void testCompletingSuspendJobDuringAcquisition() {
// then the acquisition will not fail with optimistic locking
assertNull(jobSuspensionThread.exception);

if (testRule.isOptimisticLockingExceptionSuppressible()) {
assertNull(acquisitionThread.exception);
// but the job will also not be acquired
assertEquals(0, acquisitionThread.acquiredJobs.size());
}
assertNull(acquisitionThread.exception);
// but the job will also not be acquired
assertEquals(0, acquisitionThread.acquiredJobs.size());

//--------------------------------------------

Expand Down Expand Up @@ -418,12 +416,10 @@ public void testCompletingUpdateJobDefinitionPriorityDuringExecution() {
executionThread.proceedAndWaitTillDone();

long remainingJobCount = managementService.createJobQuery().count();
if (testRule.isOptimisticLockingExceptionSuppressible()) {
assertNull(executionThread.exception);
assertNull(executionThread.exception);

// and ultimately only one job with an updated priority is left
assertEquals(1L, remainingJobCount);
}
// and ultimately only one job with an updated priority is left
assertEquals(1L, remainingJobCount);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,11 +149,9 @@ public void testConcurrentHistoryCleanupJobReconfigurationExecution() throws Int

assertNull(thread1.getException());

if (testRule.isOptimisticLockingExceptionSuppressible()) {
assertNull(thread2.getException());
assertNull(bootstrapCommand.getContextSpy().getThrowable());
assertNotNull(ProcessEngines.getProcessEngines().get(PROCESS_ENGINE_NAME));
}
assertNull(thread2.getException());
assertNull(bootstrapCommand.getContextSpy().getThrowable());
assertNotNull(ProcessEngines.getProcessEngines().get(PROCESS_ENGINE_NAME));
}

protected static class ControllableProcessEngineBootstrapCommand extends ControllableCommand<Void> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,8 @@ public void testEnableTelemetryWithoutConcurrencyIssue() throws InterruptedExcep
thread2.waitUntilDone();

assertNull(thread1.getException());
if (testRule.isOptimisticLockingExceptionSuppressible()) {
assertNull(thread2.getException());
assertThat(managementService.isTelemetryEnabled()).isFalse();
}
assertNull(thread2.getException());
assertThat(managementService.isTelemetryEnabled()).isFalse();
}

protected static class ControllableUpdateTelemetrySetupCommand extends ControllableCommand<Void> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,7 @@ public void shouldSuppressOleOnConcurrentFetchAndDelete() {
asyncThread.waitUntilDone();

// then
if (testRule.isOptimisticLockingExceptionSuppressible()) {
assertThat(taskService.getAttachment(attachment.getId())).isNull();
}
assertThat(taskService.getAttachment(attachment.getId())).isNull();
}

public class AsyncThread extends ControllableCommand<Void> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,8 @@ public void shouldSuppressOleOnConcurrentFetchAndDelete() {
asyncThread.waitUntilDone();

// then
if (testRule.isOptimisticLockingExceptionSuppressible()) {
assertThat(runtimeService.createVariableInstanceQuery().singleResult().getName()).isEqualTo(VARIABLE_NAME);
assertThat(new String((byte[]) runtimeService.createVariableInstanceQuery().singleResult().getValue())).isEqualTo(ANOTHER_VARIABLE_VALUE);
}
assertThat(runtimeService.createVariableInstanceQuery().singleResult().getName()).isEqualTo(VARIABLE_NAME);
assertThat(new String((byte[]) runtimeService.createVariableInstanceQuery().singleResult().getValue())).isEqualTo(ANOTHER_VARIABLE_VALUE);
}

public class AsyncThread extends ControllableCommand<Void> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,8 @@ public void shouldSuppressOleOnConcurrentFetchAndDelete() {
asyncThread.waitUntilDone();

// then
if (testRule.isOptimisticLockingExceptionSuppressible()) {
assertThat(runtimeService.createVariableInstanceQuery().singleResult().getName()).isEqualTo(VARIABLE_NAME);
assertThat(runtimeService.createVariableInstanceQuery().singleResult().getValue()).isEqualTo(ANOTHER_VARIABLE_VALUE);
}
assertThat(runtimeService.createVariableInstanceQuery().singleResult().getName()).isEqualTo(VARIABLE_NAME);
assertThat(runtimeService.createVariableInstanceQuery().singleResult().getValue()).isEqualTo(ANOTHER_VARIABLE_VALUE);
}

public class AsyncThread extends ControllableCommand<Void> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,11 +125,9 @@ public void testBackoffOnOptimisticLocking() {
Assert.assertEquals(1, jobExecutor2WaitEvents.size());
RecordedWaitEvent waitEvent = jobExecutor2WaitEvents.get(0);

if (testRule.isOptimisticLockingExceptionSuppressible()) {
// we don't know the exact wait time,
// since there is random jitter applied
JobAcquisitionTestHelper.assertInBetween(BASE_BACKOFF_TIME, BASE_BACKOFF_TIME + BASE_BACKOFF_TIME / 2, waitEvent.getTimeBetweenAcquisitions());
}
// we don't know the exact wait time,
// since there is random jitter applied
JobAcquisitionTestHelper.assertInBetween(BASE_BACKOFF_TIME, BASE_BACKOFF_TIME + BASE_BACKOFF_TIME / 2, waitEvent.getTimeBetweenAcquisitions());

// when performing another cycle of acquisition
JobAcquisitionTestHelper.activateInstances(engineRule.getProcessEngine(), 6);
Expand Down Expand Up @@ -158,13 +156,11 @@ public void testBackoffOnOptimisticLocking() {
RecordedWaitEvent secondWaitEvent = jobExecutor2WaitEvents.get(1);
long expectedBackoffTime = BASE_BACKOFF_TIME * BACKOFF_FACTOR; // 1000 * 2^1

if (testRule.isOptimisticLockingExceptionSuppressible()) {
// then thread 2 has tried to acquire 6 jobs this time
Assert.assertEquals(6, secondAcquisitionAttempt.getNumJobsToAcquire());
// and again increased its backoff
Assert.assertEquals(2, jobExecutor2WaitEvents.size());
JobAcquisitionTestHelper.assertInBetween(expectedBackoffTime, expectedBackoffTime + expectedBackoffTime / 2, secondWaitEvent.getTimeBetweenAcquisitions());
}
// then thread 2 has tried to acquire 6 jobs this time
Assert.assertEquals(6, secondAcquisitionAttempt.getNumJobsToAcquire());
// and again increased its backoff
Assert.assertEquals(2, jobExecutor2WaitEvents.size());
JobAcquisitionTestHelper.assertInBetween(expectedBackoffTime, expectedBackoffTime + expectedBackoffTime / 2, secondWaitEvent.getTimeBetweenAcquisitions());
}

@Test
Expand Down Expand Up @@ -208,14 +204,12 @@ public void testBackoffDecrease() {
acquisitionThread2.makeContinueAndWaitForSync(); // acquire
acquisitionThread2.makeContinueAndWaitForSync(); // continue after acquisition with next cycle

if (testRule.isOptimisticLockingExceptionSuppressible()) {
for (int i = 1; i < BACKOFF_DECREASE_THRESHOLD; i++) {
// backoff has not decreased yet
Assert.assertTrue(jobExecutor2WaitEvents.get(i).getTimeBetweenAcquisitions() > 0);
for (int i = 1; i < BACKOFF_DECREASE_THRESHOLD; i++) {
// backoff has not decreased yet
Assert.assertTrue(jobExecutor2WaitEvents.get(i).getTimeBetweenAcquisitions() > 0);

acquisitionThread2.makeContinueAndWaitForSync(); // acquire
acquisitionThread2.makeContinueAndWaitForSync(); // continue after acquisition with next cycle
}
acquisitionThread2.makeContinueAndWaitForSync(); // acquire
acquisitionThread2.makeContinueAndWaitForSync(); // continue after acquisition with next cycle
}

// it decreases its backoff again
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,6 @@ public void testJobLockingFailure() {
List<RecordedWaitEvent> jobExecutor2WaitEvents = jobExecutor2.getAcquireJobsRunnable().getWaitEvents();
Assert.assertEquals(1, jobExecutor2WaitEvents.size());

if (testRule.isOptimisticLockingExceptionSuppressible()) {
Assert.assertEquals(0, jobExecutor2WaitEvents.get(0).getTimeBetweenAcquisitions());
}
Assert.assertEquals(0, jobExecutor2WaitEvents.get(0).getTimeBetweenAcquisitions());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
import org.camunda.bpm.engine.delegate.Expression;
import org.camunda.bpm.engine.impl.cfg.ProcessEngineConfigurationImpl;
import org.camunda.bpm.engine.impl.cmmn.behavior.CaseControlRuleImpl;
import org.camunda.bpm.engine.impl.db.sql.DbSqlSessionFactory;
import org.camunda.bpm.engine.impl.el.FixedValue;
import org.camunda.bpm.engine.impl.history.HistoryLevel;
import org.camunda.bpm.engine.impl.interceptor.Command;
Expand Down Expand Up @@ -417,18 +416,6 @@ public String getDatabaseType() {
.getDatabaseType();
}

/**
* This methods is used to determine if the currently used database
* allows for OptimisticLockingExceptions to be ignored, or handled,
* without a transaction rollback and retry. Otherwise, it is false.
*
* Currently, the method only returns false when CockroachDB is used
* since this database implements its own OLE mechanism.
*/
public boolean isOptimisticLockingExceptionSuppressible() { // TODO: remove this usages
return !DbSqlSessionFactory.CRDB.equals(getDatabaseType());
}

public void deleteAllAuthorizations() {
AuthorizationService authorizationService = processEngine.getAuthorizationService();

Expand Down

0 comments on commit 8f1e4cd

Please sign in to comment.