Removed Classic connection handling as it's old, hard to maintain, forces us to run... CURATOR-427 233/head
authorrandgalt <randgalt@apache.org>
Mon, 24 Jul 2017 04:56:20 +0000 (23:56 -0500)
committerrandgalt <randgalt@apache.org>
Mon, 24 Jul 2017 04:56:20 +0000 (23:56 -0500)
14 files changed:
curator-client/src/main/java/org/apache/curator/CuratorZookeeperClient.java
curator-client/src/main/java/org/apache/curator/connection/ClassicConnectionHandlingPolicy.java [deleted file]
curator-client/src/main/java/org/apache/curator/connection/StandardConnectionHandlingPolicy.java
curator-client/src/test/java/org/apache/curator/TestEnsurePath.java
curator-framework/src/main/java/org/apache/curator/framework/CuratorFrameworkFactory.java
curator-framework/src/main/java/org/apache/curator/framework/imps/ClassicInternalConnectionHandler.java [deleted file]
curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
curator-framework/src/test/java/org/apache/curator/framework/imps/TestEnabledSessionExpiredState.java
curator-framework/src/test/java/org/apache/curator/framework/imps/TestFramework.java
curator-framework/src/test/java/org/apache/curator/framework/schema/TestSchema.java
curator-recipes/src/test/java/org/apache/curator/framework/client/TestResetConnectionWithBackgroundFailure.java
curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/BaseTestTreeCache.java
curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/TestTreeCache.java
curator-test/src/main/java/org/apache/curator/test/BaseClassForTests.java

index 7549c29..b7bb33a 100644 (file)
@@ -20,8 +20,8 @@
 package org.apache.curator;
 
 import com.google.common.base.Preconditions;
-import org.apache.curator.connection.ClassicConnectionHandlingPolicy;
 import org.apache.curator.connection.ConnectionHandlingPolicy;
+import org.apache.curator.connection.StandardConnectionHandlingPolicy;
 import org.apache.curator.drivers.OperationTrace;
 import org.apache.curator.drivers.TracerDriver;
 import org.apache.curator.ensemble.EnsembleProvider;
