[helix] 49/49: Fix getClusters() in ZKHelixAdmin for multi-zk mode (#916)

[hu...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

hulee pushed a commit to branch zooscalability
in repository https://gitbox.apache.org/repos/asf/helix.git

commit be20362fd3bb2a1259a89f1c993f9220ed0dbabd
Author: Hunter Lee <hu...@linkedin.com>
AuthorDate: Mon Mar 30 10:31:14 2020 -0700

    Fix getClusters() in ZKHelixAdmin for multi-zk mode (#916)
    
    This PR fixes the logic in getClusters() so that it works in a multi-zk environment. On multi-zk mode, the API will query for raw routing data from MSDS and produce a list of all clusters in the namespace. The behavior for single-zk mode remains the same for backward-compatibility.
---
 .../java/org/apache/helix/SystemPropertyKeys.java  |  7 +++++
 .../org/apache/helix/manager/zk/ZKHelixAdmin.java  | 36 +++++++++++++++++-----
 .../multizk/TestMultiZkHelixJavaApis.java          |  8 +++++
 .../helix/msdcommon/util/ZkValidationUtil.java     |  3 +-
 .../zookeeper/api/client/RealmAwareZkClient.java   |  8 +++++
 .../zookeeper/impl/client/DedicatedZkClient.java   | 14 +++++++++
 .../zookeeper/impl/client/FederatedZkClient.java   | 12 ++++++++
 .../zookeeper/impl/client/SharedZkClient.java      | 14 +++++++++
 8 files changed, 94 insertions(+), 8 deletions(-)

diff --git a/helix-common/src/main/java/org/apache/helix/SystemPropertyKeys.java b/helix-common/src/main/java/org/apache/helix/SystemPropertyKeys.java
index a40dbe9..9870cdd 100644
--- a/helix-common/src/main/java/org/apache/helix/SystemPropertyKeys.java
+++ b/helix-common/src/main/java/org/apache/helix/SystemPropertyKeys.java
@@ -19,6 +19,9 @@ package org.apache.helix;
  * under the License.
  */
 
+import org.apache.helix.msdcommon.constant.MetadataStoreRoutingConstants;
+
+
 public class SystemPropertyKeys {
   // Task Driver
   public static final String TASK_CONFIG_LIMITATION = "helixTask.configsLimitation";
@@ -63,4 +66,8 @@ public class SystemPropertyKeys {
 
   // Multi-ZK mode enable/disable flag
   public static final String MULTI_ZK_ENABLED = "helix.multiZkEnabled";
+
+  // System Property Metadata Store Directory Server endpoint key
+  public static final String MSDS_SERVER_ENDPOINT_KEY =
+      MetadataStoreRoutingConstants.MSDS_SERVER_ENDPOINT_KEY;
 }
diff --git a/helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java b/helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java
index 65e6ec0..fad5005 100644
--- a/helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java
+++ b/helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java
@@ -36,6 +36,7 @@ import java.util.Set;
 import java.util.TreeMap;
 import java.util.UUID;
 import java.util.concurrent.TimeUnit;
+import java.util.stream.Collectors;
 
 import org.apache.helix.AccessOption;
 import org.apache.helix.BaseDataAccessor;
@@ -83,6 +84,7 @@ import org.apache.helix.zookeeper.api.client.RealmAwareZkClient;
 import org.apache.helix.zookeeper.datamodel.ZNRecord;
 import org.apache.helix.zookeeper.impl.client.FederatedZkClient;
 import org.apache.helix.zookeeper.impl.factory.SharedZkClientFactory;
+import org.apache.helix.zookeeper.util.HttpRoutingDataReader;
 import org.apache.helix.zookeeper.zkclient.DataUpdater;
 import org.apache.helix.zookeeper.zkclient.exception.ZkException;
 import org.apache.helix.zookeeper.zkclient.exception.ZkNoNodeException;
@@ -933,17 +935,37 @@ public class ZKHelixAdmin implements HelixAdmin {
 
   @Override
   public List<String> getClusters() {
+    List<String> zkToplevelPaths;
+
     if (Boolean.getBoolean(SystemPropertyKeys.MULTI_ZK_ENABLED)
         || _zkClient instanceof FederatedZkClient) {
-      String errMsg =
-          "getClusters() is not supported in multi-realm mode! Use Metadata Store Directory Service instead!";
-      LOG.error(errMsg);
-      throw new UnsupportedOperationException(errMsg);
+      // If on multi-zk mode, we retrieve cluster information from Metadata Store Directory Service.
+      Map<String, List<String>> realmToShardingKeys;
+      String msdsEndpoint = _zkClient.getRealmAwareZkConnectionConfig().getMsdsEndpoint();
+      try {
+        if (msdsEndpoint == null || msdsEndpoint.isEmpty()) {
+          realmToShardingKeys = HttpRoutingDataReader.getRawRoutingData();
+        } else {
+          realmToShardingKeys = HttpRoutingDataReader.getRawRoutingData(msdsEndpoint);
+        }
+      } catch (IOException e) {
+        throw new HelixException(
+            "ZKHelixAdmin: Failed to read raw routing data from Metadata Store Directory Service! MSDS endpoint used: "
+                + msdsEndpoint, e);
+      }
+      if (realmToShardingKeys == null || realmToShardingKeys.isEmpty()) {
+        return Collections.emptyList();
+      }
+      // Preceding "/"s are removed: e.g.) "/CLUSTER-SHARDING-KEY" -> "CLUSTER-SHARDING-KEY"
+      zkToplevelPaths = realmToShardingKeys.values().stream().flatMap(List::stream)
+          .map(shardingKey -> shardingKey.substring(1)).collect(Collectors.toList());
+    } else {
+      // single-zk mode
+      zkToplevelPaths = _zkClient.getChildren("/");
     }
 
-    List<String> zkToplevelPathes = _zkClient.getChildren("/");
-    List<String> result = new ArrayList<String>();
-    for (String pathName : zkToplevelPathes) {
+    List<String> result = new ArrayList<>();
+    for (String pathName : zkToplevelPaths) {
       if (ZKUtil.isClusterSetup(pathName, _zkClient)) {
         result.add(pathName);
       }
diff --git a/helix-core/src/test/java/org/apache/helix/integration/multizk/TestMultiZkHelixJavaApis.java b/helix-core/src/test/java/org/apache/helix/integration/multizk/TestMultiZkHelixJavaApis.java
index be8bb0b..d2a90a1 100644
--- a/helix-core/src/test/java/org/apache/helix/integration/multizk/TestMultiZkHelixJavaApis.java
+++ b/helix-core/src/test/java/org/apache/helix/integration/multizk/TestMultiZkHelixJavaApis.java
@@ -473,4 +473,12 @@ public class TestMultiZkHelixJavaApis {
       Assert.assertEquals(context.getWorkflowState(), wfStateFromTaskDriver);
     }
   }
+
+  /**
+   * This method tests that ZKHelixAdmin::getClusters() works in multi-zk environment.
+   */
+  @Test(dependsOnMethods = "testTaskFramework")
+  public void testGetAllClusters() {
+    Assert.assertEquals(new HashSet<>(_zkHelixAdmin.getClusters()), new HashSet<>(CLUSTER_LIST));
+  }
 }
diff --git a/metadata-store-directory-common/src/main/java/org/apache/helix/msdcommon/util/ZkValidationUtil.java b/metadata-store-directory-common/src/main/java/org/apache/helix/msdcommon/util/ZkValidationUtil.java
index ab8258d..0b23431 100644
--- a/metadata-store-directory-common/src/main/java/org/apache/helix/msdcommon/util/ZkValidationUtil.java
+++ b/metadata-store-directory-common/src/main/java/org/apache/helix/msdcommon/util/ZkValidationUtil.java
@@ -28,12 +28,13 @@ public class ZkValidationUtil {
    * /abc
    * /abc/abc/abc/abc
    * /abc/localhost:1234
+   * /abc/def.hil
    * Invalid matches:
    * null or empty string
    * /abc/
    * /abc/abc/abc/abc/
    **/
   public static boolean isPathValid(String path) {
-    return path.matches("^/|(/[\\w?:-]+)+$");
+    return path.matches("^/|(/[\\w?[$&+,:;=?@#|'<>.^*()%!-]-]+)+$");
   }
 }
diff --git a/zookeeper-api/src/main/java/org/apache/helix/zookeeper/api/client/RealmAwareZkClient.java b/zookeeper-api/src/main/java/org/apache/helix/zookeeper/api/client/RealmAwareZkClient.java
index ee8c8e3..22f3678 100644
--- a/zookeeper-api/src/main/java/org/apache/helix/zookeeper/api/client/RealmAwareZkClient.java
+++ b/zookeeper-api/src/main/java/org/apache/helix/zookeeper/api/client/RealmAwareZkClient.java
@@ -266,6 +266,14 @@ public interface RealmAwareZkClient {
 
   PathBasedZkSerializer getZkSerializer();
 
+  default RealmAwareZkConnectionConfig getRealmAwareZkConnectionConfig() {
+    throw new UnsupportedOperationException("getRealmAwareZkClientConfig() is not supported!");
+  }
+
+  default RealmAwareZkClientConfig getRealmAwareZkClientConfig() {
+    throw new UnsupportedOperationException("getRealmAwareZkClientConfig() is not supported!");
+  }
+
   /**
    * A class that wraps a default implementation of
    * {@link IZkStateListener}, which means this listener
diff --git a/zookeeper-api/src/main/java/org/apache/helix/zookeeper/impl/client/DedicatedZkClient.java b/zookeeper-api/src/main/java/org/apache/helix/zookeeper/impl/client/DedicatedZkClient.java
index beeb085..09b66fb 100644
--- a/zookeeper-api/src/main/java/org/apache/helix/zookeeper/impl/client/DedicatedZkClient.java
+++ b/zookeeper-api/src/main/java/org/apache/helix/zookeeper/impl/client/DedicatedZkClient.java
@@ -59,6 +59,8 @@ public class DedicatedZkClient implements RealmAwareZkClient {
   private final ZkClient _rawZkClient;
   private final MetadataStoreRoutingData _metadataStoreRoutingData;
   private final String _zkRealmShardingKey;
+  private final RealmAwareZkClient.RealmAwareZkConnectionConfig _connectionConfig;
+  private final RealmAwareZkClient.RealmAwareZkClientConfig _clientConfig;
 
   /**
    * DedicatedZkClient connects to a single ZK realm and supports full ZkClient functionalities
@@ -77,6 +79,8 @@ public class DedicatedZkClient implements RealmAwareZkClient {
     if (clientConfig == null) {
       throw new IllegalArgumentException("RealmAwareZkClientConfig cannot be null!");
     }
+    _connectionConfig = connectionConfig;
+    _clientConfig = clientConfig;
 
     // Get the routing data from a static Singleton HttpRoutingDataReader
     String msdsEndpoint = connectionConfig.getMsdsEndpoint();
@@ -468,6 +472,16 @@ public class DedicatedZkClient implements RealmAwareZkClient {
     return _rawZkClient.getZkSerializer();
   }
 
+  @Override
+  public RealmAwareZkConnectionConfig getRealmAwareZkConnectionConfig() {
+    return _connectionConfig;
+  }
+
+  @Override
+  public RealmAwareZkClientConfig getRealmAwareZkClientConfig() {
+    return _clientConfig;
+  }
+
   /**
    * Checks whether the given path belongs matches the ZK path sharding key this DedicatedZkClient is designated to at initialization.
    * @param path
diff --git a/zookeeper-api/src/main/java/org/apache/helix/zookeeper/impl/client/FederatedZkClient.java b/zookeeper-api/src/main/java/org/apache/helix/zookeeper/impl/client/FederatedZkClient.java
index 1bfff66..1cebc90 100644
--- a/zookeeper-api/src/main/java/org/apache/helix/zookeeper/impl/client/FederatedZkClient.java
+++ b/zookeeper-api/src/main/java/org/apache/helix/zookeeper/impl/client/FederatedZkClient.java
@@ -74,6 +74,7 @@ public class FederatedZkClient implements RealmAwareZkClient {
       DedicatedZkClientFactory.class.getSimpleName();
 
   private final MetadataStoreRoutingData _metadataStoreRoutingData;
+  private final RealmAwareZkClient.RealmAwareZkConnectionConfig _connectionConfig;
   private final RealmAwareZkClient.RealmAwareZkClientConfig _clientConfig;
 
   // ZK realm -> ZkClient
@@ -102,6 +103,7 @@ public class FederatedZkClient implements RealmAwareZkClient {
     }
 
     _isClosed = false;
+    _connectionConfig = connectionConfig;
     _clientConfig = clientConfig;
     _pathBasedZkSerializer = clientConfig.getZkSerializer();
     _zkRealmToZkClientMap = new ConcurrentHashMap<>();
@@ -477,6 +479,16 @@ public class FederatedZkClient implements RealmAwareZkClient {
     return _pathBasedZkSerializer;
   }
 
+  @Override
+  public RealmAwareZkConnectionConfig getRealmAwareZkConnectionConfig() {
+    return _connectionConfig;
+  }
+
+  @Override
+  public RealmAwareZkClientConfig getRealmAwareZkClientConfig() {
+    return _clientConfig;
+  }
+
   private String create(final String path, final Object dataObject, final List<ACL> acl,
       final CreateMode mode, final String expectedSessionId) {
     if (mode.isEphemeral()) {
diff --git a/zookeeper-api/src/main/java/org/apache/helix/zookeeper/impl/client/SharedZkClient.java b/zookeeper-api/src/main/java/org/apache/helix/zookeeper/impl/client/SharedZkClient.java
index dd78aba..0989136 100644
--- a/zookeeper-api/src/main/java/org/apache/helix/zookeeper/impl/client/SharedZkClient.java
+++ b/zookeeper-api/src/main/java/org/apache/helix/zookeeper/impl/client/SharedZkClient.java
@@ -59,6 +59,8 @@ public class SharedZkClient implements RealmAwareZkClient {
   private final MetadataStoreRoutingData _metadataStoreRoutingData;
   private final String _zkRealmShardingKey;
   private final String _zkRealmAddress;
+  private final RealmAwareZkClient.RealmAwareZkConnectionConfig _connectionConfig;
+  private final RealmAwareZkClient.RealmAwareZkClientConfig _clientConfig;
 
   public SharedZkClient(RealmAwareZkClient.RealmAwareZkConnectionConfig connectionConfig,
       RealmAwareZkClient.RealmAwareZkClientConfig clientConfig)
@@ -69,6 +71,8 @@ public class SharedZkClient implements RealmAwareZkClient {
     if (clientConfig == null) {
       throw new IllegalArgumentException("RealmAwareZkClientConfig cannot be null!");
     }
+    _connectionConfig = connectionConfig;
+    _clientConfig = clientConfig;
 
     // Get the routing data from a static Singleton HttpRoutingDataReader
     String msdsEndpoint = connectionConfig.getMsdsEndpoint();
@@ -499,6 +503,16 @@ public class SharedZkClient implements RealmAwareZkClient {
   }
 
   @Override
+  public RealmAwareZkConnectionConfig getRealmAwareZkConnectionConfig() {
+    return _connectionConfig;
+  }
+
+  @Override
+  public RealmAwareZkClientConfig getRealmAwareZkClientConfig() {
+    return _clientConfig;
+  }
+
+  @Override
   public PathBasedZkSerializer getZkSerializer() {
     return _innerSharedZkClient.getZkSerializer();
   }