[ 
https://issues.apache.org/jira/browse/GOBBLIN-2181?focusedWorklogId=948078&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-948078
 ]

ASF GitHub Bot logged work on GOBBLIN-2181:
-------------------------------------------

                Author: ASF GitHub Bot
            Created on: 12/Dec/24 17:27
            Start Date: 12/Dec/24 17:27
    Worklog Time Spent: 10m 
      Work Description: khandelwal-prateek commented on code in PR #4084:
URL: https://github.com/apache/gobblin/pull/4084#discussion_r1882538576


##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/orchestration/DagProcessingEngine.java:
##########
@@ -149,6 +158,13 @@ public void run() {
           dagTask.conclude();
           log.info(dagProc.contextualizeStatus("concluded dagTask"));
         } catch (Exception e) {
+          if(!DagProcessingEngine.isTransientException(e)) {
+            log.error("Ignoring non transient exception. DagTask {} will 
conclude and will not be retried. Exception - {} ",
+                dagTask, e);

Review Comment:
   the exception is included as part of the message string instead of passed as 
a separate parameter, this would log only the exception's toString(), without 
the full stack trace. 
   
   Please update the log statement to pass the exception as a separate 
parameter to log the stack trace
   ```
   log.error("Ignoring non-transient exception. DagTask {} will conclude and 
will not be retried.", dagTask, e);
   ```



##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/orchestration/DagProcessingEngine.java:
##########
@@ -149,6 +158,13 @@ public void run() {
           dagTask.conclude();
           log.info(dagProc.contextualizeStatus("concluded dagTask"));
         } catch (Exception e) {
+          if(!DagProcessingEngine.isTransientException(e)) {
+            log.error("Ignoring non transient exception. DagTask {} will 
conclude and will not be retried. Exception - {} ",

Review Comment:
   don't see `toString()` for dagTask, what does dagTask log? It would be 
useful to have dagId & dagAction in the log.



##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/orchestration/DagProcessingEngine.java:
##########
@@ -67,6 +70,8 @@ public class DagProcessingEngine extends AbstractIdleService {
   public static final String DEFAULT_JOB_START_DEADLINE_TIME_MS = 
"defaultJobStartDeadlineTimeMillis";
   @Getter static long defaultJobStartDeadlineTimeMillis;
   public static final String DEFAULT_FLOW_FAILURE_OPTION = 
FailureOption.FINISH_ALL_POSSIBLE.name();
+  // Todo Update to fetch list from config once transient exception handling 
is implemented and retryable exceptions defined

Review Comment:
   nit: the convention is to use `TODO: `, please update from `Todo` to `TODO: 
` for consistency





Issue Time Tracking
-------------------

    Worklog Id:     (was: 948078)
    Time Spent: 50m  (was: 40m)

> Non transient exception handling by flowspec removal
> ----------------------------------------------------
>
>                 Key: GOBBLIN-2181
>                 URL: https://issues.apache.org/jira/browse/GOBBLIN-2181
>             Project: Apache Gobblin
>          Issue Type: Bug
>            Reporter: Vaibhav Singhal
>            Priority: Major
>          Time Spent: 50m
>  Remaining Estimate: 0h
>
> - Many times we experience failures in flow initialization or processing due 
> to which flow could not be concluded properly
> - Azkaban client exceptions and SQLIntegrityViolation exceptions are examples 
> which have caused failures in recent history
> - Currently most of these failures are by default considered transient 
> exceptions and are retried infinitely
> - As a side effect, it causes flows not to conclude and causes failures in 
> future flow submissions which have caused incidents recently
> - More details on analysis and options explored can be referred here - 
> https://docs.google.com/document/d/1PeuuslIVSX6gQrX1J5d0HW0HbNuNhULCHQVjPxpDghs/edit?tab=t.0
> - As a first step we want to consider all exceptions as non transient and not 
> retry and remove conclude the flow by removing flowspec and dag action
> - This issue tracks the changes to conclude the flow for non transient 
> exceptions and also mark them as failure to reflect the correct status of the 
> flow



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to