umustafi commented on code in PR #3899:
URL: https://github.com/apache/gobblin/pull/3899#discussion_r1543849308
##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/orchestration/DagManagementTaskStreamImpl.java:
##########
@@ -146,18 +141,14 @@ public DagTask next() {
private MultiActiveLeaseArbiter.LeaseAttemptStatus
retrieveLeaseStatus(DagActionStore.DagAction dagAction)
throws IOException, SchedulerException {
MultiActiveLeaseArbiter.LeaseAttemptStatus leaseAttemptStatus;
- if (!this.isMultiActiveExecutionEnabled) {
- leaseAttemptStatus = new
MultiActiveLeaseArbiter.LeaseObtainedStatus(dagAction,
System.currentTimeMillis(), Long.MAX_VALUE, null);
- } else {
- // TODO: need to handle reminder events and flag them
- leaseAttemptStatus = this.dagActionExecutionLeaseArbiter
- .tryAcquireLease(dagAction, System.currentTimeMillis(), false,
false);
- /* Schedule a reminder for the event unless the lease has been
completed to safeguard against the case where even
- we, when we might become the lease owner still fail to complete
processing
- */
- if (!(leaseAttemptStatus instanceof
MultiActiveLeaseArbiter.NoLongerLeasingStatus)) {
- scheduleReminderForEvent(leaseAttemptStatus);
- }
+ // TODO: need to handle reminder events and flag them
+ leaseAttemptStatus = this.dagActionExecutionLeaseArbiter
+ .tryAcquireLease(dagAction, System.currentTimeMillis(), false, false);
Review Comment:
I don't think this is a parameter worth passing at a class level. If I have
it configurable in the method then the caller (test class or non-test) can
easily specify the value.
--
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]
