Repository: zookeeper Updated Branches: refs/heads/branch-3.4 e5259e437 -> a188308cf
ZOOKEEPER-2415: SessionTest is using Thread deprecated API. (branch-3.4) This is the 3.4 version of https://github.com/apache/zookeeper/pull/497 It also include the following patch which was a dependency of the 3.5 version: https://github.com/apache/zookeeper/commit/2b8f98be7407da4417dd192f3de4605a99e3cd15 Author: Michi Mutsuzaki <[email protected]> Author: Andor Molnar <[email protected]> Reviewers: [email protected] Closes #506 from anmolnar/ZOOKEEPER-2415_24 and squashes the following commits: 900a7dc14 [Andor Molnar] ZOOKEEPER-2415. Refactor testSessionTimeout() to live in separate class and not to use deprecated API. Also improved performance. 58a228bd4 [Michi Mutsuzaki] ZOOKEEPER-1730. Make ZooKeeper easier to test - support simulating a session expiration (Jordan Zimmerman via michim) Change-Id: Ie28ed0e256883d214b6ccef4baa06ece8b568d4f Project: http://git-wip-us.apache.org/repos/asf/zookeeper/repo Commit: http://git-wip-us.apache.org/repos/asf/zookeeper/commit/a188308c Tree: http://git-wip-us.apache.org/repos/asf/zookeeper/tree/a188308c Diff: http://git-wip-us.apache.org/repos/asf/zookeeper/diff/a188308c Branch: refs/heads/branch-3.4 Commit: a188308cfb96da62dcd212b53247177f62771966 Parents: e5259e4 Author: Andor Molnar <[email protected]> Authored: Tue Apr 24 16:45:33 2018 -0700 Committer: Patrick Hunt <[email protected]> Committed: Tue Apr 24 16:45:33 2018 -0700 ---------------------------------------------------------------------- .../main/org/apache/zookeeper/ClientCnxn.java | 3 +- .../main/org/apache/zookeeper/Testable.java | 29 +++++ .../main/org/apache/zookeeper/ZooKeeper.java | 6 + .../org/apache/zookeeper/ZooKeeperTestable.java | 47 +++++++ .../org/apache/zookeeper/TestableZooKeeper.java | 9 ++ .../org/apache/zookeeper/test/SessionTest.java | 66 ---------- .../zookeeper/test/SessionTimeoutTest.java | 129 +++++++++++++++++++ 7 files changed, 222 insertions(+), 67 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/zookeeper/blob/a188308c/src/java/main/org/apache/zookeeper/ClientCnxn.java ---------------------------------------------------------------------- diff --git a/src/java/main/org/apache/zookeeper/ClientCnxn.java b/src/java/main/org/apache/zookeeper/ClientCnxn.java index 64b8810..390c363 100644 --- a/src/java/main/org/apache/zookeeper/ClientCnxn.java +++ b/src/java/main/org/apache/zookeeper/ClientCnxn.java @@ -1388,7 +1388,8 @@ public class ClientCnxn { private int xid = 1; - private volatile States state = States.NOT_CONNECTED; + // @VisibleForTesting + volatile States state = States.NOT_CONNECTED; /* * getXid() is called externally by ClientCnxnNIO::doIO() when packets are sent from the outgoingQueue to http://git-wip-us.apache.org/repos/asf/zookeeper/blob/a188308c/src/java/main/org/apache/zookeeper/Testable.java ---------------------------------------------------------------------- diff --git a/src/java/main/org/apache/zookeeper/Testable.java b/src/java/main/org/apache/zookeeper/Testable.java new file mode 100644 index 0000000..b38d2c3 --- /dev/null +++ b/src/java/main/org/apache/zookeeper/Testable.java @@ -0,0 +1,29 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.zookeeper; + +/** + * Abstraction that exposes various methods useful for testing ZooKeeper + */ +public interface Testable { + /** + * Cause the ZooKeeper instance to behave as if the session expired + */ + void injectSessionExpiration(); +} http://git-wip-us.apache.org/repos/asf/zookeeper/blob/a188308c/src/java/main/org/apache/zookeeper/ZooKeeper.java ---------------------------------------------------------------------- diff --git a/src/java/main/org/apache/zookeeper/ZooKeeper.java b/src/java/main/org/apache/zookeeper/ZooKeeper.java index 482e135..3e46ee7 100644 --- a/src/java/main/org/apache/zookeeper/ZooKeeper.java +++ b/src/java/main/org/apache/zookeeper/ZooKeeper.java @@ -97,6 +97,7 @@ public class ZooKeeper { Environment.logEnv("Client environment:", LOG); } + public ZooKeeperSaslClient getSaslClient() { return cnxn.zooKeeperSaslClient; } @@ -595,6 +596,11 @@ public class ZooKeeper { cnxn.start(); } + // VisibleForTesting + public Testable getTestable() { + return new ZooKeeperTestable(this, cnxn); + } + /** * The session id for this ZooKeeper client instance. The value returned is * not valid until the client connects to a server and may change after a http://git-wip-us.apache.org/repos/asf/zookeeper/blob/a188308c/src/java/main/org/apache/zookeeper/ZooKeeperTestable.java ---------------------------------------------------------------------- diff --git a/src/java/main/org/apache/zookeeper/ZooKeeperTestable.java b/src/java/main/org/apache/zookeeper/ZooKeeperTestable.java new file mode 100644 index 0000000..775d1a2 --- /dev/null +++ b/src/java/main/org/apache/zookeeper/ZooKeeperTestable.java @@ -0,0 +1,47 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.zookeeper; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +class ZooKeeperTestable implements Testable { + private static final Logger LOG = LoggerFactory + .getLogger(ZooKeeperTestable.class); + + private final ZooKeeper zooKeeper; + private final ClientCnxn clientCnxn; + + ZooKeeperTestable(ZooKeeper zooKeeper, ClientCnxn clientCnxn) { + this.zooKeeper = zooKeeper; + this.clientCnxn = clientCnxn; + } + + @Override + public void injectSessionExpiration() { + LOG.info("injectSessionExpiration() called"); + + clientCnxn.eventThread.queueEvent(new WatchedEvent( + Watcher.Event.EventType.None, + Watcher.Event.KeeperState.Expired, null)); + clientCnxn.eventThread.queueEventOfDeath(); + clientCnxn.sendThread.getClientCnxnSocket().wakeupCnxn(); + clientCnxn.state = ZooKeeper.States.CLOSED; + } +} http://git-wip-us.apache.org/repos/asf/zookeeper/blob/a188308c/src/java/test/org/apache/zookeeper/TestableZooKeeper.java ---------------------------------------------------------------------- diff --git a/src/java/test/org/apache/zookeeper/TestableZooKeeper.java b/src/java/test/org/apache/zookeeper/TestableZooKeeper.java index 4d46fdf..dd70056 100644 --- a/src/java/test/org/apache/zookeeper/TestableZooKeeper.java +++ b/src/java/test/org/apache/zookeeper/TestableZooKeeper.java @@ -121,4 +121,13 @@ public class TestableZooKeeper extends ZooKeeper { Record response, WatchRegistration watchRegistration) throws InterruptedException { return cnxn.submitRequest(h, request, response, watchRegistration); } + + /** Testing only!!! Really!!!! This is only here to test when the client + * disconnects from the server w/o sending a session disconnect (ie + * ending the session cleanly). The server will eventually notice the + * client is no longer pinging and will timeout the session. + */ + public void disconnect() { + cnxn.disconnect(); + } } http://git-wip-us.apache.org/repos/asf/zookeeper/blob/a188308c/src/java/test/org/apache/zookeeper/test/SessionTest.java ---------------------------------------------------------------------- diff --git a/src/java/test/org/apache/zookeeper/test/SessionTest.java b/src/java/test/org/apache/zookeeper/test/SessionTest.java index 0740ebb..891b0f0 100644 --- a/src/java/test/org/apache/zookeeper/test/SessionTest.java +++ b/src/java/test/org/apache/zookeeper/test/SessionTest.java @@ -22,7 +22,6 @@ import static org.apache.zookeeper.test.ClientBase.CONNECTION_TIMEOUT; import java.io.File; import java.io.IOException; -import java.util.ArrayList; import java.util.LinkedList; import java.util.List; import java.util.concurrent.CountDownLatch; @@ -236,71 +235,6 @@ public class SessionTest extends ZKTestCase { Assert.assertEquals(KeeperException.Code.SESSIONEXPIRED.toString(), cb.toString()); } - private List<Thread> findThreads(String name) { - int threadCount = Thread.activeCount(); - Thread threads[] = new Thread[threadCount*2]; - threadCount = Thread.enumerate(threads); - ArrayList<Thread> list = new ArrayList<Thread>(); - for(int i = 0; i < threadCount; i++) { - if (threads[i].getName().indexOf(name) != -1) { - list.add(threads[i]); - } - } - return list; - } - - /** - * Make sure ephemerals get cleaned up when a session times out. - */ - @Test - public void testSessionTimeout() throws Exception { - final int TIMEOUT = 5000; - List<Thread> etBefore = findThreads("EventThread"); - List<Thread> stBefore = findThreads("SendThread"); - DisconnectableZooKeeper zk = createClient(TIMEOUT); - zk.create("/stest", new byte[0], Ids.OPEN_ACL_UNSAFE, - CreateMode.EPHEMERAL); - - // Find the new event and send threads - List<Thread> etAfter = findThreads("EventThread"); - List<Thread> stAfter = findThreads("SendThread"); - Thread eventThread = null; - Thread sendThread = null; - for(Thread t: etAfter) { - if (!etBefore.contains(t)) { - eventThread = t; - break; - } - } - for(Thread t: stAfter) { - if (!stBefore.contains(t)) { - sendThread = t; - break; - } - } - sendThread.suspend(); - //zk.disconnect(); - - Thread.sleep(TIMEOUT*2); - sendThread.resume(); - eventThread.join(TIMEOUT); - Assert.assertFalse("EventThread is still running", eventThread.isAlive()); - - zk = createClient(TIMEOUT); - zk.create("/stest", new byte[0], Ids.OPEN_ACL_UNSAFE, - CreateMode.EPHEMERAL); - tearDown(); - zk.close(); - zk.disconnect(); - setUp(); - - zk = createClient(TIMEOUT); - Assert.assertTrue(zk.exists("/stest", false) != null); - Thread.sleep(TIMEOUT*2); - Assert.assertTrue(zk.exists("/stest", false) == null); - zk.close(); - } - /** * Make sure that we cannot have two connections with the same * session id. http://git-wip-us.apache.org/repos/asf/zookeeper/blob/a188308c/src/java/test/org/apache/zookeeper/test/SessionTimeoutTest.java ---------------------------------------------------------------------- diff --git a/src/java/test/org/apache/zookeeper/test/SessionTimeoutTest.java b/src/java/test/org/apache/zookeeper/test/SessionTimeoutTest.java new file mode 100644 index 0000000..09badae --- /dev/null +++ b/src/java/test/org/apache/zookeeper/test/SessionTimeoutTest.java @@ -0,0 +1,129 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.zookeeper.test; + +import org.apache.zookeeper.CreateMode; +import org.apache.zookeeper.KeeperException; +import org.apache.zookeeper.TestableZooKeeper; +import org.apache.zookeeper.WatchedEvent; +import org.apache.zookeeper.Watcher; +import org.apache.zookeeper.ZooDefs; +import org.apache.zookeeper.ZooKeeper; +import org.apache.zookeeper.data.Stat; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.File; +import java.io.IOException; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; + +public class SessionTimeoutTest extends ClientBase { + protected static final Logger LOG = LoggerFactory.getLogger(SessionTimeoutTest.class); + + private TestableZooKeeper zk; + + @Before + public void setUp() throws Exception { + super.setUp(); + zk = createClient(); + } + + @Test + public void testSessionExpiration() throws InterruptedException, + KeeperException { + final CountDownLatch expirationLatch = new CountDownLatch(1); + Watcher watcher = new Watcher() { + @Override + public void process(WatchedEvent event) { + if ( event.getState() == Event.KeeperState.Expired ) { + expirationLatch.countDown(); + } + } + }; + zk.exists("/foo", watcher); + + zk.getTestable().injectSessionExpiration(); + Assert.assertTrue(expirationLatch.await(5, TimeUnit.SECONDS)); + + boolean gotException = false; + try { + zk.exists("/foo", false); + Assert.fail("Should have thrown a SessionExpiredException"); + } catch (KeeperException.SessionExpiredException e) { + // correct + gotException = true; + } + Assert.assertTrue(gotException); + } + + /** + * Make sure ephemerals get cleaned up when session disconnects. + */ + @Test + public void testSessionDisconnect() throws KeeperException, InterruptedException, IOException { + zk.create("/sdisconnect", new byte[0], ZooDefs.Ids.OPEN_ACL_UNSAFE, + CreateMode.EPHEMERAL); + assertNotNull("Ephemeral node has not been created", zk.exists("/sdisconnect", null)); + + zk.close(); + + zk = createClient(); + assertNull("Ephemeral node shouldn't exist after client disconnect", zk.exists("/sdisconnect", null)); + } + + /** + * Make sure ephemerals are kept when session restores. + */ + @Test + public void testSessionRestore() throws KeeperException, InterruptedException, IOException { + zk.create("/srestore", new byte[0], ZooDefs.Ids.OPEN_ACL_UNSAFE, + CreateMode.EPHEMERAL); + assertNotNull("Ephemeral node has not been created", zk.exists("/srestore", null)); + + zk.disconnect(); + zk.close(); + + zk = createClient(); + assertNotNull("Ephemeral node should be present when session is restored", zk.exists("/srestore", null)); + } + + /** + * Make sure ephemerals are kept when server restarts. + */ + @Test + public void testSessionSurviveServerRestart() throws Exception { + zk.create("/sdeath", new byte[0], ZooDefs.Ids.OPEN_ACL_UNSAFE, + CreateMode.EPHEMERAL); + assertNotNull("Ephemeral node has not been created", zk.exists("/sdeath", null)); + + zk.disconnect(); + stopServer(); + startServer(); + zk = createClient(); + + assertNotNull("Ephemeral node should be present when server restarted", zk.exists("/sdeath", null)); + } +}