@@ -66,7 +66,7 @@ public class CuratorZookeeperClient implements Closeable
      */
     public CuratorZookeeperClient(String connectString, int sessionTimeoutMs, int connectionTimeoutMs, Watcher watcher, RetryPolicy retryPolicy)
     {
-        this(new DefaultZookeeperFactory(), new FixedEnsembleProvider(connectString), sessionTimeoutMs, connectionTimeoutMs, watcher, retryPolicy, false, new ClassicConnectionHandlingPolicy());
+        this(new DefaultZookeeperFactory(), new FixedEnsembleProvider(connectString), sessionTimeoutMs, connectionTimeoutMs, watcher, retryPolicy, false, new StandardConnectionHandlingPolicy());
     }
 
     /**
@@ -78,7 +78,7 @@ public class CuratorZookeeperClient implements Closeable
      */
     public CuratorZookeeperClient(EnsembleProvider ensembleProvider, int sessionTimeoutMs, int connectionTimeoutMs, Watcher watcher, RetryPolicy retryPolicy)
     {
-        this(new DefaultZookeeperFactory(), ensembleProvider, sessionTimeoutMs, connectionTimeoutMs, watcher, retryPolicy, false, new ClassicConnectionHandlingPolicy());
+        this(new DefaultZookeeperFactory(), ensembleProvider, sessionTimeoutMs, connectionTimeoutMs, watcher, retryPolicy, false, new StandardConnectionHandlingPolicy());
     }
 
     /**
@@ -95,7 +95,7 @@ public class CuratorZookeeperClient implements Closeable
      */
     public CuratorZookeeperClient(ZookeeperFactory zookeeperFactory, EnsembleProvider ensembleProvider, int sessionTimeoutMs, int connectionTimeoutMs, Watcher watcher, RetryPolicy retryPolicy, boolean canBeReadOnly)
     {
-        this(zookeeperFactory, ensembleProvider, sessionTimeoutMs, connectionTimeoutMs, watcher, retryPolicy, canBeReadOnly, new ClassicConnectionHandlingPolicy());
+        this(zookeeperFactory, ensembleProvider, sessionTimeoutMs, connectionTimeoutMs, watcher, retryPolicy, canBeReadOnly, new StandardConnectionHandlingPolicy());
     }
 
     /**
diff --git a/curator-client/src/main/java/org/apache/curator/connection/ClassicConnectionHandlingPolicy.java b/curator-client/src/main/java/org/apache/curator/connection/ClassicConnectionHandlingPolicy.java
deleted file mode 100644 (file)
index fe24b42..0000000
+++ /dev/null
@@ -1,88 +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.connection;
-
-import org.apache.curator.CuratorZookeeperClient;
-import org.apache.curator.RetryLoop;
-import org.apache.curator.utils.ThreadUtils;
-import java.util.concurrent.Callable;
-
-/**
- * Emulates the pre 3.0.0 Curator connection handling
- */
-public class ClassicConnectionHandlingPolicy implements ConnectionHandlingPolicy
-{
-    @Override
-    public int getSimulatedSessionExpirationPercent()
-    {
-        return 0;
-    }
-
-    @Override
-    public <T> T callWithRetry(CuratorZookeeperClient client, Callable<T> proc) throws Exception
-    {
-        T result = null;
-        RetryLoop retryLoop = client.newRetryLoop();
-        while ( retryLoop.shouldContinue() )
-        {
-            try
-            {
-                client.internalBlockUntilConnectedOrTimedOut();
-                result = proc.call();
-                retryLoop.markComplete();
-            }
-            catch ( Exception e )
-            {
-                ThreadUtils.checkInterrupted(e);
-                retryLoop.takeException(e);
-            }
-        }
-
-        return result;
-    }
-
-    @Override
-    public CheckTimeoutsResult checkTimeouts(Callable<String> hasNewConnectionString, long connectionStartMs, int sessionTimeoutMs, int connectionTimeoutMs) throws Exception
-    {
-        CheckTimeoutsResult result = CheckTimeoutsResult.NOP;
-        int minTimeout = Math.min(sessionTimeoutMs, connectionTimeoutMs);
-        long elapsed = System.currentTimeMillis() - connectionStartMs;
-        if ( elapsed >= minTimeout )
-        {
-            if ( hasNewConnectionString.call() != null )
-            {
-                result = CheckTimeoutsResult.NEW_CONNECTION_STRING;
-            }
-            else
-            {
-                int maxTimeout = Math.max(sessionTimeoutMs, connectionTimeoutMs);
-                if ( elapsed > maxTimeout )
-                {
-                    result = CheckTimeoutsResult.RESET_CONNECTION;
-                }
-                else
-                {
-                    result = CheckTimeoutsResult.CONNECTION_TIMEOUT;
-                }
-            }
-        }
-
-        return result;
-    }
-}
index 8f7a438..41b342c 100644 (file)
@@ -22,8 +22,6 @@ import com.google.common.base.Preconditions;
 import org.apache.curator.CuratorZookeeperClient;
 import org.apache.curator.RetryLoop;
 import org.apache.curator.utils.ThreadUtils;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 import java.util.concurrent.Callable;
 
 /**
@@ -33,7 +31,6 @@ import java.util.concurrent.Callable;
  */
 public class StandardConnectionHandlingPolicy implements ConnectionHandlingPolicy
 {
-    private final Logger log = LoggerFactory.getLogger(getClass());
     private final int expirationPercent;
 
     public StandardConnectionHandlingPolicy()
index 59c30ac..ba37d60 100644 (file)
@@ -32,7 +32,7 @@ import java.util.concurrent.Executors;
 import java.util.concurrent.Semaphore;
 import java.util.concurrent.TimeUnit;
 
-import org.apache.curator.connection.ClassicConnectionHandlingPolicy;
+import org.apache.curator.connection.StandardConnectionHandlingPolicy;
 import org.apache.curator.retry.RetryOneTime;
 import org.apache.curator.utils.EnsurePath;
 import org.apache.zookeeper.ZooKeeper;
@@ -52,7 +52,7 @@ public class TestEnsurePath
         CuratorZookeeperClient  curator = mock(CuratorZookeeperClient.class);
         RetryPolicy             retryPolicy = new RetryOneTime(1);
         RetryLoop               retryLoop = new RetryLoop(retryPolicy, null);
-        when(curator.getConnectionHandlingPolicy()).thenReturn(new ClassicConnectionHandlingPolicy());
+        when(curator.getConnectionHandlingPolicy()).thenReturn(new StandardConnectionHandlingPolicy());
         when(curator.getZooKeeper()).thenReturn(client);
         when(curator.getRetryPolicy()).thenReturn(retryPolicy);
         when(curator.newRetryLoop()).thenReturn(retryLoop);
@@ -78,7 +78,7 @@ public class TestEnsurePath
         RetryPolicy             retryPolicy = new RetryOneTime(1);
         RetryLoop               retryLoop = new RetryLoop(retryPolicy, null);
         final CuratorZookeeperClient  curator = mock(CuratorZookeeperClient.class);
-        when(curator.getConnectionHandlingPolicy()).thenReturn(new ClassicConnectionHandlingPolicy());
+        when(curator.getConnectionHandlingPolicy()).thenReturn(new StandardConnectionHandlingPolicy());
         when(curator.getZooKeeper()).thenReturn(client);
         when(curator.getRetryPolicy()).thenReturn(retryPolicy);
         when(curator.newRetryLoop()).thenReturn(retryLoop);
index 18011aa..a617198 100644 (file)
@@ -21,7 +21,6 @@ package org.apache.curator.framework;
 
 import com.google.common.collect.ImmutableList;
 import org.apache.curator.RetryPolicy;
-import org.apache.curator.connection.ClassicConnectionHandlingPolicy;
 import org.apache.curator.connection.ConnectionHandlingPolicy;
 import org.apache.curator.connection.StandardConnectionHandlingPolicy;
 import org.apache.curator.ensemble.EnsembleProvider;
@@ -145,7 +144,7 @@ public class CuratorFrameworkFactory
         private boolean canBeReadOnly = false;
         private boolean useContainerParentsIfAvailable = true;
         private ConnectionStateErrorPolicy connectionStateErrorPolicy = new StandardConnectionStateErrorPolicy();
-        private ConnectionHandlingPolicy connectionHandlingPolicy = Boolean.getBoolean("curator-use-classic-connection-handling") ? new ClassicConnectionHandlingPolicy() : new StandardConnectionHandlingPolicy();
+        private ConnectionHandlingPolicy connectionHandlingPolicy = new StandardConnectionHandlingPolicy();
         private SchemaSet schemaSet = SchemaSet.getDefaultSchemaSet();
         private boolean zk34CompatibilityMode = isZK34();
 
@@ -408,9 +407,7 @@ public class CuratorFrameworkFactory
          * </p>
          * <p>
          *     <strong>IMPORTANT: </strong> StandardConnectionHandlingPolicy has different behavior than the connection
-         *     policy handling prior to version 3.0.0. You can specify that the connection handling be the method
-         *     prior to 3.0.0 by passing in an instance of {@link ClassicConnectionHandlingPolicy} here or by
-         *     setting the command line value "curator-use-classic-connection-handling" to true (e.g. <tt>-Dcurator-use-classic-connection-handling=true</tt>).
+         *     policy handling prior to version 3.0.0.
          * </p>
          * <p>
          *     Major differences from the older behavior are:
diff --git a/curator-framework/src/main/java/org/apache/curator/framework/imps/ClassicInternalConnectionHandler.java b/curator-framework/src/main/java/org/apache/curator/framework/imps/ClassicInternalConnectionHandler.java
deleted file mode 100644 (file)
index 90a8a24..0000000
+++ /dev/null
@@ -1,70 +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.framework.imps;
-
-import org.apache.curator.framework.state.ConnectionState;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-class ClassicInternalConnectionHandler implements InternalConnectionHandler
-{
-    private final Logger log = LoggerFactory.getLogger(getClass());
-
-    @Override
-    public void checkNewConnection(CuratorFrameworkImpl client)
-    {
-        // NOP
-    }
-
-    @Override
-    public void suspendConnection(CuratorFrameworkImpl client)
-    {
-        if ( client.setToSuspended() )
-        {
-            doSyncForSuspendedConnection(client, client.getZookeeperClient().getInstanceIndex());
-        }
-    }
-
-    private void doSyncForSuspendedConnection(final CuratorFrameworkImpl client, final long instanceIndex)
-    {
-        // we appear to have disconnected, force a new ZK event and see if we can connect to another server
-        final BackgroundOperation<String> operation = new BackgroundSyncImpl(client, null);
-        OperationAndData.ErrorCallback<String> errorCallback = new OperationAndData.ErrorCallback<String>()
-        {
-            @Override
-            public void retriesExhausted(OperationAndData<String> operationAndData)
-            {
-                // if instanceIndex != newInstanceIndex, the ZooKeeper instance was reset/reallocated
-                // so the pending background sync is no longer valid.
-                // if instanceIndex is -1, this is the second try to sync - punt and mark the connection lost
-                if ( (instanceIndex < 0) || (instanceIndex == client.getZookeeperClient().getInstanceIndex()) )
-                {
-                    client.addStateChange(ConnectionState.LOST);
-                }
-                else
-                {
-                    log.debug("suspendConnection() failure ignored as the ZooKeeper instance was reset. Retrying.");
-                    // send -1 to signal that if it happens again, punt and mark the connection lost
-                    doSyncForSuspendedConnection(client, -1);
-                }
-            }
-        };
-        client.performBackgroundOperation(new OperationAndData<String>(operation, "/", null, errorCallback, null, null));
-    }
-}
index d58c56b..7488793 100644 (file)
@@ -132,8 +132,7 @@ public class CuratorFrameworkImpl implements CuratorFramework
                 builder.getConnectionHandlingPolicy()
             );
 
