phet commented on code in PR #3800:
URL: https://github.com/apache/gobblin/pull/3800#discussion_r1362704776
##########
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/api/DagActionStore.java:
##########
@@ -45,6 +45,16 @@ class DagAction {
public FlowId getFlowId() {
return new
FlowId().setFlowGroup(this.flowGroup).setFlowName(this.flowName);
}
+
+ /**
+ * Replace flow execution id with agreed upon event time to easily track
the flow
+ */
+ public static DagActionStore.DagAction
updateFlowExecutionId(DagActionStore.DagAction flowAction,
Review Comment:
this would better belong as a method of the `DagAction` class defined on
line 39
##########
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/api/MultiActiveLeaseArbiter.java:
##########
@@ -79,28 +79,42 @@ abstract class LeaseAttemptStatus {}
class NoLongerLeasingStatus extends LeaseAttemptStatus {}
/*
- The participant calling this method acquired the lease for the event in
question. The class contains the
- `eventTimestamp` associated with the lease as well as the time the caller
obtained the lease or
- `leaseAcquisitionTimestamp`.
+ The participant calling this method acquired the lease for the event in
question. `Flow action`'s flow execution id
+ is the timestamp associated with the lease and the time the caller obtained
the lease is stored within the
+ `leaseAcquisitionTimestamp` field.
*/
@Data
class LeaseObtainedStatus extends LeaseAttemptStatus {
private final DagActionStore.DagAction flowAction;
- private final long eventTimestamp;
private final long leaseAcquisitionTimestamp;
+
+ /**
+ * Returns event time in millis since epoch for the event of this lease
acquisition.
+ * @return
+ */
+ public long getEventTimeMillis() {
+ return Long.parseLong(flowAction.getFlowExecutionId());
+ }
}
/*
This flow action event already has a valid lease owned by another
participant.
- `eventTimeMillis` is the timestamp the lease is associated with, which may
be a different timestamp for the same flow
- action corresponding to the same instance of the event or a distinct one.
+ `Flow action`'s flow execution id is the timestamp the lease is associated
with, which may be a different timestamp
+ for the same flow action corresponding to the same instance of the event or
a distinct one.
Review Comment:
somewhat confusing, "which may be a different timestamp for the same..." -
reword?
##########
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/api/MultiActiveLeaseArbiter.java:
##########
@@ -79,28 +79,42 @@ abstract class LeaseAttemptStatus {}
class NoLongerLeasingStatus extends LeaseAttemptStatus {}
/*
- The participant calling this method acquired the lease for the event in
question. The class contains the
- `eventTimestamp` associated with the lease as well as the time the caller
obtained the lease or
- `leaseAcquisitionTimestamp`.
+ The participant calling this method acquired the lease for the event in
question. `Flow action`'s flow execution id
+ is the timestamp associated with the lease and the time the caller obtained
the lease is stored within the
+ `leaseAcquisitionTimestamp` field.
*/
@Data
class LeaseObtainedStatus extends LeaseAttemptStatus {
private final DagActionStore.DagAction flowAction;
- private final long eventTimestamp;
private final long leaseAcquisitionTimestamp;
+
+ /**
+ * Returns event time in millis since epoch for the event of this lease
acquisition.
+ * @return
Review Comment:
nit:
```
/** @return event time in ... */
```
--
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]
