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

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

                Author: ASF GitHub Bot
            Created on: 17/Oct/23 17:18
            Start Date: 17/Oct/23 17:18
    Worklog Time Spent: 10m 
      Work Description: phet commented on code in PR #3800:
URL: https://github.com/apache/gobblin/pull/3800#discussion_r1362496618


##########
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/api/MysqlMultiActiveLeaseArbiter.java:
##########
@@ -514,20 +516,30 @@ protected LeaseAttemptStatus 
evaluateStatusAfterLeaseAttempt(int numRowsUpdated,
     if (!selectInfoResult.getLeaseAcquisitionTimeMillis().isPresent()) {
       return new NoLongerLeasingStatus();
     }
+    DagActionStore.DagAction updatedFlowAction = 
updateFlowExecutionId(flowAction, selectInfoResult.eventTimeMillis);
     if (numRowsUpdated == 1) {
-      log.info("Obtained lease for [{}, is: {}, eventTimestamp: {}] 
successfully!", flowAction,
+      log.info("Obtained lease for [{}, is: {}, eventTimestamp: {}] 
successfully!", updatedFlowAction,
           isReminderEvent ? "reminder" : "original", 
selectInfoResult.eventTimeMillis);
-      return new LeaseObtainedStatus(flowAction, 
selectInfoResult.eventTimeMillis,
+      return new LeaseObtainedStatus(updatedFlowAction, 
selectInfoResult.eventTimeMillis,
           selectInfoResult.getLeaseAcquisitionTimeMillis().get());
     }
     log.info("Another participant acquired lease in between for [{}, is: {}, 
eventTimestamp: {}] - num rows updated: {}",
-        flowAction, isReminderEvent ? "reminder" : "original", 
selectInfoResult.eventTimeMillis, numRowsUpdated);
+        updatedFlowAction, isReminderEvent ? "reminder" : "original", 
selectInfoResult.eventTimeMillis, numRowsUpdated);
     // Another participant acquired lease in between
-    return new LeasedToAnotherStatus(flowAction, 
selectInfoResult.getEventTimeMillis(),
+    return new LeasedToAnotherStatus(updatedFlowAction, 
selectInfoResult.getEventTimeMillis(),
         selectInfoResult.getLeaseAcquisitionTimeMillis().get() + 
selectInfoResult.getDbLinger()
             - (dbCurrentTimestamp.isPresent() ? 
dbCurrentTimestamp.get().getTime() : System.currentTimeMillis()));
   }
 
+  /**
+   *   Replace flow execution id with agreed upon event time to easily track 
the flow
+   */
+  protected static DagActionStore.DagAction 
updateFlowExecutionId(DagActionStore.DagAction flowAction,

Review Comment:
   nit: should be a method of `DagAction`



##########
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/api/MysqlMultiActiveLeaseArbiter.java:
##########
@@ -319,16 +319,18 @@ public LeaseAttemptStatus 
tryAcquireLease(DagActionStore.DagAction flowAction, l
       // Lease is valid
       if (leaseValidityStatus == 1) {
         if (isWithinEpsilon) {
+          DagActionStore.DagAction updatedFlowAction = 
updateFlowExecutionId(flowAction, dbEventTimestamp.getTime());
           log.info("tryAcquireLease for [{}, is: {}, eventTimestamp: {}] - 
CASE 2: Same event, lease is valid",
-              flowAction, isReminderEvent ? "reminder" : "original", 
dbCurrentTimestamp.getTime());
+              updatedFlowAction, isReminderEvent ? "reminder" : "original", 
dbCurrentTimestamp.getTime());
           // Utilize db timestamp for reminder
-          return new LeasedToAnotherStatus(flowAction, 
dbEventTimestamp.getTime(),
+          return new LeasedToAnotherStatus(updatedFlowAction, 
dbEventTimestamp.getTime(),

Review Comment:
   each of the flowExecId updates uses the timestamp value that is also passed 
as the second arg to the `LeasedToAnotherStatus` or `LeaseObtainedStatus` ctor. 
 seems potentially unnecessary for that object still to maintain both--do we 
want it to?
   
   e.g. it could always implement:
   ```
   long getEventTimestamp() {
     return this.dagAction.getFlowExecutionId();
   }
   ```





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

    Worklog Id:     (was: 885713)
    Time Spent: 2h 10m  (was: 2h)

> Improve Logs & Metrics around Multi-active Launch Handling
> ----------------------------------------------------------
>
>                 Key: GOBBLIN-1930
>                 URL: https://issues.apache.org/jira/browse/GOBBLIN-1930
>             Project: Apache Gobblin
>          Issue Type: Improvement
>          Components: gobblin-service
>            Reporter: Urmi Mustafi
>            Assignee: Abhishek Tiwari
>            Priority: Major
>          Time Spent: 2h 10m
>  Remaining Estimate: 0h
>
> Improve logging and metrics around multi-active launch flow event handling to 
> identify any missing events between the {{MysqlMultiActiveLeaseArbiter}} 
> committing the launch event to the {{dagActionStore}} and the 
> {{DagActionMonitor}} receiving events for processing. We want to be able to 
> distinguish between the following cases of 
>  * events that are never received by the {{DagActionMonitor}}
>  * events incorrectly filtered out by the {{DagActionMonitor}}
>  * any failed submissions of dags to the {{DagManager}} either upon leader 
> change or from the {{DagActionChangeMonitor}}



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

Reply via email to