scwhittle commented on code in PR #37749:
URL: https://github.com/apache/beam/pull/37749#discussion_r2889110765


##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/windmill/work/processing/failures/WorkFailureProcessor.java:
##########
@@ -140,53 +152,59 @@ private boolean shouldRetryLocally(String computationId, 
Work work, Throwable t)
               + "Work will not be retried locally.",
           computationId,
           work.getWorkItem().getShardingKey());
-    } else if 
(WorkItemCancelledException.isWorkItemCancelledException(parsedException)) {
+      return RetryEvaluation.DO_NOT_RETRY;
+    }
+    if 
(WorkItemCancelledException.isWorkItemCancelledException(parsedException)) {
       LOG.debug(
           "Execution of work for computation '{}' on sharding key '{}' failed. 
"
               + "Work will not be retried locally.",
           computationId,
           work.getWorkItem().getShardingKey());
-    } else {
-      LastExceptionDataProvider.reportException(parsedException);
-      LOG.debug("Failed work: {}", work);
-      Duration elapsedTimeSinceStart = new Duration(work.getStartTime(), 
clock.get());
-      if (!failureTracker.trackFailure(computationId, work.getWorkItem(), 
parsedException)) {
-        LOG.error(
-            "Execution of work for computation '{}' on sharding key '{}' 
failed with uncaught exception, "
-                + "and Windmill indicated not to retry locally.",
-            computationId,
-            work.getWorkItem().getShardingKey(),
-            parsedException);
-      } else if (isOutOfMemoryError(parsedException)) {
-        String heapDump = tryToDumpHeap();
-        LOG.error(
-            "Execution of work for computation '{}' for sharding key '{}' 
failed with out-of-memory. "
-                + "Work will not be retried locally. Heap dump {}.",
-            computationId,
-            work.getWorkItem().getShardingKey(),
-            heapDump,
-            parsedException);
-      } else if 
(elapsedTimeSinceStart.isLongerThan(MAX_LOCAL_PROCESSING_RETRY_DURATION)) {
-        LOG.error(
-            "Execution of work for computation '{}' for sharding key '{}' 
failed with uncaught exception, "
-                + "and it will not be retried locally because the elapsed time 
since start {} "
-                + "exceeds {}.",
-            computationId,
-            work.getWorkItem().getShardingKey(),
-            elapsedTimeSinceStart,
-            MAX_LOCAL_PROCESSING_RETRY_DURATION,
-            parsedException);
-      } else {
-        LOG.error(
-            "Execution of work for computation '{}' on sharding key '{}' 
failed with uncaught exception. "
-                + "Work will be retried locally.",
-            computationId,
-            work.getWorkItem().getShardingKey(),
-            parsedException);
-        return true;
-      }
+      return RetryEvaluation.DO_NOT_RETRY;
     }
 
-    return false;
+    LastExceptionDataProvider.reportException(parsedException);
+    LOG.debug("Failed work: {}", work);
+    Duration elapsedTimeSinceStart = new Duration(work.getStartTime(), 
clock.get());
+    if (isOutOfMemoryError(parsedException)) {
+      String heapDump = tryToDumpHeap();

Review Comment:
   MemoryMonitor itself already catches things, if it does throw it also works 
out ok so just leaving as is to keep simpler.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to