keith-turner commented on code in PR #4126:
URL: https://github.com/apache/accumulo/pull/4126#discussion_r1441103144
##########
core/src/main/java/org/apache/accumulo/core/fate/ZooStore.java:
##########
@@ -200,11 +201,12 @@ public long reserve() {
synchronized (this) {
// suppress lgtm alert - synchronized variable is not always true
if (events == statusChangeEvents) { // lgtm
[java/constant-comparison]
- if (defered.isEmpty()) {
+ if (deferred.isEmpty()) {
this.wait(5000);
} else {
- Long minTime = Collections.min(defered.values());
- long waitTime = minTime - System.currentTimeMillis();
+ Long minTime = Collections.min(deferred.values());
Review Comment:
because nanoTime can wrap, the min wait time could be `Long.MAX_VALUE - 10`
but this could return something like `Long.MAX_VALUE + 10` if its in the values
because that would be negative.
Did the following experiment
```
jshell> var vals = List.of(Long.MAX_VALUE - 10, Long.MAX_VALUE + 10)
vals ==> [9223372036854775797, -9223372036854775799]
jshell> long currTime = Long.MAX_VALUE + 5
currTime ==> -9223372036854775804
jshell> vals.stream().mapToLong(l-> l - currTime).min().getAsLong()
$15 ==> -15
jshell> Collections.min(vals) - currTime
$16 ==> 5
```
Based on that, thinking this code could be. Hopefully the following code is
correct even if nano time wraps.
```java
long currTime = System.nanoTime();
long minDuration = deferred.values().stream().mapToLong(l-> l -
currTime).min().getAsLong()
waitTime = TimeUnit.MILLISECONDS.convert(minDuration, TimeUnit.NANOSECONDS);
```
--
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: notifications-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
