You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@curator.apache.org by eo...@apache.org on 2021/07/15 10:29:03 UTC

[curator] branch master updated: CURATOR-599: Configurable ZookeeperFactory by ZKClientConfig

This is an automated email from the ASF dual-hosted git repository.

eolivelli pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/curator.git


The following commit(s) were added to refs/heads/master by this push:
     new 414a408  CURATOR-599: Configurable ZookeeperFactory by ZKClientConfig
414a408 is described below

commit 414a4085c0228c5c4f960c43fb79d06d680eeea4
Author: liran2000 <li...@gmail.com>
AuthorDate: Thu Jul 15 12:28:50 2021 +0200

    CURATOR-599: Configurable ZookeeperFactory by ZKClientConfig
    
    Option to use ZooKeeper client config.
    
    This seems mandatory for using zookeeper.request.timeout for preventing
    the potential race condition of hanging indefinitely, as described at
    the ticket.
    
    Author: liran2000 <li...@gmail.com>
    
    Reviewers: Enrico Olivelli <eo...@apache.org>, Zili Chen, Cameron McKenzie
    
    Closes #391 from liran2000/CURATOR-599
---
 ...tory.java => ConfigurableZookeeperFactory.java} | 30 ++++-----
 .../org/apache/curator/utils/ZookeeperFactory.java | 24 +++++++
 .../curator/framework/CuratorFrameworkFactory.java | 58 ++++++++++++----
 .../framework/imps/CuratorFrameworkImpl.java       | 77 ++++++++++++++++------
 .../curator/framework/imps/TestFramework.java      | 61 ++++++++++++++---
 5 files changed, 192 insertions(+), 58 deletions(-)

diff --git a/curator-client/src/main/java/org/apache/curator/utils/ZookeeperFactory.java b/curator-client/src/main/java/org/apache/curator/utils/ConfigurableZookeeperFactory.java
similarity index 55%
copy from curator-client/src/main/java/org/apache/curator/utils/ZookeeperFactory.java
copy to curator-client/src/main/java/org/apache/curator/utils/ConfigurableZookeeperFactory.java
index 13701b1..5aa0c4b 100644
--- a/curator-client/src/main/java/org/apache/curator/utils/ZookeeperFactory.java
+++ b/curator-client/src/main/java/org/apache/curator/utils/ConfigurableZookeeperFactory.java
@@ -16,26 +16,24 @@
  * specific language governing permissions and limitations
  * under the License.
  */
+
 package org.apache.curator.utils;
 
 import org.apache.zookeeper.Watcher;
 import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.admin.ZooKeeperAdmin;
+import org.apache.zookeeper.client.ZKClientConfig;
 
-public interface ZookeeperFactory
+/**
+ * Configurable ZookeeperFactory, by using org.apache.zookeeper.client.ZKClientConfig.
+ *
+ */
+public class ConfigurableZookeeperFactory extends DefaultZookeeperFactory
 {
-    /**
-     * Allocate a new ZooKeeper instance
-     *
-     *
-     * @param connectString the connection string
-     * @param sessionTimeout session timeout in milliseconds
-     * @param watcher optional watcher
-     * @param canBeReadOnly if true, allow ZooKeeper client to enter
-     *                      read only mode in case of a network partition. See
-     *                      {@link ZooKeeper#ZooKeeper(String, int, Watcher, long, byte[], boolean)}
-     *                      for details
-     * @return the instance
-     * @throws Exception errors
-     */
-    public ZooKeeper        newZooKeeper(String connectString, int sessionTimeout, Watcher watcher, boolean canBeReadOnly) throws Exception;
+	
+    public ZooKeeper newZooKeeper(String connectString, int sessionTimeout, Watcher watcher, 
+		boolean canBeReadOnly, ZKClientConfig zkClientConfig) throws Exception
+    {
+		return new ZooKeeperAdmin(connectString, sessionTimeout, watcher, canBeReadOnly, zkClientConfig);
+    }
 }
diff --git a/curator-client/src/main/java/org/apache/curator/utils/ZookeeperFactory.java b/curator-client/src/main/java/org/apache/curator/utils/ZookeeperFactory.java
index 13701b1..1d48fac 100644
--- a/curator-client/src/main/java/org/apache/curator/utils/ZookeeperFactory.java
+++ b/curator-client/src/main/java/org/apache/curator/utils/ZookeeperFactory.java
@@ -20,6 +20,8 @@ package org.apache.curator.utils;
 
 import org.apache.zookeeper.Watcher;
 import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.admin.ZooKeeperAdmin;
