Return-Path: X-Original-To: apmail-zookeeper-commits-archive@www.apache.org Delivered-To: apmail-zookeeper-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id D1BF61765F for ; Tue, 5 May 2015 02:38:37 +0000 (UTC) Received: (qmail 45648 invoked by uid 500); 5 May 2015 02:38:37 -0000 Delivered-To: apmail-zookeeper-commits-archive@zookeeper.apache.org Received: (qmail 45613 invoked by uid 500); 5 May 2015 02:38:37 -0000 Mailing-List: contact commits-help@zookeeper.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@ Delivered-To: mailing list commits@zookeeper.apache.org Received: (qmail 45602 invoked by uid 99); 5 May 2015 02:38:37 -0000 Received: from eris.apache.org (HELO hades.apache.org) (140.211.11.105) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 05 May 2015 02:38:37 +0000 Received: from hades.apache.org (localhost [127.0.0.1]) by hades.apache.org (ASF Mail Server at hades.apache.org) with ESMTP id 7D533AC010F for ; Tue, 5 May 2015 02:38:37 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r1677724 - in /zookeeper/trunk: CHANGES.txt src/java/test/org/apache/zookeeper/RemoveWatchesTest.java Date: Tue, 05 May 2015 02:38:37 -0000 To: commits@zookeeper.apache.org From: rakeshr@apache.org X-Mailer: svnmailer-1.0.9 Message-Id: <20150505023837.7D533AC010F@hades.apache.org> Author: rakeshr Date: Tue May 5 02:38:36 2015 New Revision: 1677724 URL: http://svn.apache.org/r1677724 Log: ZOOKEEPER-2062 RemoveWatchesTest takes forever to run (Chris Nauroth via rakeshr) Modified: zookeeper/trunk/CHANGES.txt zookeeper/trunk/src/java/test/org/apache/zookeeper/RemoveWatchesTest.java Modified: zookeeper/trunk/CHANGES.txt URL: http://svn.apache.org/viewvc/zookeeper/trunk/CHANGES.txt?rev=1677724&r1=1677723&r2=1677724&view=diff ============================================================================== --- zookeeper/trunk/CHANGES.txt (original) +++ zookeeper/trunk/CHANGES.txt Tue May 5 02:38:36 2015 @@ -89,6 +89,8 @@ BUGFIXES: ZOOKEEPER-2124 Allow Zookeeper version string to have underscore '_' (Chris Nauroth via rgs) + ZOOKEEPER-2062 RemoveWatchesTest takes forever to run (Chris Nauroth via rakeshr) + IMPROVEMENTS: ZOOKEEPER-1660 Documentation for Dynamic Reconfiguration (Reed Wanderman-Milne via shralex) Modified: zookeeper/trunk/src/java/test/org/apache/zookeeper/RemoveWatchesTest.java URL: http://svn.apache.org/viewvc/zookeeper/trunk/src/java/test/org/apache/zookeeper/RemoveWatchesTest.java?rev=1677724&r1=1677723&r2=1677724&view=diff ============================================================================== --- zookeeper/trunk/src/java/test/org/apache/zookeeper/RemoveWatchesTest.java (original) +++ zookeeper/trunk/src/java/test/org/apache/zookeeper/RemoveWatchesTest.java Tue May 5 02:38:36 2015 @@ -22,6 +22,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -29,11 +30,13 @@ import java.util.concurrent.CountDownLat import java.util.concurrent.TimeoutException; import java.util.concurrent.TimeUnit; +import org.apache.commons.collections.CollectionUtils; import org.apache.zookeeper.KeeperException.Code; import org.apache.zookeeper.KeeperException.NoWatcherException; import org.apache.zookeeper.Watcher.Event.EventType; import org.apache.zookeeper.Watcher.WatcherType; import org.apache.zookeeper.ZooDefs.Ids; +import org.apache.zookeeper.server.ServerCnxn; import org.apache.zookeeper.test.ClientBase; import org.junit.Assert; import org.junit.Test; @@ -139,14 +142,18 @@ public class RemoveWatchesTest extends C Assert.assertNotNull("Didn't set data watches", zk2.exists("/node2", w2)); removeWatches(zk2, "/node1", w1, WatcherType.Data, false, Code.OK); + Assert.assertEquals("Didn't find data watcher", 1, + zk2.getDataWatches().size()); + Assert.assertEquals("Didn't find data watcher", "/node2", + zk2.getDataWatches().get(0)); + removeWatches(zk2, "/node2", w2, WatcherType.Any, false, Code.OK); + Assert.assertTrue("Didn't remove data watcher", w2.matches()); // closing session should remove ephemeral nodes and trigger data // watches if any if (zk1 != null) { zk1.close(); zk1 = null; } - Assert.assertTrue("Didn't remove data watcher", w1.matches()); - Assert.assertFalse("Should have removed data watcher", w2.matches()); List events = w1.getEventsAfterWatchRemoval(); Assert.assertFalse( @@ -176,14 +183,18 @@ public class RemoveWatchesTest extends C Assert.assertNotNull("Didn't set data watches", zk2.exists("/node1", w2)); removeWatches(zk2, "/node1", w2, WatcherType.Data, false, Code.OK); + Assert.assertEquals("Didn't find data watcher", 1, + zk2.getDataWatches().size()); + Assert.assertEquals("Didn't find data watcher", "/node1", + zk2.getDataWatches().get(0)); + removeWatches(zk2, "/node1", w1, WatcherType.Any, false, Code.OK); + Assert.assertTrue("Didn't remove data watcher", w2.matches()); // closing session should remove ephemeral nodes and trigger data // watches if any if (zk1 != null) { zk1.close(); zk1 = null; } - Assert.assertTrue("Didn't remove data watcher", w2.matches()); - Assert.assertFalse("Should have removed data watcher", w1.matches()); List events = w2.getEventsAfterWatchRemoval(); Assert.assertEquals( @@ -209,7 +220,10 @@ public class RemoveWatchesTest extends C zk2.getChildren("/node1", w2); removeWatches(zk2, "/node1", w2, WatcherType.Children, false, Code.OK); Assert.assertTrue("Didn't remove child watcher", w2.matches()); - Assert.assertFalse("Should have removed child watcher", w1.matches()); + Assert.assertEquals("Didn't find child watcher", 1, zk2 + .getChildWatches().size()); + removeWatches(zk2, "/node1", w1, WatcherType.Any, false, Code.OK); + Assert.assertTrue("Didn't remove child watcher", w1.matches()); // create child to see NodeChildren notification zk1.create("/node1/node2", null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); @@ -447,7 +461,12 @@ public class RemoveWatchesTest extends C zk2.getChildren("/node1", w2); removeWatches(zk2, "/node1", w1, WatcherType.Any, false, Code.OK); Assert.assertTrue("Didn't remove data watcher", w1.matches()); - Assert.assertFalse("Shouldn't remove child watcher", w2.matches()); + Assert.assertEquals("Didn't find child watcher", 1, zk2 + .getChildWatches().size()); + Assert.assertEquals("Didn't find data watcher", 1, zk2 + .getDataWatches().size()); + removeWatches(zk2, "/node1", w2, WatcherType.Any, false, Code.OK); + Assert.assertTrue("Didn't remove child watcher", w2.matches()); } /** @@ -472,7 +491,12 @@ public class RemoveWatchesTest extends C zk2.getChildren("/node1", w1); removeWatches(zk2, "/node1", w2, WatcherType.Any, false, Code.OK); Assert.assertTrue("Didn't remove child watcher", w2.matches()); - Assert.assertFalse("Shouldn't remove data watcher", w1.matches()); + Assert.assertEquals("Didn't find child watcher", 1, zk2 + .getChildWatches().size()); + Assert.assertEquals("Didn't find data watcher", 1, zk2 + .getDataWatches().size()); + removeWatches(zk2, "/node1", w1, WatcherType.Any, false, Code.OK); + Assert.assertTrue("Didn't remove watchers", w1.matches()); } /** @@ -708,7 +732,10 @@ public class RemoveWatchesTest extends C zk2.getChildren("/node1", w1); removeWatches(zk2, "/node1", w1, WatcherType.Any, false, Code.OK); Assert.assertTrue("Didn't remove child watcher", w1.matches()); - Assert.assertFalse("Shouldn't remove data watcher", w2.matches()); + Assert.assertEquals("Didn't find child watcher", 1, zk2 + .getChildWatches().size()); + removeWatches(zk2, "/node1", w2, WatcherType.Any, false, Code.OK); + Assert.assertTrue("Didn't remove child watcher", w2.matches()); } /** @@ -930,16 +957,16 @@ public class RemoveWatchesTest extends C Assert.assertNotNull("Didn't set data watches", zk2.exists("/node1", w2)); + Assert.assertTrue("Server session is not a watcher", + isServerSessionWatcher(zk2.getSessionId(), "/node1", + WatcherType.Data)); removeAllWatches(zk2, "/node1", WatcherType.Data, false, Code.OK); Assert.assertTrue("Didn't remove data watcher", rmWatchCount.await(CONNECTION_TIMEOUT, TimeUnit.MILLISECONDS)); - zk1.setData("/node1", "test".getBytes(), -1); - LOG.info("Waiting for data watchers notification after watch removal"); - Assert.assertFalse("Received data watch notification!", - dWatchCount.await(CONNECTION_TIMEOUT, TimeUnit.MILLISECONDS)); - Assert.assertEquals("Received watch notification after removal!", 2, - dWatchCount.getCount()); + Assert.assertFalse("Server session is still a watcher after removal", + isServerSessionWatcher(zk2.getSessionId(), "/node1", + WatcherType.Data)); } /** @@ -991,17 +1018,16 @@ public class RemoveWatchesTest extends C Assert.assertEquals("Didn't set child watches", 0, zk2.getChildren("/node1", w2).size()); + Assert.assertTrue("Server session is not a watcher", + isServerSessionWatcher(zk2.getSessionId(), "/node1", + WatcherType.Children)); removeAllWatches(zk2, "/node1", WatcherType.Children, false, Code.OK); Assert.assertTrue("Didn't remove child watcher", rmWatchCount.await(CONNECTION_TIMEOUT, TimeUnit.MILLISECONDS)); - zk1.create("/node1/node2", null, Ids.OPEN_ACL_UNSAFE, - CreateMode.PERSISTENT); - LOG.info("Waiting for child watchers to be notified"); - Assert.assertFalse("Didn't get child watch notification!", - cWatchCount.await(CONNECTION_TIMEOUT, TimeUnit.MILLISECONDS)); - Assert.assertEquals("Received watch notification after removal!", 2, - cWatchCount.getCount()); + Assert.assertFalse("Server session is still a watcher after removal", + isServerSessionWatcher(zk2.getSessionId(), "/node1", + WatcherType.Children)); } /** @@ -1067,17 +1093,15 @@ public class RemoveWatchesTest extends C Assert.assertNotNull("Didn't set data watches", zk2.exists("/node1", w2)); + Assert.assertTrue("Server session is not a watcher", + isServerSessionWatcher(zk2.getSessionId(), "/node1", + WatcherType.Data)); removeAllWatches(zk2, "/node1", WatcherType.Any, false, Code.OK); Assert.assertTrue("Didn't remove data watcher", rmWatchCount.await(CONNECTION_TIMEOUT, TimeUnit.MILLISECONDS)); - - zk1.create("/node1/node2", null, Ids.OPEN_ACL_UNSAFE, - CreateMode.PERSISTENT); - zk1.setData("/node1", "test".getBytes(), -1); - - LOG.info("Waiting for child/data watchers notification after watch removal"); - Assert.assertFalse("Received watch notification after removal!", - watchCount.await(CONNECTION_TIMEOUT, TimeUnit.MILLISECONDS)); + Assert.assertFalse("Server session is still a watcher after removal", + isServerSessionWatcher(zk2.getSessionId(), "/node1", + WatcherType.Data)); Assert.assertEquals("Received watch notification after removal!", 2, watchCount.getCount()); } @@ -1146,6 +1170,14 @@ public class RemoveWatchesTest extends C } } + /** + * Returns true if the watcher was triggered. Try to avoid using this + * method with assertFalse statements. A false return depends on a timed + * out wait on a latch, which makes tests run long. + * + * @return true if the watcher was triggered, false otherwise + * @throws InterruptedException if interrupted while waiting on latch + */ public boolean matches() throws InterruptedException { if (!latch.await(CONNECTION_TIMEOUT/5, TimeUnit.MILLISECONDS)) { LOG.error("Failed waiting to remove the watches"); @@ -1181,6 +1213,14 @@ public class RemoveWatchesTest extends C this.latch.countDown(); } + /** + * Returns true if the callback was triggered. Try to avoid using this + * method with assertFalse statements. A false return depends on a timed + * out wait on a latch, which makes tests run long. + * + * @return true if the watcher was triggered, false otherwise + * @throws InterruptedException if interrupted while waiting on latch + */ public boolean matches() throws InterruptedException { if (!latch.await(CONNECTION_TIMEOUT/5, TimeUnit.MILLISECONDS)) { return false; @@ -1188,4 +1228,25 @@ public class RemoveWatchesTest extends C return path.equals(eventPath) && rc == eventRc; } } + + /** + * Checks if a session is registered with the server as a watcher. + * + * @param long sessionId the session ID to check + * @param path the path to check for watchers + * @param type the type of watcher + * @return true if the client session is a watcher on path for the type + */ + private boolean isServerSessionWatcher(long sessionId, String path, + WatcherType type) { + Set cnxns = new HashSet<>(); + CollectionUtils.addAll(cnxns, serverFactory.getConnections().iterator()); + for (ServerCnxn cnxn : cnxns) { + if (cnxn.getSessionId() == sessionId) { + return getServer(serverFactory).getZKDatabase().getDataTree() + .containsWatcher(path, type, cnxn); + } + } + return false; + } }