accumulo git commit: ACCUMULO-3461 Fix race condition in ZooTraceClientTest

[el...@apache.org]

Repository: accumulo
Updated Branches:
  refs/heads/master 81d72b309 -> 9022987e0


ACCUMULO-3461 Fix race condition in ZooTraceClientTest

The 2nd call to ZooTraceClient.updateHostsFromKeeper is made
asynchronously (after waiting for 0ms). This will sometimes
pass and sometimes fail. Add a delegate to the 2nd call and
make a simple latch that the test case can wait on to ensure
both calls happened before we verify the mock's state.

Also fixed a bad log message that [~ecn] noticed.


Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo
Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/9022987e
Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/9022987e
Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/9022987e

Branch: refs/heads/master
Commit: 9022987e093842eb465113f5a3a80f3ef2855444
Parents: 81d72b3
Author: Josh Elser <el...@apache.org>
Authored: Fri Jan 2 16:49:54 2015 -0500
Committer: Josh Elser <el...@apache.org>
Committed: Fri Jan 2 16:51:20 2015 -0500

----------------------------------------------------------------------
 .../apache/accumulo/tracer/ZooTraceClient.java  |  4 +--
 .../accumulo/tracer/ZooTraceClientTest.java     | 29 ++++++++++++++++++--
 2 files changed, 29 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/accumulo/blob/9022987e/server/tracer/src/main/java/org/apache/accumulo/tracer/ZooTraceClient.java
----------------------------------------------------------------------
diff --git a/server/tracer/src/main/java/org/apache/accumulo/tracer/ZooTraceClient.java b/server/tracer/src/main/java/org/apache/accumulo/tracer/ZooTraceClient.java
index 10dfdc2..3db77f0 100644
--- a/server/tracer/src/main/java/org/apache/accumulo/tracer/ZooTraceClient.java
+++ b/server/tracer/src/main/java/org/apache/accumulo/tracer/ZooTraceClient.java
@@ -109,8 +109,8 @@ public class ZooTraceClient extends SendSpansViaThrift implements Watcher {
           // Once this passes, we can issue a shutdown of the pool
           svc.shutdown();
         } catch (Exception e) {
-          log.error("Unabled to get destination tracer hosts in ZooKeeper, will retry in 5 seconds", e);
-          // We failed to connect to ZK, try again in 5seconds
+          log.error("Unabled to get destination tracer hosts in ZooKeeper, will retry in " + retryPause + " milliseconds", e);
+          // We failed to connect to ZK, try again in `retryPause` milliseconds
           svc.schedule(this, retryPause, TimeUnit.MILLISECONDS);
         }
       }

http://git-wip-us.apache.org/repos/asf/accumulo/blob/9022987e/server/tracer/src/test/java/org/apache/accumulo/tracer/ZooTraceClientTest.java
----------------------------------------------------------------------
diff --git a/server/tracer/src/test/java/org/apache/accumulo/tracer/ZooTraceClientTest.java b/server/tracer/src/test/java/org/apache/accumulo/tracer/ZooTraceClientTest.java
index ebb2a7c..2a3d0d1 100644
--- a/server/tracer/src/test/java/org/apache/accumulo/tracer/ZooTraceClientTest.java
+++ b/server/tracer/src/test/java/org/apache/accumulo/tracer/ZooTraceClientTest.java
@@ -16,25 +16,50 @@
  */
 package org.apache.accumulo.tracer;
 
+import java.util.concurrent.atomic.AtomicBoolean;
+
 import org.easymock.EasyMock;
 import org.junit.Test;
 
 public class ZooTraceClientTest {
 
+  /**
+   * An extension on ZooTraceClient which acts as a latch on updateHostsFromZooKeeper using the provided {@link AtomicBoolean}
+   */
+  private static class UpdateHostsDelegate extends ZooTraceClient {
+    private final AtomicBoolean done;
+
+    private UpdateHostsDelegate(AtomicBoolean done) {
+      this.done = done;
+    }
+
+    @Override
+    public void updateHostsFromZooKeeper() {
+      this.done.set(true);
+    }
+  }
+
   @Test
   public void testConnectFailureRetries() throws Exception {
     ZooTraceClient client = EasyMock.createMockBuilder(ZooTraceClient.class).addMockedMethod("updateHostsFromZooKeeper").createStrictMock();
     client.setRetryPause(0l);
+    AtomicBoolean done = new AtomicBoolean(false);
 
     client.updateHostsFromZooKeeper();
     EasyMock.expectLastCall().andThrow(new RuntimeException()).once();
     client.updateHostsFromZooKeeper();
-    EasyMock.expectLastCall();
+    // Expect the second call to updateHostsFromZooKeeper, but wait for it to fire before verification
+    EasyMock.expectLastCall().andDelegateTo(new UpdateHostsDelegate(done));
 
     EasyMock.replay(client);
 
     client.setInitialTraceHosts();
-    
+
+    while (!done.get()) {
+      // The 2nd call to updateHostsFromZooKeeper is async. Wait for it for fire before verifying it was called.
+      Thread.sleep(200);
+    }
+
     EasyMock.verify(client);
 
   }