-        boolean isClassic = (builder.getConnectionHandlingPolicy().getSimulatedSessionExpirationPercent() == 0);
-        internalConnectionHandler = isClassic ? new ClassicInternalConnectionHandler() : new StandardInternalConnectionHandler();
+        internalConnectionHandler = new StandardInternalConnectionHandler();
         listeners = new ListenerContainer<CuratorListener>();
         unhandledErrorListeners = new ListenerContainer<UnhandledErrorListener>();
         backgroundOperations = new DelayQueue<OperationAndData<?>>();
index f1bbc3b..e61ee9f 100644 (file)
@@ -174,10 +174,4 @@ public class TestEnabledSessionExpiredState extends BaseClassForTests
 
         Assert.assertNull(states.poll(timing.multiple(.5).milliseconds(), TimeUnit.MILLISECONDS));  // there should be no other events
     }
-
-    @Override
-    protected boolean enabledSessionExpiredStateAware()
-    {
-        return true;
-    }
 }
index a1d6f51..0539dbf 100644 (file)
@@ -75,15 +75,11 @@ public class TestFramework extends BaseClassForTests
     @Test
     public void testSessionLossWithLongTimeout() throws Exception
     {
-
         final Timing timing = new Timing();
-        //Change this to TRUE and the test will pass.
-        System.setProperty("curator-use-classic-connection-handling", Boolean.FALSE.toString());
-                
+
         try(final CuratorFramework client = CuratorFrameworkFactory.newClient(server.getConnectString(), timing.forWaiting().milliseconds(),
                                                                               timing.connection(), new RetryOneTime(1)))
         {
-            
             final CountDownLatch connectedLatch = new CountDownLatch(1);
             final CountDownLatch lostLatch = new CountDownLatch(1);
             final CountDownLatch restartedLatch = new CountDownLatch(1);
@@ -119,10 +115,6 @@ public class TestFramework extends BaseClassForTests
             server.restart();
             Assert.assertTrue(timing.awaitLatch(restartedLatch));
         }
-        finally
-        {
-            System.clearProperty("curator-use-classic-connection-handling");
-        }
     }
 
     @Test
