[GitHub] [helix] mgao0 commented on a diff in pull request #2290: Code cleanup and FederatedZkClient Multi test.

[GitBox <gi...@apache.org>]

mgao0 commented on code in PR #2290:
URL: https://github.com/apache/helix/pull/2290#discussion_r1028650182


##########
helix-core/src/test/java/org/apache/helix/integration/multizk/TestMultiZkConnectionConfig.java:
##########
@@ -0,0 +1,190 @@
+package org.apache.helix.integration.multizk;
+
+/*
+ * 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.
+ */
+
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.helix.HelixAdmin;
+import org.apache.helix.HelixManagerProperty;
+import org.apache.helix.TestHelper;
+import org.apache.helix.integration.manager.ClusterControllerManager;
+import org.apache.helix.integration.manager.MockParticipantManager;
+import org.apache.helix.integration.task.MockTask;
+import org.apache.helix.manager.zk.ZKHelixAdmin;
+import org.apache.helix.model.InstanceConfig;
+import org.apache.helix.participant.StateMachineEngine;
+import org.apache.helix.task.TaskFactory;
+import org.apache.helix.task.TaskStateModelFactory;
+import org.apache.helix.tools.ClusterSetup;
+import org.apache.helix.zookeeper.api.client.RealmAwareZkClient;
+import org.apache.helix.zookeeper.constant.RoutingDataReaderType;
+import org.apache.helix.zookeeper.datamodel.serializer.ZNRecordSerializer;
+import org.apache.helix.zookeeper.impl.client.FederatedZkClient;
+import org.apache.helix.zookeeper.routing.RoutingDataManager;
+import org.testng.Assert;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+
+/**
+ * Some Helix user do not have multizk routing configs in jvm system config but pass
+ * in through RealAwareZkConnectionConfig.
+ * This test class will not set jvm routing zk config and test Helix functionality.
+ * Tests were similar to TestMultiZkHelixJavaApis but without "MSDS_SERVER_ENDPOINT_KEY"
+ * in system property
+ */
+public class TestMultiZkConnectionConfig extends TestMultiZkHelixJavaApis {
+
+  @BeforeClass
+  public void beforeClass() throws Exception {
+    super.beforeClass();
+    // Routing data may be set by other tests using the same endpoint; reset() for good measure
+    RoutingDataManager.getInstance().reset();
+    // Create a FederatedZkClient for admin work
+
+    try {
+      _zkClient =
+              new FederatedZkClient(new RealmAwareZkClient.RealmAwareZkConnectionConfig.Builder()
+                      .setRoutingDataSourceEndpoint(_msdsEndpoint + "," + ZK_PREFIX + ZK_START_PORT)
+                      .setRoutingDataSourceType(RoutingDataReaderType.HTTP_ZK_FALLBACK.name()).build(),
+                      new RealmAwareZkClient.RealmAwareZkClientConfig());
+      _zkClient.setZkSerializer(new ZNRecordSerializer());
+    } catch (Exception ex) {
+      for (StackTraceElement elm : ex.getStackTrace()) {
+        System.out.println(elm);
+      }
+    }
+    System.out.println("end start");
+  }
+
+  @Override
+  public void setupCluster() {
+    // Create two ClusterSetups using two different constructors
+    // Note: ZK Address here could be anything because multiZk mode is on (it will be ignored)
+    _clusterSetupZkAddr = new ClusterSetup(_zkClient);
+    _clusterSetupBuilder = new ClusterSetup.Builder().setRealmAwareZkConnectionConfig(
+                    new RealmAwareZkClient.RealmAwareZkConnectionConfig.Builder()
+                            .setRoutingDataSourceEndpoint(_msdsEndpoint + "," + ZK_PREFIX + ZK_START_PORT)
+                            .setRoutingDataSourceType(RoutingDataReaderType.HTTP_ZK_FALLBACK.name()).build())
+            .build();
+  }
+
+
+  /**
+   * Test Helix Participant creation and addition.
+   * Helix Java APIs tested in this method are:
+   * ZkHelixAdmin and ZKHelixManager (mock participant/controller)
+   */
+  @Test(dependsOnMethods = "testCreateClusters")
+  public void testCreateParticipants() throws Exception {
+    // Create two ClusterSetups using two different constructors
+    // Note: ZK Address here could be anything because multiZk mode is on (it will be ignored)
+    //HelixAdmin helixAdminZkAddr = new ZKHelixAdmin(ZK_SERVER_MAP.keySet().iterator().next());

Review Comment:
   I'm curious if there a reason to have this comment? Also saw a few cases like this in the rest of the lines.



##########
helix-core/src/test/java/org/apache/helix/integration/multizk/MultiZkTestBase.java:
##########
@@ -0,0 +1,183 @@
+package org.apache.helix.integration.multizk;
+
+/*
+ * 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.
+ */
+
+import com.google.common.collect.ImmutableList;
+import org.apache.helix.HelixAdmin;
+import org.apache.helix.SystemPropertyKeys;
+import org.apache.helix.TestHelper;
+import org.apache.helix.integration.manager.ClusterControllerManager;
+import org.apache.helix.integration.manager.MockParticipantManager;
+import org.apache.helix.msdcommon.constant.MetadataStoreRoutingConstants;
+import org.apache.helix.msdcommon.mock.MockMetadataStoreDirectoryServer;
+import org.apache.helix.participant.StateMachineEngine;
+import org.apache.helix.participant.statemachine.StateModelFactory;
+import org.apache.helix.task.TaskStateModelFactory;
+import org.apache.helix.zookeeper.api.client.HelixZkClient;
+import org.apache.helix.zookeeper.api.client.RealmAwareZkClient;
+import org.apache.helix.zookeeper.datamodel.serializer.ZNRecordSerializer;
+import org.apache.helix.zookeeper.impl.factory.DedicatedZkClientFactory;
+import org.apache.helix.zookeeper.zkclient.ZkServer;
+import org.testng.Assert;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+
+import java.util.*;
+
+public class MultiZkTestBase {
+    protected static final int NUM_ZK = 3;
+    protected static final Map<String, ZkServer> ZK_SERVER_MAP = new HashMap<>();
+    protected static final Map<String, HelixZkClient> ZK_CLIENT_MAP = new HashMap<>();
+    protected static final Map<String, ClusterControllerManager> MOCK_CONTROLLERS = new HashMap<>();
+    protected final Set<MockParticipantManager> MOCK_PARTICIPANTS = new HashSet<>();
+    protected static final List<String> CLUSTER_LIST =
+            ImmutableList.of("CLUSTER_1", "CLUSTER_2", "CLUSTER_3");
+
+    protected MockMetadataStoreDirectoryServer _msds;
+    protected static final Map<String, Collection<String>> _rawRoutingData = new HashMap<>();
+    protected RealmAwareZkClient _zkClient;
+    protected HelixAdmin _zkHelixAdmin;
+
+    // Save System property configs from before this test and pass onto after the test
+    protected final Map<String, String> _configStore = new HashMap<>();
+
+    protected static final String ZK_PREFIX = "localhost:";
+    protected static final int ZK_START_PORT = 8977;
+    protected String _msdsEndpoint;
+
+    @BeforeClass
+    public void beforeClass() throws Exception {
+        // Create 3 in-memory zookeepers and routing mapping
+        for (int i = 0; i < NUM_ZK; i++) {
+            String zkAddress = ZK_PREFIX + (ZK_START_PORT + i);
+            ZK_SERVER_MAP.put(zkAddress, TestHelper.startZkServer(zkAddress));
+            ZK_CLIENT_MAP.put(zkAddress, DedicatedZkClientFactory.getInstance()
+                    .buildZkClient(new HelixZkClient.ZkConnectionConfig(zkAddress),
+                            new HelixZkClient.ZkClientConfig().setZkSerializer(new ZNRecordSerializer())));
+
+            // One cluster per ZkServer created
+            _rawRoutingData.put(zkAddress, Collections.singletonList("/" + CLUSTER_LIST.get(i)));
+        }
+
+        // Create a Mock MSDS
+        final String msdsHostName = "localhost";
+        final int msdsPort = 11117;
+        final String msdsNamespace = "multiZkTest";
+        _msdsEndpoint =
+                "http://" + msdsHostName + ":" + msdsPort + "/admin/v2/namespaces/" + msdsNamespace;
+        _msds = new MockMetadataStoreDirectoryServer(msdsHostName, msdsPort, msdsNamespace,
+                _rawRoutingData);
+        _msds.startServer();
+
+        // Save previously-set system configs
+        String prevMultiZkEnabled = System.getProperty(SystemPropertyKeys.MULTI_ZK_ENABLED);
+        String prevMsdsServerEndpoint =
+                System.getProperty(MetadataStoreRoutingConstants.MSDS_SERVER_ENDPOINT_KEY);
+        if (prevMultiZkEnabled != null) {
+            _configStore.put(SystemPropertyKeys.MULTI_ZK_ENABLED, prevMultiZkEnabled);
+        }
+        if (prevMsdsServerEndpoint != null) {
+            _configStore
+                    .put(MetadataStoreRoutingConstants.MSDS_SERVER_ENDPOINT_KEY, prevMsdsServerEndpoint);
+        }
+
+        // Turn on multiZk mode in System config
+        System.setProperty(SystemPropertyKeys.MULTI_ZK_ENABLED, "true");
+        // MSDS endpoint: http://localhost:11117/admin/v2/namespaces/multiZkTest

Review Comment:
   Is this comment meant to be somewhere else?



-- 
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: reviews-unsubscribe@helix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org