Repository: curator Updated Branches: refs/heads/CURATOR-3.0 ea36769af -> afa8f7a45
Removed usages of Javaassist and simplified how the in-memory ZK server is created and run. Tests should be more reliable and the testing server should respond faster Project: http://git-wip-us.apache.org/repos/asf/curator/repo Commit: http://git-wip-us.apache.org/repos/asf/curator/commit/911f02b1 Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/911f02b1 Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/911f02b1 Branch: refs/heads/CURATOR-3.0 Commit: 911f02b15e9585e893bdd2c9bdd860fbf85bba4c Parents: 73b05aa Author: randgalt <randg...@apache.org> Authored: Wed Sep 9 20:13:56 2015 -0500 Committer: randgalt <randg...@apache.org> Committed: Wed Sep 9 20:13:56 2015 -0500 ---------------------------------------------------------------------- .../recipes/leader/ChaosMonkeyCnxnFactory.java | 22 ++- curator-test/pom.xml | 5 - .../apache/curator/test/BaseClassForTests.java | 1 + .../apache/curator/test/ByteCodeRewrite.java | 131 ------------ .../org/apache/curator/test/TestingCluster.java | 5 - .../org/apache/curator/test/TestingServer.java | 5 - .../curator/test/TestingZooKeeperMain.java | 197 +++++++++++-------- pom.xml | 7 - 8 files changed, 134 insertions(+), 239 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/curator/blob/911f02b1/curator-recipes/src/test/java/org/apache/curator/framework/recipes/leader/ChaosMonkeyCnxnFactory.java ---------------------------------------------------------------------- diff --git a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/leader/ChaosMonkeyCnxnFactory.java b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/leader/ChaosMonkeyCnxnFactory.java index 1c0e50e..3aeec81 100644 --- a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/leader/ChaosMonkeyCnxnFactory.java +++ b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/leader/ChaosMonkeyCnxnFactory.java @@ -19,6 +19,7 @@ package org.apache.curator.framework.recipes.leader; +import org.apache.curator.test.TestingZooKeeperMain; import org.apache.zookeeper.ZooDefs; import org.apache.zookeeper.proto.CreateRequest; import org.apache.zookeeper.server.ByteBufferInputStream; @@ -49,23 +50,20 @@ public class ChaosMonkeyCnxnFactory extends NIOServerCnxnFactory /* How long after the first error, connections are rejected */ public static final long LOCKOUT_DURATION_MS = 6000; - public ChaosMonkeyCnxnFactory() throws IOException - { - } - @Override public void startup(ZooKeeperServer zks) throws IOException, InterruptedException { super.startup(new ChaosMonkeyZookeeperServer(zks)); } - public static class ChaosMonkeyZookeeperServer extends ZooKeeperServer { + private final ZooKeeperServer zks; private long firstError = 0; public ChaosMonkeyZookeeperServer(ZooKeeperServer zks) { + this.zks = zks; setTxnLogFactory(zks.getTxnLogFactory()); setTickTime(zks.getTickTime()); setMinSessionTimeout(zks.getMinSessionTimeout()); @@ -73,6 +71,20 @@ public class ChaosMonkeyCnxnFactory extends NIOServerCnxnFactory } @Override + public void startup() + { + super.startup(); + if ( zks instanceof TestingZooKeeperMain.TestZooKeeperServer ) + { + ((TestingZooKeeperMain.TestZooKeeperServer)zks).noteStartup(); + } + else + { + throw new RuntimeException("Unknown ZooKeeperServer: " + zks.getClass()); + } + } + + @Override public void submitRequest(Request si) { long remaining = firstError != 0 ? LOCKOUT_DURATION_MS - (System.currentTimeMillis() - firstError) : 0; http://git-wip-us.apache.org/repos/asf/curator/blob/911f02b1/curator-test/pom.xml ---------------------------------------------------------------------- diff --git a/curator-test/pom.xml b/curator-test/pom.xml index 80eedb2..d401da5 100644 --- a/curator-test/pom.xml +++ b/curator-test/pom.xml @@ -41,11 +41,6 @@ </dependency> <dependency> - <groupId>org.javassist</groupId> - <artifactId>javassist</artifactId> - </dependency> - - <dependency> <groupId>org.apache.commons</groupId> <artifactId>commons-math</artifactId> </dependency> http://git-wip-us.apache.org/repos/asf/curator/blob/911f02b1/curator-test/src/main/java/org/apache/curator/test/BaseClassForTests.java ---------------------------------------------------------------------- diff --git a/curator-test/src/main/java/org/apache/curator/test/BaseClassForTests.java b/curator-test/src/main/java/org/apache/curator/test/BaseClassForTests.java index 1f6503d..a3bc2b5 100644 --- a/curator-test/src/main/java/org/apache/curator/test/BaseClassForTests.java +++ b/curator-test/src/main/java/org/apache/curator/test/BaseClassForTests.java @@ -19,6 +19,7 @@ package org.apache.curator.test; +import org.apache.zookeeper.server.ServerCnxnFactory; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.testng.IInvokedMethod; http://git-wip-us.apache.org/repos/asf/curator/blob/911f02b1/curator-test/src/main/java/org/apache/curator/test/ByteCodeRewrite.java ---------------------------------------------------------------------- diff --git a/curator-test/src/main/java/org/apache/curator/test/ByteCodeRewrite.java b/curator-test/src/main/java/org/apache/curator/test/ByteCodeRewrite.java deleted file mode 100644 index d8a1c3a..0000000 --- a/curator-test/src/main/java/org/apache/curator/test/ByteCodeRewrite.java +++ /dev/null @@ -1,131 +0,0 @@ -/** - * 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.curator.test; - -import javassist.CannotCompileException; -import javassist.ClassPool; -import javassist.CtClass; -import javassist.CtField; -import javassist.CtMethod; -import javassist.NotFoundException; -import javassist.bytecode.AccessFlag; - -public class ByteCodeRewrite -{ - public static void apply() - { - // NOP - only needed so that static initializer is run - } - - static - { - /* - This ugliness is necessary. There is no way to tell ZK to not register JMX beans. Something - in the shutdown of a QuorumPeer causes the state of the MBeanRegistry to get confused and - generates an assert Exception. - */ - ClassPool pool = ClassPool.getDefault(); - try - { - pool.appendClassPath(new javassist.LoaderClassPath(TestingCluster.class.getClassLoader())); // re: https://github.com/Netflix/curator/issues/11 - - try - { - CtClass cc = pool.get("org.apache.zookeeper.server.ZooKeeperServer"); - fixMethods(cc, "registerJMX", "unregisterJMX"); - } - catch ( NotFoundException ignore ) - { - // ignore - } - - try - { - CtClass cc = pool.get("org.apache.zookeeper.server.quorum.LearnerZooKeeperServer"); - fixMethods(cc, "registerJMX", "unregisterJMX"); - } - catch ( NotFoundException ignore ) - { - // ignore - } - - try - { - CtClass cc = pool.get("org.apache.zookeeper.jmx.MBeanRegistry"); - fixMethods(cc, "register", "unregister"); - } - catch ( NotFoundException ignore ) - { - // ignore - } - - try - { - CtClass cc = pool.get("org.apache.zookeeper.server.ZooKeeperServerMain"); - makeVolatile(cc); - } - catch ( NotFoundException e ) - { - // ignore - } - - try - { - CtClass cc = pool.get("org.apache.zookeeper.server.ServerCnxnFactory"); - makeVolatile(cc); - } - catch ( NotFoundException e ) - { - // ignore - } - } - catch ( Exception e ) - { - e.printStackTrace(); - } - } - - private static void makeVolatile(CtClass cc) throws CannotCompileException - { - for ( CtField field : cc.getDeclaredFields() ) - { - if ( (field.getModifiers() & (AccessFlag.ABSTRACT | AccessFlag.NATIVE | AccessFlag.SYNTHETIC | AccessFlag.STATIC | AccessFlag.FINAL)) == 0 ) - { - field.setModifiers(field.getModifiers() | AccessFlag.VOLATILE); - } - } - cc.toClass(); - } - - private static void fixMethods(CtClass cc, String... methodNames) throws CannotCompileException - { - for ( CtMethod method : cc.getDeclaredMethods() ) - { - for ( String methodName : methodNames ) - { - if ( method.getName().equals(methodName) ) - { - method.setBody(null); - } - } - } - cc.toClass(); - } -} http://git-wip-us.apache.org/repos/asf/curator/blob/911f02b1/curator-test/src/main/java/org/apache/curator/test/TestingCluster.java ---------------------------------------------------------------------- diff --git a/curator-test/src/main/java/org/apache/curator/test/TestingCluster.java b/curator-test/src/main/java/org/apache/curator/test/TestingCluster.java index f6bdbd8..e2a1ae8 100644 --- a/curator-test/src/main/java/org/apache/curator/test/TestingCluster.java +++ b/curator-test/src/main/java/org/apache/curator/test/TestingCluster.java @@ -38,11 +38,6 @@ import java.util.Map; */ public class TestingCluster implements Closeable { - static - { - ByteCodeRewrite.apply(); - } - private final List<TestingZooKeeperServer> servers; /** http://git-wip-us.apache.org/repos/asf/curator/blob/911f02b1/curator-test/src/main/java/org/apache/curator/test/TestingServer.java ---------------------------------------------------------------------- diff --git a/curator-test/src/main/java/org/apache/curator/test/TestingServer.java b/curator-test/src/main/java/org/apache/curator/test/TestingServer.java index 7ed8add..5228c8e 100644 --- a/curator-test/src/main/java/org/apache/curator/test/TestingServer.java +++ b/curator-test/src/main/java/org/apache/curator/test/TestingServer.java @@ -28,11 +28,6 @@ import java.io.IOException; */ public class TestingServer implements Closeable { - static - { - ByteCodeRewrite.apply(); - } - private final TestingZooKeeperServer testingZooKeeperServer; private final InstanceSpec spec; http://git-wip-us.apache.org/repos/asf/curator/blob/911f02b1/curator-test/src/main/java/org/apache/curator/test/TestingZooKeeperMain.java ---------------------------------------------------------------------- diff --git a/curator-test/src/main/java/org/apache/curator/test/TestingZooKeeperMain.java b/curator-test/src/main/java/org/apache/curator/test/TestingZooKeeperMain.java index 3d0c341..702824a 100644 --- a/curator-test/src/main/java/org/apache/curator/test/TestingZooKeeperMain.java +++ b/curator-test/src/main/java/org/apache/curator/test/TestingZooKeeperMain.java @@ -19,24 +19,35 @@ package org.apache.curator.test; +import org.apache.zookeeper.jmx.MBeanRegistry; +import org.apache.zookeeper.jmx.ZKMBeanInfo; import org.apache.zookeeper.server.ServerCnxnFactory; import org.apache.zookeeper.server.ServerConfig; import org.apache.zookeeper.server.ZKDatabase; import org.apache.zookeeper.server.ZooKeeperServer; -import org.apache.zookeeper.server.ZooKeeperServerMain; +import org.apache.zookeeper.server.persistence.FileTxnSnapLog; import org.apache.zookeeper.server.quorum.QuorumPeer; import org.apache.zookeeper.server.quorum.QuorumPeerConfig; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import javax.management.JMException; import java.io.IOException; import java.lang.reflect.Field; +import java.lang.reflect.Modifier; import java.nio.channels.ServerSocketChannel; import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicReference; -public class TestingZooKeeperMain extends ZooKeeperServerMain implements ZooKeeperMainFace +public class TestingZooKeeperMain implements ZooKeeperMainFace { + private static final Logger log = LoggerFactory.getLogger(TestingZooKeeperMain.class); + private final CountDownLatch latch = new CountDownLatch(1); private final AtomicReference<Exception> startingException = new AtomicReference<Exception>(null); + private volatile ServerCnxnFactory cnxnFactory; + private volatile ZooKeeperServer zkServer; + static final int MAX_WAIT_MS = new Timing().milliseconds(); @Override @@ -44,15 +55,15 @@ public class TestingZooKeeperMain extends ZooKeeperServerMain implements ZooKeep { try { - Field cnxnFactoryField = ZooKeeperServerMain.class.getDeclaredField("cnxnFactory"); - cnxnFactoryField.setAccessible(true); - ServerCnxnFactory cnxnFactory = (ServerCnxnFactory)cnxnFactoryField.get(this); - cnxnFactory.closeAll(); + if ( cnxnFactory != null ) + { + cnxnFactory.closeAll(); - Field ssField = cnxnFactory.getClass().getDeclaredField("ss"); - ssField.setAccessible(true); - ServerSocketChannel ss = (ServerSocketChannel)ssField.get(cnxnFactory); - ss.close(); + Field ssField = cnxnFactory.getClass().getDeclaredField("ss"); + ssField.setAccessible(true); + ServerSocketChannel ss = (ServerSocketChannel)ssField.get(cnxnFactory); + ss.close(); + } close(); } @@ -65,12 +76,36 @@ public class TestingZooKeeperMain extends ZooKeeperServerMain implements ZooKeep @Override public void runFromConfig(QuorumPeerConfig config) throws Exception { + try + { + Field instance = MBeanRegistry.class.getDeclaredField("instance"); + instance.setAccessible(true); + MBeanRegistry nopMBeanRegistry = new MBeanRegistry() + { + @Override + public void register(ZKMBeanInfo bean, ZKMBeanInfo parent) throws JMException + { + // NOP + } + + @Override + public void unregister(ZKMBeanInfo bean) + { + // NOP + } + }; + instance.set(null, nopMBeanRegistry); + } + catch ( Exception e ) + { + log.error("Could not fix MBeanRegistry"); + } + ServerConfig serverConfig = new ServerConfig(); serverConfig.readFrom(config); - latch.countDown(); try { - super.runFromConfig(serverConfig); + internalRunFromConfig(serverConfig); } catch ( IOException e ) { @@ -91,28 +126,20 @@ public class TestingZooKeeperMain extends ZooKeeperServerMain implements ZooKeep { latch.await(); - ServerCnxnFactory cnxnFactory = getServerConnectionFactory(); - if ( cnxnFactory != null ) + if ( zkServer != null ) { - final ZooKeeperServer zkServer = getZooKeeperServer(cnxnFactory); - if ( zkServer != null ) + //noinspection SynchronizeOnNonFinalField + synchronized(zkServer) { - synchronized(zkServer) + while ( !zkServer.isRunning() ) { - if ( !zkServer.isRunning() ) - { - zkServer.wait(); - } + zkServer.wait(); } } - else - { - throw new Exception("No zkServer"); - } } else { - throw new Exception("No connection factory"); + throw new Exception("No zkServer. zkServer is volatile: " + Modifier.isVolatile(cnxnFactory.getClass().getDeclaredField("zkServer").getModifiers())); } Exception exception = startingException.get(); @@ -127,27 +154,27 @@ public class TestingZooKeeperMain extends ZooKeeperServerMain implements ZooKeep { try { - shutdown(); + cnxnFactory.shutdown(); } catch ( Throwable e ) { e.printStackTrace(); // just ignore - this class is only for testing } + finally + { + cnxnFactory = null; + } try { - ServerCnxnFactory cnxnFactory = getServerConnectionFactory(); - if ( cnxnFactory != null ) + if ( zkServer != null ) { - ZooKeeperServer zkServer = getZooKeeperServer(cnxnFactory); - if ( zkServer != null ) + zkServer.shutdown(); + ZKDatabase zkDb = zkServer.getZKDatabase(); + if ( zkDb != null ) { - ZKDatabase zkDb = zkServer.getZKDatabase(); - if ( zkDb != null ) - { - // make ZK server close its log files - zkDb.close(); - } + // make ZK server close its log files + zkDb.close(); } } } @@ -155,61 +182,69 @@ public class TestingZooKeeperMain extends ZooKeeperServerMain implements ZooKeep { e.printStackTrace(); // just ignore - this class is only for testing } + finally + { + zkServer = null; + } } - private ServerCnxnFactory getServerConnectionFactory() throws Exception + // copied from ZooKeeperServerMain.java + private void internalRunFromConfig(ServerConfig config) throws IOException { - Field cnxnFactoryField = ZooKeeperServerMain.class.getDeclaredField("cnxnFactory"); - cnxnFactoryField.setAccessible(true); - ServerCnxnFactory cnxnFactory; - - // Wait until the cnxnFactory field is non-null or up to 1s, whichever comes first. - long startTime = System.currentTimeMillis(); - do - { - cnxnFactory = (ServerCnxnFactory)cnxnFactoryField.get(this); - if ( cnxnFactory == null ) - { - try - { - Thread.sleep(10); - } - catch ( InterruptedException e ) - { - Thread.currentThread().interrupt(); - throw e; - } + log.info("Starting server"); + FileTxnSnapLog txnLog = null; + try { + // Note that this thread isn't going to be doing anything else, + // so rather than spawning another thread, we will just call + // run() in this thread. + // create a file logger url from the command line args + txnLog = new FileTxnSnapLog(config.getDataLogDir(), config.getDataDir()); + zkServer = new TestZooKeeperServer(txnLog, config); + + cnxnFactory = ServerCnxnFactory.createFactory(); + cnxnFactory.configure(config.getClientPortAddress(), + config.getMaxClientCnxns()); + cnxnFactory.startup(zkServer); + latch.countDown(); + cnxnFactory.join(); + if ( zkServer.isRunning()) { + zkServer.shutdown(); + } + } catch (InterruptedException e) { + // warn, but generally this is ok + Thread.currentThread().interrupt(); + log.warn("Server interrupted", e); + } finally { + if (txnLog != null) { + txnLog.close(); } } - while ( (cnxnFactory == null) && ((System.currentTimeMillis() - startTime) <= MAX_WAIT_MS) ); - - return cnxnFactory; } - private ZooKeeperServer getZooKeeperServer(ServerCnxnFactory cnxnFactory) throws Exception + public static class TestZooKeeperServer extends ZooKeeperServer { - ZooKeeperServer zkServer; + public TestZooKeeperServer(FileTxnSnapLog txnLog, ServerConfig config) + { + super(txnLog, config.getTickTime(), config.getMinSessionTimeout(), config.getMaxSessionTimeout(), null); + } - // Wait until the zkServer field is non-null - long startTime = System.currentTimeMillis(); - do + protected void registerJMX() { - zkServer = cnxnFactory.getZooKeeperServer(); - if ( zkServer == null ) - { - try - { - Thread.sleep(10); - } - catch ( InterruptedException e ) - { - Thread.currentThread().interrupt(); - throw e; - } - } + // NOP } - while ( (zkServer == null) && ((System.currentTimeMillis() - startTime) <= MAX_WAIT_MS) ); - return zkServer; + @Override + protected void unregisterJMX() + { + // NOP + } + + public void noteStartup() + { + synchronized (this) { + running = true; + notifyAll(); + } + } } } http://git-wip-us.apache.org/repos/asf/curator/blob/911f02b1/pom.xml ---------------------------------------------------------------------- diff --git a/pom.xml b/pom.xml index b48ff54..59ed3e0 100644 --- a/pom.xml +++ b/pom.xml @@ -65,7 +65,6 @@ <maven-javadoc-plugin-version>2.10.3</maven-javadoc-plugin-version> <doxia-module-confluence-version>1.6</doxia-module-confluence-version> <maven-license-plugin-version>1.9.0</maven-license-plugin-version> - <javassist-version>3.18.1-GA</javassist-version> <commons-math-version>2.2</commons-math-version> <jackson-mapper-asl-version>1.9.13</jackson-mapper-asl-version> <jersey-version>1.18.1</jersey-version> @@ -346,12 +345,6 @@ </dependency> <dependency> - <groupId>org.javassist</groupId> - <artifactId>javassist</artifactId> - <version>${javassist-version}</version> - </dependency> - - <dependency> <groupId>org.apache.commons</groupId> <artifactId>commons-math</artifactId> <version>${commons-math-version}</version>