index 6fb946e..cd8e977 100644 (file)
@@ -299,12 +299,6 @@ public class TestSchema extends BaseClassForTests
         }
     }
 
-    @Override
-    protected boolean enabledSessionExpiredStateAware()
-    {
-        return true;
-    }
-
     private CuratorFramework newClient(SchemaSet schemaSet)
     {
         return CuratorFrameworkFactory.builder()
index bfc9cf4..852d9aa 100644 (file)
@@ -86,11 +86,6 @@ public class TestResetConnectionWithBackgroundFailure extends BaseClassForTests
             client.getConnectionStateListenable().addListener(listener1);
             log.debug("Starting ZK server");
             server.restart();
-            if ( Boolean.getBoolean("curator-use-classic-connection-handling") )
-            {
-                Assert.assertEquals(listenerSequence.poll(forWaiting.milliseconds(), TimeUnit.MILLISECONDS), ConnectionState.SUSPENDED);
-                Assert.assertEquals(listenerSequence.poll(forWaiting.milliseconds(), TimeUnit.MILLISECONDS), ConnectionState.LOST);
-            }
             Assert.assertEquals(listenerSequence.poll(forWaiting.milliseconds(), TimeUnit.MILLISECONDS), ConnectionState.CONNECTED);
 
             log.debug("Stopping ZK server");
index 70ddcd5..b984624 100644 (file)
@@ -40,7 +40,7 @@ public class BaseTestTreeCache extends BaseClassForTests
 {
     CuratorFramework client;
     TreeCache cache;
-    private final AtomicBoolean hadBackgroundException = new AtomicBoolean(false);
+    protected final AtomicBoolean hadBackgroundException = new AtomicBoolean(false);
     private final BlockingQueue<TreeCacheEvent> events = new LinkedBlockingQueue<TreeCacheEvent>();
     private final Timing timing = new Timing();
 
index a53627a..ebaf43e 100644 (file)
@@ -23,12 +23,14 @@ import com.google.common.collect.ImmutableSet;
 import org.apache.curator.framework.CuratorFramework;
 import org.apache.curator.framework.api.UnhandledErrorListener;
 import org.apache.curator.framework.recipes.cache.TreeCacheEvent.Type;
+import org.apache.curator.framework.state.ConnectionState;
 import org.apache.curator.test.compatibility.KillSession2;
 import org.apache.curator.utils.CloseableUtils;
 import org.apache.zookeeper.CreateMode;
 import org.testng.Assert;
 import org.testng.annotations.Test;
 import java.util.concurrent.Semaphore;
+import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 
 public class TestTreeCache extends BaseTestTreeCache
index 1c87be4..52446df 100644 (file)
@@ -25,7 +25,6 @@ import org.testng.IInvokedMethod;
 import org.testng.IInvokedMethodListener2;
 import org.testng.IRetryAnalyzer;
 import org.testng.ITestContext;
-import org.testng.ITestNGMethod;
 import org.testng.ITestResult;
 import org.testng.annotations.AfterMethod;
 import org.testng.annotations.BeforeMethod;
@@ -86,7 +85,7 @@ public class BaseClassForTests
     @BeforeSuite(alwaysRun = true)
     public void beforeSuite(ITestContext context)
     {
-        context.getSuite().addListener(new MethodListener(log, enabledSessionExpiredStateAware()));
+        context.getSuite().addListener(new MethodListener(log));
     }
 
     @BeforeMethod
@@ -135,11 +134,6 @@ public class BaseClassForTests
         }
     }
 
