Skip to content

Commit

Permalink
ZOOKEEPER-2692: Fix race condition in testWatchAutoResetWithPending
Browse files Browse the repository at this point in the history
We occasionally run into an issue with testWatchAutoResetWithPending where we get flaky test behavior due to not being able to reliably predict when the client has received notification from each watch that may be fired (perhaps due to resource contention on the box running the tests). This patch works around that by waiting for a one second quiet period, after which we can more safely assume all watches that will be fired have been fired.

Here is an example of the test failure: https://builds.apache.org/job/ZooKeeper-trunk-jdk8/935/

Author: Abraham Fine <[email protected]>

Reviewers: Michael Han <[email protected]>

Closes apache#171 from afine/ZOOKEEPER-2692
  • Loading branch information
Abraham Fine authored and hanm committed Feb 16, 2017
1 parent c164ee4 commit 1912fa8
Showing 1 changed file with 16 additions and 2 deletions.
18 changes: 16 additions & 2 deletions src/java/test/org/apache/zookeeper/test/WatcherTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@
public class WatcherTest extends ClientBase {
protected static final Logger LOG = LoggerFactory.getLogger(WatcherTest.class);

private long timeOfLastWatcherInvocation;

private final static class MyStatCallback implements StatCallback {
int rc;
public void processResult(int rc, String path, Object ctx, Stat stat) {
Expand All @@ -59,6 +61,7 @@ private class MyWatcher extends CountdownWatcher {
public void process(WatchedEvent event) {
super.process(event);
if (event.getType() != Event.EventType.None) {
timeOfLastWatcherInvocation = System.currentTimeMillis();
try {
events.put(event);
} catch (InterruptedException e) {
Expand Down Expand Up @@ -172,7 +175,6 @@ public void testWatcherCount()
}

final static int COUNT = 100;
boolean hasSeenDelete = true;
/**
* This test checks that watches for pending requests do not get triggered,
* but watches set by previous requests do.
Expand Down Expand Up @@ -206,7 +208,7 @@ public void testWatchAutoResetWithPending() throws Exception {
startServer();
watches[COUNT/2-1].waitForConnected(60000);
Assert.assertEquals(null, zk.exists("/test", false));
Thread.sleep(10);
waitForAllWatchers();
for(int i = 0; i < COUNT/2; i++) {
Assert.assertEquals("For " + i, 1, watches[i].events.size());
}
Expand All @@ -221,6 +223,18 @@ public void testWatchAutoResetWithPending() throws Exception {
zk.close();
}

/**
* Wait until no watcher has been fired in the last second to ensure that all watches
* that are waiting to be fired have been fired
* @throws Exception
*/
private void waitForAllWatchers() throws Exception {
timeOfLastWatcherInvocation = System.currentTimeMillis();
while (System.currentTimeMillis() - timeOfLastWatcherInvocation < 1000) {
Thread.sleep(1000);
}
}

final int TIMEOUT = 5000;

@Test
Expand Down

0 comments on commit 1912fa8

Please sign in to comment.