Skip to content

Commit

Permalink
CURATOR-599: Configurable ZookeeperFactory by ZKClientConfig
Browse files Browse the repository at this point in the history
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 <liran2000@gmail.com>

Reviewers: Enrico Olivelli <eolivelli@apache.org>, Zili Chen, Cameron McKenzie

Closes #391 from liran2000/CURATOR-599
  • Loading branch information
liran2000 authored and eolivelli committed Jul 15, 2021
1 parent 4dffb5e commit 414a408
Show file tree
Hide file tree
Showing 5 changed files with 217 additions and 42 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/**
* 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.utils;

import org.apache.zookeeper.Watcher;
import org.apache.zookeeper.ZooKeeper;
import org.apache.zookeeper.admin.ZooKeeperAdmin;
import org.apache.zookeeper.client.ZKClientConfig;

/**
* Configurable ZookeeperFactory, by using org.apache.zookeeper.client.ZKClientConfig.
*
*/
public class ConfigurableZookeeperFactory extends DefaultZookeeperFactory
{

public ZooKeeper newZooKeeper(String connectString, int sessionTimeout, Watcher watcher,
boolean canBeReadOnly, ZKClientConfig zkClientConfig) throws Exception
{
return new ZooKeeperAdmin(connectString, sessionTimeout, watcher, canBeReadOnly, zkClientConfig);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@

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
{
Expand All @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -41,15 +49,10 @@
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
Expand Down Expand Up @@ -108,6 +111,29 @@ public static CuratorFramework newClient(String connectString, int sessionTimeou
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
Expand Down Expand Up @@ -150,6 +176,7 @@ public static class Builder
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
Expand Down Expand Up @@ -466,6 +493,11 @@ public Builder simulatedSessionExpirationPercent(int simulatedSessionExpirationP
this.simulatedSessionExpirationPercent = simulatedSessionExpirationPercent;
return this;
}

public Builder zkClientConfig(ZKClientConfig zkClientConfig) {
this.zkClientConfig = zkClientConfig;
return this;
}

/**
* Add an enforced schema set
Expand Down Expand Up @@ -584,6 +616,10 @@ public ConnectionStateErrorPolicy getConnectionStateErrorPolicy()
public int getSimulatedSessionExpirationPercent() {
return simulatedSessionExpirationPercent;
}

public ZKClientConfig getZkClientConfig() {
return zkClientConfig;
}

public SchemaSet getSchemaSet()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -30,7 +46,25 @@
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;
Expand All @@ -51,17 +85,14 @@
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
{
Expand Down Expand Up @@ -108,7 +139,7 @@ interface DebugBackgroundListener

public CuratorFrameworkImpl(CuratorFrameworkFactory.Builder builder)
{
ZookeeperFactory localZookeeperFactory = makeZookeeperFactory(builder.getZookeeperFactory());
ZookeeperFactory localZookeeperFactory = makeZookeeperFactory(builder.getZookeeperFactory(), builder.getZkClientConfig());
this.client = new CuratorZookeeperClient
(
localZookeeperFactory,
Expand Down Expand Up @@ -200,23 +231,26 @@ public QuorumVerifier getCurrentConfig()
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)
{
Expand Down Expand Up @@ -1061,4 +1095,5 @@ private void processEvent(final CuratorEvent curatorEvent)
}
});
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,16 @@
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;
Expand All @@ -50,26 +59,24 @@
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
Expand Down Expand Up @@ -1073,7 +1080,41 @@ public void testCreateModes() throws Exception
}
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");
}
}
}

Expand Down

0 comments on commit 414a408

Please sign in to comment.