Skip to content

ZOOKEEPER-4943: Use Duration for session timeout in ZooKeeperBuilder #2274

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 12 additions & 11 deletions zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeper.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.lang.reflect.Constructor;
import java.net.InetSocketAddress;
import java.net.SocketAddress;
import java.time.Duration;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
Expand Down Expand Up @@ -462,7 +463,7 @@ public boolean isConnected() {
* if an invalid chroot path is specified
*/
public ZooKeeper(String connectString, int sessionTimeout, Watcher watcher) throws IOException {
this(new ZooKeeperBuilder(connectString, sessionTimeout)
this(new ZooKeeperBuilder(connectString, Duration.ofMillis(sessionTimeout))
.withDefaultWatcher(watcher)
.toOptions());
}
Expand Down Expand Up @@ -517,7 +518,7 @@ public ZooKeeper(
int sessionTimeout,
Watcher watcher,
ZKClientConfig conf) throws IOException {
this(new ZooKeeperBuilder(connectString, sessionTimeout)
this(new ZooKeeperBuilder(connectString, Duration.ofMillis(sessionTimeout))
.withDefaultWatcher(watcher)
.withClientConfig(conf)
.toOptions());
Expand Down Expand Up @@ -586,7 +587,7 @@ public ZooKeeper(
Watcher watcher,
boolean canBeReadOnly,
HostProvider aHostProvider) throws IOException {
this(new ZooKeeperBuilder(connectString, sessionTimeout)
this(new ZooKeeperBuilder(connectString, Duration.ofMillis(sessionTimeout))
.withDefaultWatcher(watcher)
.withCanBeReadOnly(canBeReadOnly)
.withHostProvider(ignored -> aHostProvider)
Expand Down Expand Up @@ -660,7 +661,7 @@ public ZooKeeper(
HostProvider hostProvider,
ZKClientConfig clientConfig
) throws IOException {
this(new ZooKeeperBuilder(connectString, sessionTimeout)
this(new ZooKeeperBuilder(connectString, Duration.ofMillis(sessionTimeout))
.withDefaultWatcher(watcher)
.withCanBeReadOnly(canBeReadOnly)
.withHostProvider(ignored -> hostProvider)
Expand Down Expand Up @@ -746,7 +747,7 @@ public ZooKeeper(
int sessionTimeout,
Watcher watcher,
boolean canBeReadOnly) throws IOException {
this(new ZooKeeperBuilder(connectString, sessionTimeout)
this(new ZooKeeperBuilder(connectString, Duration.ofMillis(sessionTimeout))
.withDefaultWatcher(watcher)
.withCanBeReadOnly(canBeReadOnly)
.toOptions());
Expand Down Expand Up @@ -812,7 +813,7 @@ public ZooKeeper(
Watcher watcher,
boolean canBeReadOnly,
ZKClientConfig conf) throws IOException {
this(new ZooKeeperBuilder(connectString, sessionTimeout)
this(new ZooKeeperBuilder(connectString, Duration.ofMillis(sessionTimeout))
.withDefaultWatcher(watcher)
.withCanBeReadOnly(canBeReadOnly)
.withClientConfig(conf)
Expand Down Expand Up @@ -877,7 +878,7 @@ public ZooKeeper(
Watcher watcher,
long sessionId,
byte[] sessionPasswd) throws IOException {
this(new ZooKeeperBuilder(connectString, sessionTimeout)
this(new ZooKeeperBuilder(connectString, Duration.ofMillis(sessionTimeout))
.withDefaultWatcher(watcher)
.withSession(sessionId, sessionPasswd)
.toOptions());
Expand Down Expand Up @@ -955,7 +956,7 @@ public ZooKeeper(
byte[] sessionPasswd,
boolean canBeReadOnly,
HostProvider aHostProvider) throws IOException {
this(new ZooKeeperBuilder(connectString, sessionTimeout)
this(new ZooKeeperBuilder(connectString, Duration.ofMillis(sessionTimeout))
.withDefaultWatcher(watcher)
.withSession(sessionId, sessionPasswd)
.withCanBeReadOnly(canBeReadOnly)
Expand Down Expand Up @@ -1041,7 +1042,7 @@ public ZooKeeper(
boolean canBeReadOnly,
HostProvider hostProvider,
ZKClientConfig clientConfig) throws IOException {
this(new ZooKeeperBuilder(connectString, sessionTimeout)
this(new ZooKeeperBuilder(connectString, Duration.ofMillis(sessionTimeout))
.withSession(sessionId, sessionPasswd)
.withDefaultWatcher(watcher)
.withCanBeReadOnly(canBeReadOnly)
Expand Down Expand Up @@ -1072,7 +1073,7 @@ public ZooKeeper(
@InterfaceAudience.Private
public ZooKeeper(ZooKeeperOptions options) throws IOException {
String connectString = options.getConnectString();
int sessionTimeout = options.getSessionTimeout();
int sessionTimeout = options.getSessionTimeoutMs();
long sessionId = options.getSessionId();
byte[] sessionPasswd = sessionId == 0 ? new byte[16] : options.getSessionPasswd();
Watcher watcher = options.getDefaultWatcher();
Expand Down Expand Up @@ -1188,7 +1189,7 @@ public ZooKeeper(
long sessionId,
byte[] sessionPasswd,
boolean canBeReadOnly) throws IOException {
this(new ZooKeeperBuilder(connectString, sessionTimeout)
this(new ZooKeeperBuilder(connectString, Duration.ofMillis(sessionTimeout))
.withDefaultWatcher(watcher)
.withSession(sessionId, sessionPasswd)
.withCanBeReadOnly(canBeReadOnly)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import java.io.IOException;
import java.net.InetSocketAddress;
import java.time.Duration;
import java.util.Collection;
import java.util.Objects;
import java.util.function.Function;
import org.apache.yetus.audience.InterfaceAudience;
import org.apache.yetus.audience.InterfaceStability;
Expand All @@ -36,7 +38,7 @@
@InterfaceStability.Evolving
public class ZooKeeperBuilder {
private final String connectString;
private final int sessionTimeout;
private final Duration sessionTimeout;
private Function<Collection<InetSocketAddress>, HostProvider> hostProvider;
private Watcher defaultWatcher;
private boolean canBeReadOnly = false;
Expand All @@ -56,12 +58,12 @@ public class ZooKeeperBuilder {
* would be relative to this root - ie getting/setting/etc...
* "/foo/bar" would result in operations being run on
* "/app/a/foo/bar" (from the server perspective).
* @param sessionTimeoutMs
* session timeout in milliseconds
* @param sessionTimeout
* session timeout
*/
public ZooKeeperBuilder(String connectString, int sessionTimeoutMs) {
public ZooKeeperBuilder(String connectString, Duration sessionTimeout) {
this.connectString = connectString;
this.sessionTimeout = sessionTimeoutMs;
this.sessionTimeout = Objects.requireNonNull(sessionTimeout, "session timeout must not be null");
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import java.net.InetSocketAddress;
import java.time.Duration;
import java.util.Collection;
import java.util.function.Function;
import org.apache.yetus.audience.InterfaceAudience;
Expand All @@ -31,7 +32,7 @@
@InterfaceAudience.Private
public class ZooKeeperOptions {
private final String connectString;
private final int sessionTimeout;
private final Duration sessionTimeout;
private final Watcher defaultWatcher;
private final Function<Collection<InetSocketAddress>, HostProvider> hostProvider;
private final boolean canBeReadOnly;
Expand All @@ -40,7 +41,7 @@ public class ZooKeeperOptions {
private final ZKClientConfig clientConfig;

ZooKeeperOptions(String connectString,
int sessionTimeout,
Duration sessionTimeout,
Watcher defaultWatcher,
Function<Collection<InetSocketAddress>, HostProvider> hostProvider,
boolean canBeReadOnly,
Expand All @@ -61,8 +62,8 @@ public String getConnectString() {
return connectString;
}

public int getSessionTimeout() {
return sessionTimeout;
public int getSessionTimeoutMs() {
return (int) Long.min(Integer.MAX_VALUE, sessionTimeout.toMillis());
}

public Watcher getDefaultWatcher() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import java.time.Duration;
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.TimeUnit;
Expand Down Expand Up @@ -69,7 +70,7 @@ private void testClient(BlockingQueue<WatchedEvent> events, ZooKeeper zk) throws
@Test
public void testBuildClient() throws Exception {
BlockingQueue<WatchedEvent> events = new LinkedBlockingQueue<>();
ZooKeeper zk = new ZooKeeperBuilder(hostPort, 1000)
ZooKeeper zk = new ZooKeeperBuilder(hostPort, Duration.ofMillis(1000))
.withDefaultWatcher(events::offer)
.build();
testClient(events, zk);
Expand All @@ -78,7 +79,7 @@ public void testBuildClient() throws Exception {
@Test
public void testBuildAdminClient() throws Exception {
BlockingQueue<WatchedEvent> events = new LinkedBlockingQueue<>();
ZooKeeper zk = new ZooKeeperBuilder(hostPort, 1000)
ZooKeeper zk = new ZooKeeperBuilder(hostPort, Duration.ofMillis(1000))
.withDefaultWatcher(events::offer)
.buildAdmin();
testClient(events, zk);
Expand Down