phet commented on code in PR #4076:
URL: https://github.com/apache/gobblin/pull/4076#discussion_r1847013522


##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/orchestration/MultiActiveLeaseArbiter.java:
##########
@@ -61,6 +61,17 @@ public interface MultiActiveLeaseArbiter {
   LeaseAttemptStatus tryAcquireLease(DagActionStore.LeaseParams leaseParams, 
boolean adoptConsensusFlowExecutionId)
       throws IOException;
 
+  /**
+   * This method checks if lease can be acquired on provided flow in lease 
params
+   * returns true if entry for the same flow does not exists within epsilon 
time
+   * in leaseArbiterStore, else returns false
+   * @param leaseParams   uniquely identifies the flow, the present action 
upon it, the time the action
+   *                      was triggered, and if the dag action event we're 
checking on is a reminder event
+   * @return true if lease can be acquired on the flow passed in the lease 
params, false otherwise
+   */
+  boolean isLeaseAcquirable(DagActionStore.LeaseParams leaseParams)

Review Comment:
   the method name itself suggests a pre-check capability (e.g. first check 
whether it's acquirable and if so, then `tryAcquireLease`... being assured of 
success).
   
   of course, because check-then-act patterns are susceptible to race 
conditions, we'd never actually provide such an API - let's not confuse anyone!
   
   how about `boolean existsSimilarLeaseWithinConsolidationPeriod(LeaseParams)`?
   
   (or `existsEquivalentLeaseWithinConsolidationPeriod`)
   
   



-- 
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: dev-unsubscr...@gobblin.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to