This is an automated email from the ASF dual-hosted git repository.
zhangduo pushed a commit to branch branch-2
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/branch-2 by this push:
new 8b5d059ee84 HBASE-28529 Use ZKClientConfig instead of system
properties when setting zookeeper configurations (#5835)
8b5d059ee84 is described below
commit 8b5d059ee84209c77f195da0f3f2ee9468fe6ce6
Author: Duo Zhang <zhang...@apache.org>
AuthorDate: Tue Apr 23 21:57:47 2024 +0800
HBASE-28529 Use ZKClientConfig instead of system properties when setting
zookeeper configurations (#5835)
Signed-off-by: Wellington Chevreuil <wchevre...@apache.org>
Reviewed-by: Andor Molnár <an...@apache.org>
Reviewed-by: BukrosSzabolcs <szabo...@cloudera.com>
(cherry picked from commit 6c6e776eea6ebd62a3a030a1820c4eef2636553c)
---
.../hadoop/hbase/zookeeper/ReadOnlyZKClient.java | 13 ++--
.../apache/hadoop/hbase/zookeeper/ZKConfig.java | 29 ++++-----
.../hadoop/hbase/zookeeper/TestZKConfig.java | 47 ++------------
.../hbase/zookeeper/RecoverableZooKeeper.java | 75 ++++++++++------------
.../apache/hadoop/hbase/zookeeper/ZKWatcher.java | 4 +-
.../hbase/zookeeper/TestRecoverableZooKeeper.java | 2 +-
6 files changed, 62 insertions(+), 108 deletions(-)
diff --git
a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ReadOnlyZKClient.java
b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ReadOnlyZKClient.java
index 979094fda80..64b151dc19a 100644
---
a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ReadOnlyZKClient.java
+++
b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ReadOnlyZKClient.java
@@ -38,6 +38,7 @@ import org.apache.yetus.audience.InterfaceAudience;
import org.apache.zookeeper.KeeperException;
import org.apache.zookeeper.KeeperException.Code;
import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.client.ZKClientConfig;
import org.apache.zookeeper.data.Stat;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -75,6 +76,8 @@ public final class ReadOnlyZKClient implements Closeable {
private final int keepAliveTimeMs;
+ private final ZKClientConfig zkClientConfig;
+
private static abstract class Task implements Delayed {
protected long time = System.nanoTime();
@@ -136,10 +139,12 @@ public final class ReadOnlyZKClient implements Closeable {
this.retryIntervalMs =
conf.getInt(RECOVERY_RETRY_INTERVAL_MILLIS,
DEFAULT_RECOVERY_RETRY_INTERVAL_MILLIS);
this.keepAliveTimeMs = conf.getInt(KEEPALIVE_MILLIS,
DEFAULT_KEEPALIVE_MILLIS);
+ this.zkClientConfig = ZKConfig.getZKClientConfig(conf);
LOG.debug(
- "Connect {} to {} with session timeout={}ms, retries {}, "
- + "retry interval {}ms, keepAlive={}ms",
- getId(), connectString, sessionTimeoutMs, maxRetries, retryIntervalMs,
keepAliveTimeMs);
+ "Connect {} to {} with session timeout={}ms, retries={}, "
+ + "retry interval={}ms, keepAlive={}ms, zk client config={}",
+ getId(), connectString, sessionTimeoutMs, maxRetries, retryIntervalMs,
keepAliveTimeMs,
+ zkClientConfig);
Threads.setDaemonThreadRunning(new Thread(this::run),
"ReadOnlyZKClient-" + connectString + "@" + getId());
}
@@ -316,7 +321,7 @@ public final class ReadOnlyZKClient implements Closeable {
// may be closed when session expired
if (zookeeper == null || !zookeeper.getState().isAlive()) {
zookeeper = new ZooKeeper(connectString, sessionTimeoutMs, e -> {
- });
+ }, zkClientConfig);
}
return zookeeper;
}
diff --git
a/hbase-common/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKConfig.java
b/hbase-common/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKConfig.java
index 5c24418214b..0edde8da266 100644
--- a/hbase-common/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKConfig.java
+++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKConfig.java
@@ -25,19 +25,22 @@ import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.HConstants;
import org.apache.hadoop.util.StringUtils;
import org.apache.yetus.audience.InterfaceAudience;
+import org.apache.zookeeper.client.ZKClientConfig;
import org.apache.hbase.thirdparty.com.google.common.base.Splitter;
/**
* Utility methods for reading, and building the ZooKeeper configuration. The
order and priority for
- * reading the config are as follows: (1). Property with
"hbase.zookeeper.property." prefix from
- * HBase XML (2). other zookeeper related properties in HBASE XML
+ * reading the config are as follows:
+ * <ol>
+ * <li>Property with "hbase.zookeeper.property." prefix from HBase XML.</li>
+ * <li>other zookeeper related properties in HBASE XML</li>
+ * </ol>
*/
@InterfaceAudience.Private
public final class ZKConfig {
private static final String VARIABLE_START = "${";
- private static final String ZOOKEEPER_JAVA_PROPERTY_PREFIX = "zookeeper.";
private ZKConfig() {
}
@@ -131,7 +134,6 @@ public final class ZKConfig {
* @return Quorum servers
*/
public static String getZKQuorumServersString(Configuration conf) {
- setZooKeeperClientSystemProperties(HConstants.ZK_CFG_PROPERTY_PREFIX,
conf);
return getZKQuorumServersStringFromHbaseConfig(conf);
}
@@ -299,13 +301,19 @@ public final class ZKConfig {
}
}
+ public static ZKClientConfig getZKClientConfig(Configuration conf) {
+ Properties zkProperties = extractZKPropsFromHBaseConfig(conf);
+ ZKClientConfig zkClientConfig = new ZKClientConfig();
+ zkProperties.forEach((k, v) -> zkClientConfig.setProperty(k.toString(),
v.toString()));
+ return zkClientConfig;
+ }
+
/**
* Get the client ZK Quorum servers string
* @param conf the configuration to read
* @return Client quorum servers, or null if not specified
*/
public static String getClientZKQuorumServersString(Configuration conf) {
- setZooKeeperClientSystemProperties(HConstants.ZK_CFG_PROPERTY_PREFIX,
conf);
String clientQuromServers = conf.get(HConstants.CLIENT_ZOOKEEPER_QUORUM);
if (clientQuromServers == null) {
return null;
@@ -318,15 +326,4 @@ public final class ZKConfig {
final String[] serverHosts = StringUtils.getStrings(clientQuromServers);
return buildZKQuorumServerString(serverHosts, clientZkClientPort);
}
-
- private static void setZooKeeperClientSystemProperties(String prefix,
Configuration conf) {
- Properties zkProperties = extractZKPropsFromHBaseConfig(conf);
- for (Entry<Object, Object> entry : zkProperties.entrySet()) {
- String key = entry.getKey().toString().trim();
- String value = entry.getValue().toString().trim();
- if (System.getProperty(ZOOKEEPER_JAVA_PROPERTY_PREFIX + key) == null) {
- System.setProperty(ZOOKEEPER_JAVA_PROPERTY_PREFIX + key, value);
- }
- }
- }
}
diff --git
a/hbase-common/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKConfig.java
b/hbase-common/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKConfig.java
index 63df9043bae..2a7b7bc2768 100644
---
a/hbase-common/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKConfig.java
+++
b/hbase-common/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKConfig.java
@@ -29,6 +29,7 @@ import org.apache.hadoop.hbase.HBaseConfiguration;
import org.apache.hadoop.hbase.HConstants;
import org.apache.hadoop.hbase.testclassification.MiscTests;
import org.apache.hadoop.hbase.testclassification.SmallTests;
+import org.apache.zookeeper.client.ZKClientConfig;
import org.junit.ClassRule;
import org.junit.Test;
import org.junit.experimental.categories.Category;
@@ -100,62 +101,22 @@ public class TestZKConfig {
}
@Test
- public void testZooKeeperTlsPropertiesClient() {
+ public void testZooKeeperTlsProperties() {
// Arrange
Configuration conf = HBaseConfiguration.create();
for (String p : ZOOKEEPER_CLIENT_TLS_PROPERTIES) {
conf.set(HConstants.ZK_CFG_PROPERTY_PREFIX + p, p);
- String zkprop = "zookeeper." + p;
- System.clearProperty(zkprop);
}
// Act
- ZKConfig.getClientZKQuorumServersString(conf);
+ ZKClientConfig zkClientConfig = ZKConfig.getZKClientConfig(conf);
// Assert
for (String p : ZOOKEEPER_CLIENT_TLS_PROPERTIES) {
- String zkprop = "zookeeper." + p;
- assertEquals("Invalid or unset system property: " + zkprop, p,
System.getProperty(zkprop));
- System.clearProperty(zkprop);
+ assertEquals("Invalid or unset system property: " + p, p,
zkClientConfig.getProperty(p));
}
}
- @Test
- public void testZooKeeperTlsPropertiesServer() {
- // Arrange
- Configuration conf = HBaseConfiguration.create();
- for (String p : ZOOKEEPER_CLIENT_TLS_PROPERTIES) {
- conf.set(HConstants.ZK_CFG_PROPERTY_PREFIX + p, p);
- String zkprop = "zookeeper." + p;
- System.clearProperty(zkprop);
- }
-
- // Act
- ZKConfig.getZKQuorumServersString(conf);
-
- // Assert
- for (String p : ZOOKEEPER_CLIENT_TLS_PROPERTIES) {
- String zkprop = "zookeeper." + p;
- assertEquals("Invalid or unset system property: " + zkprop, p,
System.getProperty(zkprop));
- System.clearProperty(zkprop);
- }
- }
-
- @Test
- public void testZooKeeperPropertiesDoesntOverwriteSystem() {
- // Arrange
- System.setProperty("zookeeper.a.b.c", "foo");
- Configuration conf = HBaseConfiguration.create();
- conf.set(HConstants.ZK_CFG_PROPERTY_PREFIX + "a.b.c", "bar");
-
- // Act
- ZKConfig.getZKQuorumServersString(conf);
-
- // Assert
- assertEquals("foo", System.getProperty("zookeeper.a.b.c"));
- System.clearProperty("zookeeper.a.b.c");
- }
-
private void testKey(String ensemble, int port, String znode) throws
IOException {
testKey(ensemble, port, znode, false); // not support multiple client ports
}
diff --git
a/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/RecoverableZooKeeper.java
b/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/RecoverableZooKeeper.java
index 8ef2c73574e..a70bed2839f 100644
---
a/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/RecoverableZooKeeper.java
+++
b/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/RecoverableZooKeeper.java
@@ -42,6 +42,7 @@ import org.apache.zookeeper.Watcher;
import org.apache.zookeeper.ZooDefs;
import org.apache.zookeeper.ZooKeeper;
import org.apache.zookeeper.ZooKeeper.States;
+import org.apache.zookeeper.client.ZKClientConfig;
import org.apache.zookeeper.data.ACL;
import org.apache.zookeeper.data.Stat;
import org.apache.zookeeper.proto.CreateRequest;
@@ -50,19 +51,22 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/**
- * A zookeeper that can handle 'recoverable' errors. To handle recoverable
errors, developers need
- * to realize that there are two classes of requests: idempotent and
non-idempotent requests. Read
- * requests and unconditional sets and deletes are examples of idempotent
requests, they can be
- * reissued with the same results. (Although, the delete may throw a
NoNodeException on reissue its
- * effect on the ZooKeeper state is the same.) Non-idempotent requests need
special handling,
- * application and library writers need to keep in mind that they may need to
encode information in
- * the data or name of znodes to detect retries. A simple example is a create
that uses a sequence
- * flag. If a process issues a create("/x-", ..., SEQUENCE) and gets a
connection loss exception,
- * that process will reissue another create("/x-", ..., SEQUENCE) and get back
x-111. When the
- * process does a getChildren("/"), it sees x-1,x-30,x-109,x-110,x-111, now it
could be that x-109
- * was the result of the previous create, so the process actually owns both
x-109 and x-111. An easy
- * way around this is to use "x-process id-" when doing the create. If the
process is using an id of
- * 352, before reissuing the create it will do a getChildren("/") and see
"x-222-1", "x-542-30",
+ * A zookeeper that can handle 'recoverable' errors.
+ * <p>
+ * To handle recoverable errors, developers need to realize that there are two
classes of requests:
+ * idempotent and non-idempotent requests. Read requests and unconditional
sets and deletes are
+ * examples of idempotent requests, they can be reissued with the same results.
+ * <p>
+ * (Although, the delete may throw a NoNodeException on reissue its effect on
the ZooKeeper state is
+ * the same.) Non-idempotent requests need special handling, application and
library writers need to
+ * keep in mind that they may need to encode information in the data or name
of znodes to detect
+ * retries. A simple example is a create that uses a sequence flag. If a
process issues a
+ * create("/x-", ..., SEQUENCE) and gets a connection loss exception, that
process will reissue
+ * another create("/x-", ..., SEQUENCE) and get back x-111. When the process
does a
+ * getChildren("/"), it sees x-1,x-30,x-109,x-110,x-111, now it could be that
x-109 was the result
+ * of the previous create, so the process actually owns both x-109 and x-111.
An easy way around
+ * this is to use "x-process id-" when doing the create. If the process is
using an id of 352,
+ * before reissuing the create it will do a getChildren("/") and see
"x-222-1", "x-542-30",
* "x-352-109", x-333-110". The process will know that the original create
succeeded an the znode it
* created is "x-352-109".
* @see "http://wiki.apache.org/hadoop/ZooKeeper/ErrorHandling";
@@ -80,37 +84,31 @@ public class RecoverableZooKeeper {
private final int sessionTimeout;
private final String quorumServers;
private final int maxMultiSize;
+ private final ZKClientConfig zkClientConfig;
/**
- * See {@link #connect(Configuration, String, Watcher, String)}
+ * See {@link #connect(Configuration, String, Watcher, String,
ZKClientConfig)}.
*/
public static RecoverableZooKeeper connect(Configuration conf, Watcher
watcher)
throws IOException {
String ensemble = ZKConfig.getZKQuorumServersString(conf);
- return connect(conf, ensemble, watcher);
- }
-
- /**
- * See {@link #connect(Configuration, String, Watcher, String)}
- */
- public static RecoverableZooKeeper connect(Configuration conf, String
ensemble, Watcher watcher)
- throws IOException {
- return connect(conf, ensemble, watcher, null);
+ return connect(conf, ensemble, watcher, null, null);
}
/**
* Creates a new connection to ZooKeeper, pulling settings and ensemble
config from the specified
* configuration object using methods from {@link ZKConfig}. Sets the
connection status monitoring
* watcher to the specified watcher.
- * @param conf configuration to pull ensemble and other settings from
- * @param watcher watcher to monitor connection changes
- * @param ensemble ZooKeeper servers quorum string
- * @param identifier value used to identify this client instance.
+ * @param conf configuration to pull ensemble and other settings
from
+ * @param watcher watcher to monitor connection changes
+ * @param ensemble ZooKeeper servers quorum string
+ * @param identifier value used to identify this client instance.
+ * @param zkClientConfig client specific configurations for this instance
* @return connection to zookeeper
* @throws IOException if unable to connect to zk or config problem
*/
public static RecoverableZooKeeper connect(Configuration conf, String
ensemble, Watcher watcher,
- final String identifier) throws IOException {
+ final String identifier, ZKClientConfig zkClientConfig) throws IOException
{
if (ensemble == null) {
throw new IOException("Unable to determine ZooKeeper ensemble");
}
@@ -123,14 +121,12 @@ public class RecoverableZooKeeper {
int maxSleepTime = conf.getInt("zookeeper.recovery.retry.maxsleeptime",
60000);
int multiMaxSize = conf.getInt("zookeeper.multi.max.size", 1024 * 1024);
return new RecoverableZooKeeper(ensemble, timeout, watcher, retry,
retryIntervalMillis,
- maxSleepTime, identifier, multiMaxSize);
+ maxSleepTime, identifier, multiMaxSize, zkClientConfig);
}
- @edu.umd.cs.findbugs.annotations.SuppressWarnings(value = "DE_MIGHT_IGNORE",
- justification = "None. Its always been this way.")
- public RecoverableZooKeeper(String quorumServers, int sessionTimeout,
Watcher watcher,
- int maxRetries, int retryIntervalMillis, int maxSleepTime, String
identifier, int maxMultiSize)
- throws IOException {
+ RecoverableZooKeeper(String quorumServers, int sessionTimeout, Watcher
watcher, int maxRetries,
+ int retryIntervalMillis, int maxSleepTime, String identifier, int
maxMultiSize,
+ ZKClientConfig zkClientConfig) throws IOException {
// TODO: Add support for zk 'chroot'; we don't add it to the quorumServers
String as we should.
this.retryCounterFactory =
new RetryCounterFactory(maxRetries + 1, retryIntervalMillis,
maxSleepTime);
@@ -148,12 +144,7 @@ public class RecoverableZooKeeper {
this.sessionTimeout = sessionTimeout;
this.quorumServers = quorumServers;
this.maxMultiSize = maxMultiSize;
-
- try {
- checkZk();
- } catch (Exception x) {
- /* ignore */
- }
+ this.zkClientConfig = zkClientConfig;
}
/**
@@ -172,10 +163,10 @@ public class RecoverableZooKeeper {
* @return The created ZooKeeper connection object
* @throws KeeperException if a ZooKeeper operation fails
*/
- protected synchronized ZooKeeper checkZk() throws KeeperException {
+ private synchronized ZooKeeper checkZk() throws KeeperException {
if (this.zk == null) {
try {
- this.zk = new ZooKeeper(quorumServers, sessionTimeout, watcher);
+ this.zk = new ZooKeeper(quorumServers, sessionTimeout, watcher,
zkClientConfig);
} catch (IOException ex) {
LOG.warn("Unable to create ZooKeeper Connection", ex);
throw new KeeperException.OperationTimeoutException();
diff --git
a/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKWatcher.java
b/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKWatcher.java
index 8f0cfc811b8..3879cb7ba91 100644
---
a/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKWatcher.java
+++
b/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKWatcher.java
@@ -176,8 +176,8 @@ public class ZKWatcher implements Watcher, Abortable,
Closeable {
this.abortable = abortable;
this.znodePaths = new ZNodePaths(conf);
PendingWatcher pendingWatcher = new PendingWatcher();
- this.recoverableZooKeeper =
- RecoverableZooKeeper.connect(conf, quorum, pendingWatcher, identifier);
+ this.recoverableZooKeeper = RecoverableZooKeeper.connect(conf, quorum,
pendingWatcher,
+ identifier, ZKConfig.getZKClientConfig(conf));
pendingWatcher.prepare(this);
if (canCreateBaseZNode) {
try {
diff --git
a/hbase-zookeeper/src/test/java/org/apache/hadoop/hbase/zookeeper/TestRecoverableZooKeeper.java
b/hbase-zookeeper/src/test/java/org/apache/hadoop/hbase/zookeeper/TestRecoverableZooKeeper.java
index 693de0e8819..f313ea7f661 100644
---
a/hbase-zookeeper/src/test/java/org/apache/hadoop/hbase/zookeeper/TestRecoverableZooKeeper.java
+++
b/hbase-zookeeper/src/test/java/org/apache/hadoop/hbase/zookeeper/TestRecoverableZooKeeper.java
@@ -76,7 +76,7 @@ public class TestRecoverableZooKeeper {
Configuration conf = TEST_UTIL.getConfiguration();
ZKWatcher zkw = new ZKWatcher(conf, "testSetDataVersionMismatchInLoop",
abortable, true);
String ensemble = ZKConfig.getZKQuorumServersString(conf);
- RecoverableZooKeeper rzk = RecoverableZooKeeper.connect(conf, ensemble,
zkw);
+ RecoverableZooKeeper rzk = RecoverableZooKeeper.connect(conf, ensemble,
zkw, null, null);
rzk.create(znode, new byte[0], Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
rzk.setData(znode, "OPENING".getBytes(), 0);
Field zkField = RecoverableZooKeeper.class.getDeclaredField("zk");
