curator-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From randg...@apache.org
Subject [1/8] curator git commit: 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
Date Sat, 26 Sep 2015 00:19:29 GMT
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 <randgalt@apache.org>
Authored: Wed Sep 9 20:13:56 2015 -0500
Committer: randgalt <randgalt@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>


Mime
View raw message