+import org.apache.zookeeper.client.ZKClientConfig;
 
 public interface ZookeeperFactory
 {
@@ -38,4 +40,26 @@ public interface ZookeeperFactory
      * @throws Exception errors
      */
     public ZooKeeper        newZooKeeper(String connectString, int sessionTimeout, Watcher watcher, boolean canBeReadOnly) throws Exception;
+    
+    /**
+     * Allocate a new ZooKeeper instance
+     *
+     *
+     * @param connectString the connection string
+     * @param sessionTimeout session timeout in milliseconds
+     * @param watcher optional watcher
+     * @param canBeReadOnly if true, allow ZooKeeper client to enter
+     *                      read only mode in case of a network partition. See
+     *                      {@link ZooKeeper#ZooKeeper(String, int, Watcher, long, byte[], boolean)}
+     *                      for details
+     * @param zkClientConfig ZooKeeper client config
+     * @return the instance
+     * @throws Exception errors
+     */
+    public default ZooKeeper newZooKeeper(String connectString, int sessionTimeout, Watcher watcher, boolean canBeReadOnly, ZKClientConfig zkClientConfig) throws Exception {
+		if (zkClientConfig == null) {
+			return newZooKeeper(connectString, sessionTimeout, watcher, canBeReadOnly);
+		}
+    	return new ZooKeeperAdmin(connectString, sessionTimeout, watcher, canBeReadOnly, zkClientConfig);
+    }
 }
diff --git a/curator-framework/src/main/java/org/apache/curator/framework/CuratorFrameworkFactory.java b/curator-framework/src/main/java/org/apache/curator/framework/CuratorFrameworkFactory.java
index ca5b203..02b4552 100644
--- a/curator-framework/src/main/java/org/apache/curator/framework/CuratorFrameworkFactory.java
+++ b/curator-framework/src/main/java/org/apache/curator/framework/CuratorFrameworkFactory.java
@@ -19,8 +19,16 @@
 
 package org.apache.curator.framework;
 
-import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableList;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.concurrent.Executor;
+import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.curator.CuratorZookeeperClient;
 import org.apache.curator.RetryPolicy;
 import org.apache.curator.ensemble.EnsembleProvider;
 import org.apache.curator.ensemble.fixed.FixedEnsembleProvider;
@@ -41,15 +49,10 @@ import org.apache.curator.utils.ZookeeperFactory;
 import org.apache.zookeeper.CreateMode;
 import org.apache.zookeeper.Watcher;
 import org.apache.zookeeper.ZooKeeper;
