From 100653f76e52f1555ce9424924031acdeb941888 Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Mon, 20 Aug 2018 15:37:49 +0200 Subject: [PATCH] SONAR-11145 fix CE queue status not updated when task does not exist --- .../sonar/ce/queue/InternalCeQueueImpl.java | 31 +++++++---- .../ce/queue/InternalCeQueueImplTest.java | 54 +++++++++++++++++++ 2 files changed, 74 insertions(+), 11 deletions(-) diff --git a/server/sonar-ce/src/main/java/org/sonar/ce/queue/InternalCeQueueImpl.java b/server/sonar-ce/src/main/java/org/sonar/ce/queue/InternalCeQueueImpl.java index 693b1bf21fd..7e3edc896fa 100644 --- a/server/sonar-ce/src/main/java/org/sonar/ce/queue/InternalCeQueueImpl.java +++ b/server/sonar-ce/src/main/java/org/sonar/ce/queue/InternalCeQueueImpl.java @@ -114,15 +114,27 @@ public class InternalCeQueueImpl extends CeQueueImpl implements InternalCeQueue @Override public void remove(CeTask task, CeActivityDto.Status status, @Nullable CeTaskResult taskResult, @Nullable Throwable error) { checkArgument(error == null || status == CeActivityDto.Status.FAILED, "Error can be provided only when status is FAILED"); + + long executionTimeInMs = 0L; try (DbSession dbSession = dbClient.openSession(false)) { CeQueueDto queueDto = dbClient.ceQueueDao().selectByUuid(dbSession, task.getUuid()) .orElseThrow(() -> new IllegalStateException("Task does not exist anymore: " + task)); CeActivityDto activityDto = new CeActivityDto(queueDto); activityDto.setStatus(status); - updateQueueStatus(status, activityDto); + executionTimeInMs = updateExecutionFields(activityDto); updateTaskResult(activityDto, taskResult); updateError(activityDto, error); remove(dbSession, queueDto, activityDto); + } finally { + updateQueueStatus(status, executionTimeInMs); + } + } + + private void updateQueueStatus(CeActivityDto.Status status, long executionTimeInMs) { + if (status == CeActivityDto.Status.SUCCESS) { + queueStatus.addSuccess(executionTimeInMs); + } else { + queueStatus.addError(executionTimeInMs); } } @@ -165,19 +177,16 @@ public class InternalCeQueueImpl extends CeQueueImpl implements InternalCeQueue } } - private void updateQueueStatus(CeActivityDto.Status status, CeActivityDto activityDto) { + private long updateExecutionFields(CeActivityDto activityDto) { Long startedAt = activityDto.getStartedAt(); if (startedAt == null) { - return; + return 0L; } - activityDto.setExecutedAt(system2.now()); - long executionTimeInMs = activityDto.getExecutedAt() - startedAt; + long now = system2.now(); + long executionTimeInMs = now - startedAt; + activityDto.setExecutedAt(now); activityDto.setExecutionTimeMs(executionTimeInMs); - if (status == CeActivityDto.Status.SUCCESS) { - queueStatus.addSuccess(executionTimeInMs); - } else { - queueStatus.addError(executionTimeInMs); - } + return executionTimeInMs; } @Override @@ -187,7 +196,7 @@ public class InternalCeQueueImpl extends CeQueueImpl implements InternalCeQueue wornOutTasks.forEach(queueDto -> { CeActivityDto activityDto = new CeActivityDto(queueDto); activityDto.setStatus(CeActivityDto.Status.CANCELED); - updateQueueStatus(CeActivityDto.Status.CANCELED, activityDto); + updateExecutionFields(activityDto); remove(dbSession, queueDto, activityDto); }); } diff --git a/server/sonar-ce/src/test/java/org/sonar/ce/queue/InternalCeQueueImplTest.java b/server/sonar-ce/src/test/java/org/sonar/ce/queue/InternalCeQueueImplTest.java index b31efe0a60d..2b7229ee4d2 100644 --- a/server/sonar-ce/src/test/java/org/sonar/ce/queue/InternalCeQueueImplTest.java +++ b/server/sonar-ce/src/test/java/org/sonar/ce/queue/InternalCeQueueImplTest.java @@ -54,7 +54,11 @@ import org.sonar.server.organization.DefaultOrganizationProvider; import static java.util.Arrays.asList; import static java.util.Collections.emptyMap; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.fail; +import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; import static org.sonar.ce.container.ComputeEngineStatus.Status.STARTED; import static org.sonar.ce.container.ComputeEngineStatus.Status.STOPPING; @@ -245,6 +249,56 @@ public class InternalCeQueueImplTest { assertThat(activityDto.getErrorStacktrace()).isEqualToIgnoringWhitespace(stacktraceToString(error)); } + @Test + public void remove_updates_queueStatus_success_even_if_task_does_not_exist_in_DB() { + CEQueueStatus queueStatus = mock(CEQueueStatus.class); + + CeTask task = submit(CeTaskTypes.REPORT, "PROJECT_1"); + db.getDbClient().ceQueueDao().deleteByUuid(db.getSession(), task.getUuid()); + db.commit(); + + InternalCeQueueImpl underTest = new InternalCeQueueImpl(system2, db.getDbClient(), null, queueStatus, null, null); + + try { + underTest.remove(task, CeActivityDto.Status.SUCCESS, null, null); + fail("remove should have thrown a IllegalStateException"); + } catch (IllegalStateException e) { + verify(queueStatus).addSuccess(anyLong()); + } + } + + @Test + public void remove_updates_queueStatus_failure_even_if_task_does_not_exist_in_DB() { + CEQueueStatus queueStatusMock = mock(CEQueueStatus.class); + + CeTask task = submit(CeTaskTypes.REPORT, "PROJECT_1"); + db.getDbClient().ceQueueDao().deleteByUuid(db.getSession(), task.getUuid()); + db.commit(); + InternalCeQueueImpl underTest = new InternalCeQueueImpl(system2, db.getDbClient(), null, queueStatusMock, null, null); + + try { + underTest.remove(task, CeActivityDto.Status.FAILED, null, null); + fail("remove should have thrown a IllegalStateException"); + } catch (IllegalStateException e) { + verify(queueStatusMock).addError(anyLong()); + } + } + + @Test + public void cancelWornOuts_does_not_update_queueStatus() { + CEQueueStatus queueStatusMock = mock(CEQueueStatus.class); + + CeTask task = submit(CeTaskTypes.REPORT, "PROJECT_1"); + db.executeUpdateSql("update ce_queue set status = 'PENDING', started_at = 123 where uuid = '" + task.getUuid() + "'"); + db.commit(); + InternalCeQueueImpl underTest = new InternalCeQueueImpl(system2, db.getDbClient(), null, queueStatusMock, null, null); + + underTest.cancelWornOuts(); + + assertThat(db.getDbClient().ceActivityDao().selectByUuid(db.getSession(), task.getUuid())).isPresent(); + verifyZeroInteractions(queueStatusMock); + } + private static class TypedExceptionImpl extends RuntimeException implements TypedException { private final String type; -- 2.39.5