From 90ec94e86d40a7a74dcb5c09451aca6a3cc696a7 Mon Sep 17 00:00:00 2001 From: Kezhu Wang Date: Sun, 17 Aug 2025 12:20:06 +0800 Subject: [PATCH] ZOOKEEPER-4963: Add `ZooKeeper::builder` to replace `new ZooKeeperBuilder` `ZooKeeper::builder` should be better: 1. More exposure chance as a newly introduce method in class `ZooKeeper`. 2. No need to import or remember the newly introduced class as most `ZooKeeper` instances could be built in one chain. `ZooKeeperBuilder` is introduced in 3.10.0 so it safe to do this. Refs: ZOOKEEPER-4697 --- .../java/org/apache/zookeeper/ZooKeeper.java | 61 ++++++++++++++++--- .../zookeeper/client/ZooKeeperBuilder.java | 19 ++---- .../client/ZooKeeperBuilderTest.java | 5 +- 3 files changed, 59 insertions(+), 26 deletions(-) diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeper.java b/zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeper.java index c6ca3bef0a7..79878e9d98f 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeper.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeper.java @@ -419,6 +419,25 @@ public boolean isConnected() { } } + /** + * Creates a builder with given connect string and session timeout. + * + * @param connectString + * comma separated host:port pairs, each corresponding to a zk + * server. e.g. "127.0.0.1:3000,127.0.0.1:3001,127.0.0.1:3002" + * If the optional chroot suffix is used the example would look + * like: "127.0.0.1:3000,127.0.0.1:3001,127.0.0.1:3002/app/a" + * where the client would be rooted at "/app/a" and all paths + * 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 sessionTimeout + * session timeout + */ + public static ZooKeeperBuilder builder(String connectString, Duration sessionTimeout) { + return new ZooKeeperBuilder(connectString, sessionTimeout); + } + /** * To create a ZooKeeper client object, the application needs to pass a * connection string containing a comma separated list of host:port pairs, @@ -461,9 +480,11 @@ public boolean isConnected() { * in cases of network failure * @throws IllegalArgumentException * if an invalid chroot path is specified + * + * @see #builder(String, Duration) for builder style construction */ public ZooKeeper(String connectString, int sessionTimeout, Watcher watcher) throws IOException { - this(new ZooKeeperBuilder(connectString, Duration.ofMillis(sessionTimeout)) + this(builder(connectString, Duration.ofMillis(sessionTimeout)) .withDefaultWatcher(watcher) .toOptions()); } @@ -512,13 +533,15 @@ public ZooKeeper(String connectString, int sessionTimeout, Watcher watcher) thro * in cases of network failure * @throws IllegalArgumentException * if an invalid chroot path is specified + * + * @see #builder(String, Duration) for builder style construction */ public ZooKeeper( String connectString, int sessionTimeout, Watcher watcher, ZKClientConfig conf) throws IOException { - this(new ZooKeeperBuilder(connectString, Duration.ofMillis(sessionTimeout)) + this(builder(connectString, Duration.ofMillis(sessionTimeout)) .withDefaultWatcher(watcher) .withClientConfig(conf) .toOptions()); @@ -580,6 +603,8 @@ public ZooKeeper( * in cases of network failure * @throws IllegalArgumentException * if an invalid chroot path is specified + * + * @see #builder(String, Duration) for builder style construction */ public ZooKeeper( String connectString, @@ -587,7 +612,7 @@ public ZooKeeper( Watcher watcher, boolean canBeReadOnly, HostProvider aHostProvider) throws IOException { - this(new ZooKeeperBuilder(connectString, Duration.ofMillis(sessionTimeout)) + this(builder(connectString, Duration.ofMillis(sessionTimeout)) .withDefaultWatcher(watcher) .withCanBeReadOnly(canBeReadOnly) .withHostProvider(ignored -> aHostProvider) @@ -652,6 +677,8 @@ public ZooKeeper( * in cases of network failure * @throws IllegalArgumentException * if an invalid chroot path is specified + * + * @see #builder(String, Duration) for builder style construction */ public ZooKeeper( String connectString, @@ -661,7 +688,7 @@ public ZooKeeper( HostProvider hostProvider, ZKClientConfig clientConfig ) throws IOException { - this(new ZooKeeperBuilder(connectString, Duration.ofMillis(sessionTimeout)) + this(builder(connectString, Duration.ofMillis(sessionTimeout)) .withDefaultWatcher(watcher) .withCanBeReadOnly(canBeReadOnly) .withHostProvider(ignored -> hostProvider) @@ -741,13 +768,15 @@ ClientCnxn createConnection( * in cases of network failure * @throws IllegalArgumentException * if an invalid chroot path is specified + * + * @see #builder(String, Duration) for builder style construction */ public ZooKeeper( String connectString, int sessionTimeout, Watcher watcher, boolean canBeReadOnly) throws IOException { - this(new ZooKeeperBuilder(connectString, Duration.ofMillis(sessionTimeout)) + this(builder(connectString, Duration.ofMillis(sessionTimeout)) .withDefaultWatcher(watcher) .withCanBeReadOnly(canBeReadOnly) .toOptions()); @@ -806,6 +835,8 @@ public ZooKeeper( * in cases of network failure * @throws IllegalArgumentException * if an invalid chroot path is specified + * + * @see #builder(String, Duration) for builder style construction */ public ZooKeeper( String connectString, @@ -813,7 +844,7 @@ public ZooKeeper( Watcher watcher, boolean canBeReadOnly, ZKClientConfig conf) throws IOException { - this(new ZooKeeperBuilder(connectString, Duration.ofMillis(sessionTimeout)) + this(builder(connectString, Duration.ofMillis(sessionTimeout)) .withDefaultWatcher(watcher) .withCanBeReadOnly(canBeReadOnly) .withClientConfig(conf) @@ -871,6 +902,8 @@ public ZooKeeper( * @throws IOException in cases of network failure * @throws IllegalArgumentException if an invalid chroot path is specified * @throws IllegalArgumentException for an invalid list of ZooKeeper hosts + * + * @see #builder(String, Duration) for builder style construction */ public ZooKeeper( String connectString, @@ -878,7 +911,7 @@ public ZooKeeper( Watcher watcher, long sessionId, byte[] sessionPasswd) throws IOException { - this(new ZooKeeperBuilder(connectString, Duration.ofMillis(sessionTimeout)) + this(builder(connectString, Duration.ofMillis(sessionTimeout)) .withDefaultWatcher(watcher) .withSession(sessionId, sessionPasswd) .toOptions()); @@ -947,6 +980,8 @@ public ZooKeeper( * use this as HostProvider to enable custom behaviour. * @throws IOException in cases of network failure * @throws IllegalArgumentException if an invalid chroot path is specified + * + * @see #builder(String, Duration) for builder style construction */ public ZooKeeper( String connectString, @@ -956,7 +991,7 @@ public ZooKeeper( byte[] sessionPasswd, boolean canBeReadOnly, HostProvider aHostProvider) throws IOException { - this(new ZooKeeperBuilder(connectString, Duration.ofMillis(sessionTimeout)) + this(builder(connectString, Duration.ofMillis(sessionTimeout)) .withDefaultWatcher(watcher) .withSession(sessionId, sessionPasswd) .withCanBeReadOnly(canBeReadOnly) @@ -1032,6 +1067,8 @@ public ZooKeeper( * @throws IllegalArgumentException if an invalid chroot path is specified * * @since 3.5.5 + * + * @see #builder(String, Duration) for builder style construction */ public ZooKeeper( String connectString, @@ -1042,7 +1079,7 @@ public ZooKeeper( boolean canBeReadOnly, HostProvider hostProvider, ZKClientConfig clientConfig) throws IOException { - this(new ZooKeeperBuilder(connectString, Duration.ofMillis(sessionTimeout)) + this(builder(connectString, Duration.ofMillis(sessionTimeout)) .withSession(sessionId, sessionPasswd) .withDefaultWatcher(watcher) .withCanBeReadOnly(canBeReadOnly) @@ -1054,6 +1091,8 @@ public ZooKeeper( /** * Create a ZooKeeper client and establish session asynchronously. * + *

This is private and export for internal usage. + * *

This constructor will initiate connection to the server and return * immediately - potentially (usually) before the session is fully established. * The watcher from options will be notified of any changes in state. This @@ -1181,6 +1220,8 @@ public ZooKeeper(ZooKeeperOptions options) throws IOException { * majority in the background. * @throws IOException in cases of network failure * @throws IllegalArgumentException if an invalid chroot path is specified + * + * @see #builder(String, Duration) for builder style construction */ public ZooKeeper( String connectString, @@ -1189,7 +1230,7 @@ public ZooKeeper( long sessionId, byte[] sessionPasswd, boolean canBeReadOnly) throws IOException { - this(new ZooKeeperBuilder(connectString, Duration.ofMillis(sessionTimeout)) + this(builder(connectString, Duration.ofMillis(sessionTimeout)) .withDefaultWatcher(watcher) .withSession(sessionId, sessionPasswd) .withCanBeReadOnly(canBeReadOnly) diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/client/ZooKeeperBuilder.java b/zookeeper-server/src/main/java/org/apache/zookeeper/client/ZooKeeperBuilder.java index 36f815ab2da..f484dcfeef5 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/client/ZooKeeperBuilder.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/client/ZooKeeperBuilder.java @@ -47,22 +47,11 @@ public class ZooKeeperBuilder { private ZKClientConfig clientConfig; /** - * Creates a builder with given connect string and session timeout. - * - * @param connectString - * comma separated host:port pairs, each corresponding to a zk - * server. e.g. "127.0.0.1:3000,127.0.0.1:3001,127.0.0.1:3002" - * If the optional chroot suffix is used the example would look - * like: "127.0.0.1:3000,127.0.0.1:3001,127.0.0.1:3002/app/a" - * where the client would be rooted at "/app/a" and all paths - * 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 sessionTimeout - * session timeout + * This is private and export for internal usage. Use {@link ZooKeeper#builder(String, Duration)} instead. */ + @InterfaceAudience.Private public ZooKeeperBuilder(String connectString, Duration sessionTimeout) { - this.connectString = connectString; + this.connectString = Objects.requireNonNull(connectString, "connect string must not be null"); this.sessionTimeout = Objects.requireNonNull(sessionTimeout, "session timeout must not be null"); } @@ -145,6 +134,8 @@ public ZooKeeperBuilder withClientConfig(ZKClientConfig clientConfig) { /** * Creates a {@link ZooKeeperOptions} with configured options. * + *

This is private and export for internal usage. + * * @apiNote helper to delegate existing constructors to {@link ZooKeeper#ZooKeeper(ZooKeeperOptions)} */ @InterfaceAudience.Private diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/client/ZooKeeperBuilderTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/client/ZooKeeperBuilderTest.java index 250982e74cb..0a7b4aae50b 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/client/ZooKeeperBuilderTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/client/ZooKeeperBuilderTest.java @@ -29,6 +29,7 @@ import org.apache.zookeeper.Watcher; import org.apache.zookeeper.ZooDefs; import org.apache.zookeeper.ZooKeeper; +import org.apache.zookeeper.admin.ZooKeeperAdmin; import org.apache.zookeeper.common.Time; import org.apache.zookeeper.test.ClientBase; import org.junit.jupiter.api.Test; @@ -70,7 +71,7 @@ private void testClient(BlockingQueue events, ZooKeeper zk) throws @Test public void testBuildClient() throws Exception { BlockingQueue events = new LinkedBlockingQueue<>(); - ZooKeeper zk = new ZooKeeperBuilder(hostPort, Duration.ofMillis(1000)) + ZooKeeper zk = ZooKeeper.builder(hostPort, Duration.ofMillis(1000)) .withDefaultWatcher(events::offer) .build(); testClient(events, zk); @@ -79,7 +80,7 @@ public void testBuildClient() throws Exception { @Test public void testBuildAdminClient() throws Exception { BlockingQueue events = new LinkedBlockingQueue<>(); - ZooKeeper zk = new ZooKeeperBuilder(hostPort, Duration.ofMillis(1000)) + ZooKeeperAdmin zk = ZooKeeper.builder(hostPort, Duration.ofMillis(1000)) .withDefaultWatcher(events::offer) .buildAdmin(); testClient(events, zk);