[GitHub] [accumulo] keith-turner commented on a change in pull request #1359: Use Java Conccurent to manage Timeouts

[GitBox <gi...@apache.org>]

keith-turner commented on a change in pull request #1359: Use Java Conccurent to manage Timeouts
URL: https://github.com/apache/accumulo/pull/1359#discussion_r324757361
 
 

 ##########
 File path: server/master/src/main/java/org/apache/accumulo/master/EventCoordinator.java
 ##########
 @@ -16,33 +16,53 @@
  */
 package org.apache.accumulo.master;
 
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.LongAdder;
+import java.util.concurrent.locks.Condition;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 public class EventCoordinator {
 
-  private static final Logger log = LoggerFactory.getLogger(EventCoordinator.class);
-  long eventCounter = 0;
+  private static final Logger log =
+      LoggerFactory.getLogger(EventCoordinator.class);
+
+  private final Lock eventCoordinatorLock = new ReentrantLock();
+  private final Condition eventReceived = eventCoordinatorLock.newCondition();
+
+  private final LongAdder eventCounter = new LongAdder();
 
-  synchronized long waitForEvents(long millis, long lastEvent) {
-    // Did something happen since the last time we waited?
-    if (lastEvent == eventCounter) {
-      // no
-      if (millis <= 0)
-        return eventCounter;
-      try {
-        wait(millis);
-      } catch (InterruptedException e) {
-        log.debug("ignoring InterruptedException", e);
+  long waitForEvents(final long millis, final long lastEvent) {
+    long nanos = TimeUnit.MILLISECONDS.toNanos(millis);
+    eventCoordinatorLock.lock();
 
 Review comment:
   It seems like the spurious condition could be handled in the existing code with the addition of a loop w/o resorting to adding a java lock.  This a matter of personal preference, but I think using a Java lock in this case makes the code more cumbersome than needed.  However I am not opposed to the change as long as its correct.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services