Bill commented on a change in pull request #5743:
URL: https://github.com/apache/geode/pull/5743#discussion_r524634772
##########
File path: geode-common/src/main/java/org/apache/geode/internal/Retry.java
##########
@@ -47,45 +47,44 @@ public void sleep(long sleepTime, TimeUnit sleepTimeUnit)
throws InterruptedExce
}
}
- private static final SteadyClock steadyClock = new SteadyClock();
+ private static final SteadyTimer steadyClock = new SteadyTimer();
/**
* Try the supplier function until the predicate is true or timeout occurs.
*
* @param timeout to retry for
+ * @param timeoutUnit the unit for timeout
* @param interval time between each try
- * @param timeUnit to retry for
+ * @param intervalUnit the unit for interval
* @param supplier to execute until predicate is true or times out
* @param predicate to test for retry
* @param <T> type of return value
* @return value from supplier after it passes predicate or times out.
*/
- public static <T> T tryFor(long timeout,
- long interval,
- TimeUnit timeUnit,
+ public static <T> T tryFor(long timeout, TimeUnit timeoutUnit,
+ long interval, TimeUnit intervalUnit,
Supplier<T> supplier,
Predicate<T> predicate) throws TimeoutException, InterruptedException {
- return tryFor(timeout, interval, timeUnit, supplier, predicate,
steadyClock);
+ return tryFor(timeout, timeoutUnit, interval, intervalUnit, supplier,
predicate, steadyClock);
}
@VisibleForTesting
- static <T> T tryFor(long timeout,
- long interval,
- TimeUnit timeUnit,
+ static <T> T tryFor(long timeout, TimeUnit timeoutUnit,
+ long interval, TimeUnit intervalUnit,
Supplier<T> supplier,
Predicate<T> predicate,
- Clock clock) throws TimeoutException, InterruptedException {
- long until = clock.nanoTime() + NANOSECONDS.convert(timeout, timeUnit);
+ Timer timer) throws TimeoutException, InterruptedException {
+ long until = timer.nanoTime() + NANOSECONDS.convert(timeout, timeoutUnit);
T value;
do {
value = supplier.get();
if (predicate.test(value)) {
return value;
} else {
- clock.sleep(interval, timeUnit);
+ timer.sleep(interval, intervalUnit);
Review comment:
The example @jinmeiliao gave covers use-cases where the number of
iterations is potentially large. Those are cases where time to run predicate +
time to run supplier + sleep _interval_ << _timeout_.
There are other use-cases where that relation does not hold. In those cases
perhaps we will only execute the body of the loop once. Imagine time to run
predicate + time to run supplier + sleep _interval_ (=roughly-equal-to=)
_timeout_ i.e. the timeout interval is roughly of the same magnitude as the
other work to be done each time through the loop. In that case,
@pivotal-jbarrett's concern about overshooting the timeout seem more important.
This second scenario seems to me to be worth the effort of the added complexity
of calculating `sleepTime = min(interval, until - now)` and returning
immediately if that's less-than-or-equal-to zero.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
