ndimiduk commented on code in PR #5745:
URL: https://github.com/apache/hbase/pull/5745#discussion_r1514273036
##########
hbase-common/src/main/java/org/apache/hadoop/hbase/HBaseConfiguration.java:
##########
@@ -250,12 +250,11 @@ private static void applyClusterKeyToConf(Configuration
conf, String key) throws
conf.set(HConstants.ZOOKEEPER_ZNODE_PARENT, zkClusterKey.getZnodeParent());
// Without the right registry, the above configs are useless. Also, we
don't use setClass()
// here because the ConnectionRegistry* classes are not resolvable from
this module.
- // This will be broken if ZkConnectionRegistry class gets renamed or
moved. Is there a better
- // way?
- LOG.info("Overriding client registry implementation to {}",
- HConstants.ZK_CONNECTION_REGISTRY_CLASS);
- conf.set(HConstants.CLIENT_CONNECTION_REGISTRY_IMPL_CONF_KEY,
- HConstants.ZK_CONNECTION_REGISTRY_CLASS);
+ // This will be broken if ZkConnectionRegistryInternal class gets renamed
or moved. Is there
+ // a better way?
+ String ZKConnectionRegistryClass =
"org.apache.hadoop.hbase.client.ZKConnectionRegistryInternal";
Review Comment:
Have it so that cluster-internal connections make use of the internal
version.
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/client/ZKConnectionRegistry.java:
##########
@@ -17,235 +17,35 @@
*/
package org.apache.hadoop.hbase.client;
-import static org.apache.hadoop.hbase.client.RegionInfo.DEFAULT_REPLICA_ID;
-import static
org.apache.hadoop.hbase.client.RegionInfoBuilder.FIRST_META_REGIONINFO;
-import static
org.apache.hadoop.hbase.client.RegionReplicaUtil.getRegionInfoForDefaultReplica;
-import static
org.apache.hadoop.hbase.client.RegionReplicaUtil.getRegionInfoForReplica;
-import static
org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil.lengthOfPBMagic;
-import static org.apache.hadoop.hbase.trace.TraceUtil.tracedFuture;
-import static org.apache.hadoop.hbase.util.FutureUtils.addListener;
-import static org.apache.hadoop.hbase.zookeeper.ZKMetadata.removeMetaData;
-
-import java.io.IOException;
-import java.util.List;
-import java.util.concurrent.CompletableFuture;
-import java.util.stream.Collectors;
-import org.apache.commons.lang3.mutable.MutableInt;
import org.apache.hadoop.conf.Configuration;
-import org.apache.hadoop.hbase.ClusterId;
-import org.apache.hadoop.hbase.HRegionLocation;
-import org.apache.hadoop.hbase.RegionLocations;
-import org.apache.hadoop.hbase.ServerName;
-import org.apache.hadoop.hbase.exceptions.DeserializationException;
-import org.apache.hadoop.hbase.master.RegionState;
+import org.apache.hadoop.hbase.HBaseInterfaceAudience;
import org.apache.hadoop.hbase.security.User;
-import org.apache.hadoop.hbase.util.Pair;
-import org.apache.hadoop.hbase.zookeeper.ReadOnlyZKClient;
-import org.apache.hadoop.hbase.zookeeper.ZNodePaths;
import org.apache.yetus.audience.InterfaceAudience;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos;
-import org.apache.hadoop.hbase.shaded.protobuf.generated.ZooKeeperProtos;
-
/**
* Zookeeper based registry implementation.
+ * @deprecated As of 2.6.0, replaced by {@link RpcConnectionRegistry}, which
is the default connection mechanism as of 3.0.0. Expected to be removed in
4.0.0.
+ * @see <a
href="https://issues.apache.org/jira/browse/HBASE-23324";>HBASE-23324</a> and
its parent ticket for details.
*/
[email protected]
-class ZKConnectionRegistry implements ConnectionRegistry {
-
+@Deprecated
[email protected](HBaseInterfaceAudience.CONFIG)
+class ZKConnectionRegistry extends ZKConnectionRegistryInternal {
Review Comment:
The diff isn't rendering well, but I renamed the existing
`ZKConnectionRegistry` to `ZKConnectionRegistryInternal` and then created the
new class named `ZKConnectionRegistry` that subclasses and adds the warning.
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/client/ZKConnectionRegistryInternal.java:
##########
@@ -0,0 +1,252 @@
+/*
+ * 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.hadoop.hbase.client;
+
+import static org.apache.hadoop.hbase.client.RegionInfo.DEFAULT_REPLICA_ID;
+import static
org.apache.hadoop.hbase.client.RegionInfoBuilder.FIRST_META_REGIONINFO;
+import static
org.apache.hadoop.hbase.client.RegionReplicaUtil.getRegionInfoForDefaultReplica;
+import static
org.apache.hadoop.hbase.client.RegionReplicaUtil.getRegionInfoForReplica;
+import static
org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil.lengthOfPBMagic;
+import static org.apache.hadoop.hbase.trace.TraceUtil.tracedFuture;
+import static org.apache.hadoop.hbase.util.FutureUtils.addListener;
+import static org.apache.hadoop.hbase.zookeeper.ZKMetadata.removeMetaData;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.concurrent.CompletableFuture;
+import java.util.stream.Collectors;
+import org.apache.commons.lang3.mutable.MutableInt;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.ClusterId;
+import org.apache.hadoop.hbase.HRegionLocation;
+import org.apache.hadoop.hbase.RegionLocations;
+import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.exceptions.DeserializationException;
+import org.apache.hadoop.hbase.master.RegionState;
+import org.apache.hadoop.hbase.security.User;
+import org.apache.hadoop.hbase.util.Pair;
+import org.apache.hadoop.hbase.zookeeper.ReadOnlyZKClient;
+import org.apache.hadoop.hbase.zookeeper.ZNodePaths;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos;
+import org.apache.hadoop.hbase.shaded.protobuf.generated.ZooKeeperProtos;
+
+/**
+ * Zookeeper based registry implementation.
+ */
[email protected]
+class ZKConnectionRegistryInternal implements ConnectionRegistry {
+
+ // intentionally points to the original class name.
+ private static final Logger LOG =
LoggerFactory.getLogger(ZKConnectionRegistry.class);
+
+ private final ReadOnlyZKClient zk;
+
+ private final ZNodePaths znodePaths;
+
+ // User not used, but for rpc based registry we need it
+ ZKConnectionRegistryInternal(Configuration conf, User ignored) {
+ this.znodePaths = new ZNodePaths(conf);
+ this.zk = new ReadOnlyZKClient(conf);
+ }
+
+ private interface Converter<T> {
+ T convert(byte[] data) throws Exception;
+ }
+
+ private <T> CompletableFuture<T> getAndConvert(String path, Converter<T>
converter) {
+ CompletableFuture<T> future = new CompletableFuture<>();
+ addListener(zk.get(path), (data, error) -> {
+ if (error != null) {
+ future.completeExceptionally(error);
+ return;
+ }
+ try {
+ future.complete(converter.convert(data));
+ } catch (Exception e) {
+ future.completeExceptionally(e);
+ }
+ });
+ return future;
+ }
+
+ private static String getClusterId(byte[] data) throws
DeserializationException {
+ if (data == null || data.length == 0) {
+ return null;
+ }
+ data = removeMetaData(data);
+ return ClusterId.parseFrom(data).toString();
+ }
+
+ @Override
+ public CompletableFuture<String> getClusterId() {
+ return tracedFuture(
+ () -> getAndConvert(znodePaths.clusterIdZNode,
ZKConnectionRegistryInternal::getClusterId),
+ "ZKConnectionRegistryInternal.getClusterId");
Review Comment:
I should renamed these spans to use the public name.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
