From 601672ff2d1e2cd79c4cf41301ac916650d72788 Mon Sep 17 00:00:00 2001
From: Jean Aurambault <aurambaj@users.noreply.github.com>
Date: Thu, 20 Jul 2023 13:29:15 -0700
Subject: [PATCH] Exit Job early when the Pollable Task is already finished
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Pollable tasks are marked as finished before the Quartz job actually terminates. There is no global transaction that binds the finishing of the PollableTask with the Quartz job. As such it is possible that a Pollable Task is marked as finished but the Quartz job is not and I’m suspecting that if the scheduler goes down after the pollable task being marked as finished and before the Quartz job is marked as finished then the Quartz job might be retriggered. That would lead to running again a finished PollableTask.

This exit early when the Pollable Task is already finished
---
 .../l10n/mojito/quartz/QuartzPollableJob.java | 59 +++++++++++--------
 1 file changed, 33 insertions(+), 26 deletions(-)

diff --git a/webapp/src/main/java/com/box/l10n/mojito/quartz/QuartzPollableJob.java b/webapp/src/main/java/com/box/l10n/mojito/quartz/QuartzPollableJob.java
index b5ba1040e2..7c7b56f06c 100644
--- a/webapp/src/main/java/com/box/l10n/mojito/quartz/QuartzPollableJob.java
+++ b/webapp/src/main/java/com/box/l10n/mojito/quartz/QuartzPollableJob.java
@@ -46,33 +46,40 @@ public void execute(JobExecutionContext context) throws JobExecutionException {
     Long pollableTaskId = context.getMergedJobDataMap().getLong(POLLABLE_TASK_ID);
     currentPollableTask = pollableTaskService.getPollableTask(pollableTaskId);
 
-    ExceptionHolder exceptionHolder = new ExceptionHolder(currentPollableTask);
-
-    try {
-      I callInput;
-
-      String inputStringFromJob = context.getMergedJobDataMap().getString(INPUT);
-
-      if (inputStringFromJob != null) {
-        logger.debug("Inlined data, read from job data");
-        callInput =
-            (I) objectMapper.readValueUnchecked(inputStringFromJob, typeTokenInput.getRawType());
-      } else {
-        logger.debug("No inlined data, read from blob storage");
-        callInput =
-            (I) pollableTaskBlobStorage.getInput(pollableTaskId, typeTokenInput.getRawType());
-      }
-
-      O callOutput = call(callInput);
-
-      if (!typeTokenOutput.getRawType().equals(Void.class)) {
-        pollableTaskBlobStorage.saveOutput(pollableTaskId, callOutput);
+    if (currentPollableTask.getFinishedDate() != null) {
+      logger.error(
+          "PollableTask is already finished. Suspected invalid retry (potential scheduler shutdown "
+              + "after pollable was marked finished but befre Quartz job was recorded as finished)");
+    } else {
+      ExceptionHolder exceptionHolder = new ExceptionHolder(currentPollableTask);
+
+      try {
+        I callInput;
+
+        String inputStringFromJob = context.getMergedJobDataMap().getString(INPUT);
+
+        if (inputStringFromJob != null) {
+          logger.debug("Inlined data, read from job data");
+          callInput =
+              (I) objectMapper.readValueUnchecked(inputStringFromJob, typeTokenInput.getRawType());
+        } else {
+          logger.debug("No inlined data, read from blob storage");
+          callInput =
+              (I) pollableTaskBlobStorage.getInput(pollableTaskId, typeTokenInput.getRawType());
+        }
+
+        O callOutput = call(callInput);
+
+        if (!typeTokenOutput.getRawType().equals(Void.class)) {
+          pollableTaskBlobStorage.saveOutput(pollableTaskId, callOutput);
+        }
+      } catch (Throwable t) {
+        pollableTaskExceptionUtils.processException(t, exceptionHolder);
+      } finally {
+        currentPollableTask =
+            pollableTaskService.finishTask(
+                currentPollableTask.getId(), null, exceptionHolder, null);
       }
-    } catch (Throwable t) {
-      pollableTaskExceptionUtils.processException(t, exceptionHolder);
-    } finally {
-      currentPollableTask =
-          pollableTaskService.finishTask(currentPollableTask.getId(), null, exceptionHolder, null);
     }
   }