phet commented on code in PR #3952:
URL: https://github.com/apache/gobblin/pull/3952#discussion_r1620226064
##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/orchestration/MysqlMultiActiveLeaseArbiter.java:
##########
@@ -305,24 +306,26 @@ public LeaseAttemptStatus
tryAcquireLease(DagActionStore.DagAction dagAction, lo
DagActionStore.DagAction updatedDagAction =
adoptConsensusFlowExecutionId ?
dagAction.updateFlowExecutionId(dbEventTimestamp.getTime()) : dagAction;
log.debug("tryAcquireLease for [{}, is: {}, eventTimestamp: {}] -
CASE 2: Same event, lease is valid",
- updatedDagAction, isReminderEvent ? "reminder" : "original",
dbCurrentTimestamp.getTime());
+ updatedDagAction, dagAction.isReminder ? "reminder" :
"original", dbCurrentTimestamp.getTime());
// Utilize db timestamp for reminder
- return new
LeaseAttemptStatus.LeasedToAnotherStatus(updatedDagAction,
dbEventTimestamp.getTime(),
+ updatedDagAction.setEventTimeMillis(dbEventTimestamp.getTime());
Review Comment:
I see this being updated, but unless I missed something critical, it won't
actually be durably persisted to the DB, will it?
`DagAction` is meant for sharing between the various distributed hosts. in
most every distributed sharing situation, mutability is at best challenging,
and at worst a tragic entrapment into error. therefore I DON'T SUGGEST to
durably persist the event time. immutability should be our standard technique.
where there's a need for additional local-to-each-host info, just compose
around `DagAction`. e.g.:
```
@Data
class LocalDagActionHolder {
private final DagAction dagAction;
private final long timestampMillis; // again... what did you mean by
"event"? (use more specific description)
}
```
[class name could be improved]
--
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]