-    protected boolean enabledSessionExpiredStateAware()
-    {
-        return false;
-    }
-
     private static class RetryContext
     {
         final AtomicBoolean isRetrying = new AtomicBoolean(false);
@@ -182,14 +176,12 @@ public class BaseClassForTests
     private static class MethodListener implements IInvokedMethodListener2
     {
         private final Logger log;
-        private final boolean sessionExpiredStateAware;
 
         private static final String ATTRIBUTE_NAME = "__curator";
 
-        MethodListener(Logger log, boolean sessionExpiredStateAware)
+        MethodListener(Logger log)
         {
             this.log = log;
-            this.sessionExpiredStateAware = sessionExpiredStateAware;
         }
 
         @Override
@@ -215,12 +207,6 @@ public class BaseClassForTests
                     retryContext = new RetryContext();
                     context.setAttribute(ATTRIBUTE_NAME, retryContext);
                 }
-
-                if ( !sessionExpiredStateAware )
-                {
-                    System.setProperty("curator-use-classic-connection-handling", Boolean.toString(retryContext.runVersion.get() > 0));
-                    log.info("curator-use-classic-connection-handling: " + Boolean.toString(retryContext.runVersion.get() > 0));
-                }
             }
             else if ( method.isTestMethod() )
             {
@@ -235,14 +221,6 @@ public class BaseClassForTests
         @Override
         public void afterInvocation(IInvokedMethod method, ITestResult testResult, ITestContext context)
         {
-            if ( method.getTestMethod().isBeforeSuiteConfiguration() && !sessionExpiredStateAware )
-            {
-                for ( ITestNGMethod testMethod : context.getAllTestMethods() )
-                {
-                    testMethod.setInvocationCount(2);
-                }
-            }
-
             if ( method.isTestMethod() )
             {
                 RetryContext retryContext = (RetryContext)context.getAttribute(ATTRIBUTE_NAME);
@@ -252,7 +230,6 @@ public class BaseClassForTests
                 }
                 else
                 {
-                    System.clearProperty("curator-use-classic-connection-handling");
                     if ( testResult.isSuccess() || (testResult.getStatus() == ITestResult.FAILURE) )
                     {
                         retryContext.isRetrying.set(false);