-import java.net.InetAddress;
-import java.net.UnknownHostException;
-import java.util.Arrays;
-import java.util.List;
-import java.util.Objects;
-import java.util.concurrent.Executor;
-import java.util.concurrent.ThreadFactory;
-import java.util.concurrent.TimeUnit;
-import org.apache.curator.CuratorZookeeperClient;
+import org.apache.zookeeper.client.ZKClientConfig;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
 
 /**
  * Factory methods for creating framework-style clients
@@ -108,6 +111,29 @@ public class CuratorFrameworkFactory
             retryPolicy(retryPolicy).
             build();
     }
+    
+    /**
+     * Create a new client
+     *
+     * @param connectString       list of servers to connect to
+     * @param sessionTimeoutMs    session timeout
+     * @param connectionTimeoutMs connection timeout
+     * @param retryPolicy         retry policy to use
+     * @param zkClientConfig      ZKClientConfig
+     * @return client
+     * 
+     * @since 5.1.1, supported from ZooKeeper 3.6.1 and above.
+     */
+    public static CuratorFramework newClient(String connectString, int sessionTimeoutMs, int connectionTimeoutMs, RetryPolicy retryPolicy, ZKClientConfig zkClientConfig)
+    {
+        return builder().
+            connectString(connectString).
+            sessionTimeoutMs(sessionTimeoutMs).
+            connectionTimeoutMs(connectionTimeoutMs).
+            retryPolicy(retryPolicy).
+            zkClientConfig(zkClientConfig).
+            build();
+    }
 
     /**
      * Return the local address as bytes that can be used as a node payload
@@ -150,6 +176,7 @@ public class CuratorFrameworkFactory
         private Executor runSafeService = null;
         private ConnectionStateListenerManagerFactory connectionStateListenerManagerFactory = ConnectionStateListenerManagerFactory.standard;
         private int simulatedSessionExpirationPercent = 100;
+        private ZKClientConfig zkClientConfig;
 
         /**
          * Apply the current values and build a new CuratorFramework
@@ -466,6 +493,11 @@ public class CuratorFrameworkFactory
             this.simulatedSessionExpirationPercent = simulatedSessionExpirationPercent;
             return this;
         }
+        
+        public Builder zkClientConfig(ZKClientConfig zkClientConfig) {
+        	this.zkClientConfig = zkClientConfig;
+        	return this;
+        }
 
         /**
          * Add an enforced schema set
@@ -584,6 +616,10 @@ public class CuratorFrameworkFactory
         public int getSimulatedSessionExpirationPercent() {
             return simulatedSessionExpirationPercent;
         }
+        
+        public ZKClientConfig getZkClientConfig() {
+            return zkClientConfig;
+        }
 
         public SchemaSet getSchemaSet()
         {
diff --git a/curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java b/curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
index 218175a..9f4a827 100644
--- a/curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
+++ b/curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
@@ -19,9 +19,25 @@
 
 package org.apache.curator.framework.imps;
 
-import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableList;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.List;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.Callable;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.DelayQueue;
+import java.util.concurrent.Executor;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.concurrent.atomic.AtomicReference;
+
 import org.apache.curator.CuratorConnectionLossException;
 import org.apache.curator.CuratorZookeeperClient;
 import org.apache.curator.RetryLoop;
@@ -30,7 +46,25 @@ import org.apache.curator.framework.AuthInfo;
 import org.apache.curator.framework.CuratorFramework;
 import org.apache.curator.framework.CuratorFrameworkFactory;
 import org.apache.curator.framework.WatcherRemoveCuratorFramework;
-import org.apache.curator.framework.api.*;
+import org.apache.curator.framework.api.ACLProvider;
+import org.apache.curator.framework.api.CompressionProvider;
+import org.apache.curator.framework.api.CreateBuilder;
+import org.apache.curator.framework.api.CuratorEvent;
+import org.apache.curator.framework.api.CuratorEventType;
+import org.apache.curator.framework.api.CuratorListener;
+import org.apache.curator.framework.api.DeleteBuilder;
+import org.apache.curator.framework.api.ExistsBuilder;
+import org.apache.curator.framework.api.GetACLBuilder;
+import org.apache.curator.framework.api.GetChildrenBuilder;
+import org.apache.curator.framework.api.GetConfigBuilder;
+import org.apache.curator.framework.api.GetDataBuilder;
+import org.apache.curator.framework.api.ReconfigBuilder;
+import org.apache.curator.framework.api.RemoveWatchesBuilder;
+import org.apache.curator.framework.api.SetACLBuilder;
+import org.apache.curator.framework.api.SetDataBuilder;
+import org.apache.curator.framework.api.SyncBuilder;
+import org.apache.curator.framework.api.UnhandledErrorListener;
+import org.apache.curator.framework.api.WatchesBuilder;
 import org.apache.curator.framework.api.transaction.CuratorMultiTransaction;
 import org.apache.curator.framework.api.transaction.CuratorTransaction;
 import org.apache.curator.framework.api.transaction.TransactionOp;
@@ -51,17 +85,14 @@ import org.apache.zookeeper.KeeperException;
 import org.apache.zookeeper.WatchedEvent;
 import org.apache.zookeeper.Watcher;
 import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.client.ZKClientConfig;
 import org.apache.zookeeper.server.quorum.flexible.QuorumVerifier;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.List;
-import java.util.concurrent.*;
-import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.concurrent.atomic.AtomicLong;
-import java.util.concurrent.atomic.AtomicReference;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
 
 public class CuratorFrameworkImpl implements CuratorFramework
 {
@@ -108,7 +139,7 @@ public class CuratorFrameworkImpl implements CuratorFramework
 
     public CuratorFrameworkImpl(CuratorFrameworkFactory.Builder builder)
     {
-        ZookeeperFactory localZookeeperFactory = makeZookeeperFactory(builder.getZookeeperFactory());
+        ZookeeperFactory localZookeeperFactory = makeZookeeperFactory(builder.getZookeeperFactory(), builder.getZkClientConfig());
         this.client = new CuratorZookeeperClient
             (
                 localZookeeperFactory,
@@ -200,23 +231,26 @@ public class CuratorFrameworkImpl implements CuratorFramework
         return (ensembleTracker != null) ? ensembleTracker.getCurrentConfig() : null;
     }
 
-    private ZookeeperFactory makeZookeeperFactory(final ZookeeperFactory actualZookeeperFactory)
+    private ZookeeperFactory makeZookeeperFactory(final ZookeeperFactory actualZookeeperFactory, ZKClientConfig zkClientConfig)
     {
-        return new ZookeeperFactory()
+    	return new ZookeeperFactory()
         {
             @Override
             public ZooKeeper newZooKeeper(String connectString, int sessionTimeout, Watcher watcher, boolean canBeReadOnly) throws Exception
             {
-                ZooKeeper zooKeeper = actualZookeeperFactory.newZooKeeper(connectString, sessionTimeout, watcher, canBeReadOnly);
-                for ( AuthInfo auth : authInfos )
-                {
-                    zooKeeper.addAuthInfo(auth.getScheme(), auth.getAuth());
-                }
-
+                ZooKeeper zooKeeper = actualZookeeperFactory.newZooKeeper(connectString, sessionTimeout, watcher, canBeReadOnly, zkClientConfig);
+                addAuthInfos(zooKeeper);
                 return zooKeeper;
             }
         };
     }
+    
+    private void addAuthInfos(ZooKeeper zooKeeper) {
+		for ( AuthInfo auth: authInfos)
+		{
+		    zooKeeper.addAuthInfo(auth.getScheme(), auth.getAuth());
+		}
+	}
 
     private ThreadFactory getThreadFactory(CuratorFrameworkFactory.Builder builder)
     {
@@ -1061,4 +1095,5 @@ public class CuratorFrameworkImpl implements CuratorFramework
             }
         });
     }
+	
 }
diff --git a/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFramework.java b/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFramework.java
index cd24fa2..3e5b94a 100644
--- a/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFramework.java
+++ b/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFramework.java
@@ -25,7 +25,16 @@ import static org.junit.jupiter.api.Assertions.assertNotSame;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
-import com.google.common.collect.Lists;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ArrayBlockingQueue;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.TimeUnit;
+
 import org.apache.curator.framework.AuthInfo;
 import org.apache.curator.framework.CuratorFramework;
 import org.apache.curator.framework.CuratorFrameworkFactory;
@@ -50,26 +59,24 @@ import org.apache.zookeeper.WatchedEvent;
 import org.apache.zookeeper.Watcher;
 import org.apache.zookeeper.ZooDefs;
 import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.client.ZKClientConfig;
 import org.apache.zookeeper.data.ACL;
 import org.apache.zookeeper.data.Stat;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Tag;
 import org.junit.jupiter.api.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.List;
-import java.util.concurrent.ArrayBlockingQueue;
-import java.util.concurrent.BlockingQueue;
-import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.LinkedBlockingQueue;
-import java.util.concurrent.TimeUnit;
+import com.google.common.collect.Lists;
 
 @SuppressWarnings("deprecation")
 @Tag(CuratorTestBase.zk35TestCompatibilityGroup)
 public class TestFramework extends BaseClassForTests
 {
+	private final Logger log = LoggerFactory.getLogger(getClass());
+	
     @BeforeEach
     @Override
     public void setup() throws Exception
@@ -1073,7 +1080,41 @@ public class TestFramework extends BaseClassForTests
         }
         finally
         {
-            CloseableUtils.closeQuietly(client);
+        	CloseableUtils.closeQuietly(client);
+        }
+    }
+    
+    @Test
+    public void testConfigurableZookeeper() throws Exception
+    {
+    	CuratorFramework client = null;
+        try
+        {
+        	ZKClientConfig zkClientConfig = new ZKClientConfig();
+    		String zookeeperRequestTimeout = "30000";
+    		zkClientConfig.setProperty(ZKClientConfig.ZOOKEEPER_REQUEST_TIMEOUT, zookeeperRequestTimeout);
+    		client = CuratorFrameworkFactory.newClient(server.getConnectString(), 30000, 30000, new RetryOneTime(1), zkClientConfig);
+            client.start();
+        	
+            byte[] writtenBytes = {1, 2, 3};
+            client.create().forPath("/test", writtenBytes);
+
+            byte[] readBytes = client.getData().forPath("/test");
+            assertArrayEquals(writtenBytes, readBytes);
+            assertEquals(zookeeperRequestTimeout, client.getZookeeperClient().getZooKeeper().getClientConfig().getProperty(ZKClientConfig.ZOOKEEPER_REQUEST_TIMEOUT));
+
+        } catch (NoSuchMethodError e) {
+            log.debug("NoSuchMethodError: ", e);
+            log.info("Got NoSuchMethodError, meaning probably this cannot be used with ZooKeeper version < 3.6.1");
+		}
+        finally
+        {
+        	try {
+				CloseableUtils.closeQuietly(client);
+			} catch (NoSuchMethodError e) {
+				log.debug("close: NoSuchMethodError: ", e);
+				log.info("close: Got NoSuchMethodError, meaning probably this cannot be used with ZooKeeper version < 3.6.1");
+			}
         }
     }