You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@helix.apache.org by hu...@apache.org on 2020/06/03 05:37:27 UTC
[helix] branch master updated: Fix test testDropInstance (#1053)
This is an automated email from the ASF dual-hosted git repository.
hulee pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/helix.git
The following commit(s) were added to refs/heads/master by this push:
new 02dfe1a Fix test testDropInstance (#1053)
02dfe1a is described below
commit 02dfe1a29bc8a8f713575c92b2b522fb851e0f41
Author: Huizhi Lu <ih...@gmail.com>
AuthorDate: Tue Jun 2 22:37:21 2020 -0700
Fix test testDropInstance (#1053)
Test testDropInstance doesn't assert expected HelixException, so when a live instance is not created, the test still passes. The reason is the test uses a sharedZkClient to create a live instance but fails.
This PR addresses this issue by creating a dedicated zkclient in ZkBaseDataAccessor to create a live instance.
---
.../manager/zk/GenericBaseDataAccessorBuilder.java | 4 +-
.../org/apache/helix/tools/TestClusterSetup.java | 105 ++++++++++++---------
2 files changed, 62 insertions(+), 47 deletions(-)
diff --git a/helix-core/src/main/java/org/apache/helix/manager/zk/GenericBaseDataAccessorBuilder.java b/helix-core/src/main/java/org/apache/helix/manager/zk/GenericBaseDataAccessorBuilder.java
index d19ebb1..92a4df4 100644
--- a/helix-core/src/main/java/org/apache/helix/manager/zk/GenericBaseDataAccessorBuilder.java
+++ b/helix-core/src/main/java/org/apache/helix/manager/zk/GenericBaseDataAccessorBuilder.java
@@ -99,8 +99,6 @@ public class GenericBaseDataAccessorBuilder<B extends GenericBaseDataAccessorBui
break;
case SINGLE_REALM:
- // Create a HelixZkClient: Use a SharedZkClient because ZkBaseDataAccessor does not need to
- // do ephemeral operations.
if (_zkClientType == ZkClientType.DEDICATED) {
// If DEDICATED, then we use a dedicated HelixZkClient because we must support ephemeral
// operations
@@ -108,6 +106,8 @@ public class GenericBaseDataAccessorBuilder<B extends GenericBaseDataAccessorBui
.buildZkClient(new HelixZkClient.ZkConnectionConfig(zkAddress),
clientConfig.createHelixZkClientConfig());
} else {
+ // if SHARED: Use a SharedZkClient because ZkBaseDataAccessor does not need to
+ // do ephemeral operations.
zkClient = SharedZkClientFactory.getInstance()
.buildZkClient(new HelixZkClient.ZkConnectionConfig(zkAddress),
clientConfig.createHelixZkClientConfig());
diff --git a/helix-core/src/test/java/org/apache/helix/tools/TestClusterSetup.java b/helix-core/src/test/java/org/apache/helix/tools/TestClusterSetup.java
index ee5ae07..249c41e 100644
--- a/helix-core/src/test/java/org/apache/helix/tools/TestClusterSetup.java
+++ b/helix-core/src/test/java/org/apache/helix/tools/TestClusterSetup.java
@@ -32,6 +32,7 @@ import org.apache.helix.PropertyKey;
import org.apache.helix.PropertyKey.Builder;
import org.apache.helix.PropertyPathBuilder;
import org.apache.helix.TestHelper;
+import org.apache.helix.zookeeper.api.client.RealmAwareZkClient;
import org.apache.helix.zookeeper.datamodel.ZNRecord;
import org.apache.helix.ZkUnitTestBase;
import org.apache.helix.cloud.azure.AzureConstants;
@@ -52,6 +53,7 @@ import org.testng.annotations.BeforeClass;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
+
public class TestClusterSetup extends ZkUnitTestBase {
protected static final String CLUSTER_NAME = "TestClusterSetup";
protected static final String TEST_DB = "TestDB";
@@ -343,6 +345,8 @@ public class TestClusterSetup extends ZkUnitTestBase {
String className = TestHelper.getTestClassName();
String methodName = TestHelper.getTestMethodName();
String clusterName = className + "_" + methodName;
+ String instanceAddress = "localhost:12918";
+ String instanceName = "localhost_12918";
System.out.println("START " + clusterName + " at " + new Date(System.currentTimeMillis()));
@@ -356,51 +360,65 @@ public class TestClusterSetup extends ZkUnitTestBase {
"MasterSlave", true); // do rebalance
// add fake liveInstance
- ZKHelixDataAccessor accessor =
- new ZKHelixDataAccessor(clusterName, new ZkBaseDataAccessor<ZNRecord>(ZK_ADDR));
- Builder keyBuilder = new Builder(clusterName);
- LiveInstance liveInstance = new LiveInstance("localhost_12918");
- liveInstance.setSessionId("session_0");
- liveInstance.setHelixVersion("version_0");
- accessor.setProperty(keyBuilder.liveInstance("localhost_12918"), liveInstance);
+ HelixDataAccessor accessor = new ZKHelixDataAccessor(clusterName,
+ new ZkBaseDataAccessor.Builder<ZNRecord>()
+ .setRealmMode(RealmAwareZkClient.RealmMode.SINGLE_REALM)
+ .setZkClientType(ZkBaseDataAccessor.ZkClientType.DEDICATED)
+ .setZkAddress(ZK_ADDR)
+ .build());
- // drop without stop the process, should throw exception
try {
- ClusterSetup.processCommandLineArgs(new String[] {
- "--zkSvr", ZK_ADDR, "--dropNode", clusterName, "localhost:12918"
- });
- Assert.fail("Should throw exception since localhost_12918 is still in LIVEINSTANCES/");
- } catch (Exception e) {
- // OK
+ Builder keyBuilder = new Builder(clusterName);
+ LiveInstance liveInstance = new LiveInstance(instanceName);
+ liveInstance.setSessionId("session_0");
+ liveInstance.setHelixVersion("version_0");
+ Assert.assertTrue(accessor.setProperty(keyBuilder.liveInstance(instanceName), liveInstance));
+
+ // Drop instance without stopping the live instance, should throw HelixException
+ try {
+ ClusterSetup.processCommandLineArgs(
+ new String[]{"--zkSvr", ZK_ADDR, "--dropNode", clusterName, instanceAddress});
+ Assert.fail("Should throw exception since localhost_12918 is still in LIVEINSTANCES/");
+ } catch (HelixException expected) {
+ Assert.assertEquals(expected.getMessage(),
+ "Cannot drop instance " + instanceName + " as it is still live. Please stop it first");
+ }
+ accessor.removeProperty(keyBuilder.liveInstance(instanceName));
+
+ // drop without disable, should throw exception
+ try {
+ ClusterSetup.processCommandLineArgs(
+ new String[]{"--zkSvr", ZK_ADDR, "--dropNode", clusterName, instanceAddress});
+ Assert.fail("Should throw exception since " + instanceName + " is enabled");
+ } catch (HelixException expected) {
+ Assert.assertEquals(expected.getMessage(),
+ "Node " + instanceName + " is enabled, cannot drop");
+ }
+
+ // Disable the instance
+ ClusterSetup.processCommandLineArgs(
+ new String[]{"--zkSvr", ZK_ADDR, "--enableInstance", clusterName, instanceName, "false"});
+ // Drop the instance
+ ClusterSetup.processCommandLineArgs(
+ new String[]{"--zkSvr", ZK_ADDR, "--dropNode", clusterName, instanceAddress});
+
+ Assert.assertNull(accessor.getProperty(keyBuilder.instanceConfig(instanceName)),
+ "Instance config should be dropped");
+ Assert.assertFalse(_gZkClient.exists(PropertyPathBuilder.instance(clusterName, instanceName)),
+ "Instance/host should be dropped");
+ } finally {
+ // Have to close the dedicated zkclient in accessor to avoid zkclient leakage.
+ accessor.getBaseDataAccessor().close();
+ TestHelper.dropCluster(clusterName, _gZkClient);
+
+ // Verify the cluster has been dropped.
+ Assert.assertTrue(TestHelper.verify(() -> {
+ if (_gZkClient.exists("/" + clusterName)) {
+ TestHelper.dropCluster(clusterName, _gZkClient);
+ }
+ return true;
+ }, TestHelper.WAIT_DURATION));
}
- accessor.removeProperty(keyBuilder.liveInstance("localhost_12918"));
-
- // drop without disable, should throw exception
- try {
- ClusterSetup.processCommandLineArgs(new String[] {
- "--zkSvr", ZK_ADDR, "--dropNode", clusterName, "localhost:12918"
- });
- Assert.fail("Should throw exception since localhost_12918 is enabled");
- } catch (Exception e) {
- // e.printStackTrace();
- // OK
- }
-
- // drop it
- ClusterSetup.processCommandLineArgs(new String[] {
- "--zkSvr", ZK_ADDR, "--enableInstance", clusterName, "localhost_12918", "false"
- });
- ClusterSetup.processCommandLineArgs(new String[] {
- "--zkSvr", ZK_ADDR, "--dropNode", clusterName, "localhost:12918"
- });
-
- Assert.assertNull(accessor.getProperty(keyBuilder.instanceConfig("localhost_12918")),
- "Instance config should be dropped");
- Assert.assertFalse(
- _gZkClient.exists(PropertyPathBuilder.instance(clusterName, "localhost_12918")),
- "Instance/host should be dropped");
-
- TestHelper.dropCluster(clusterName, _gZkClient);
System.out.println("END " + clusterName + " at " + new Date(System.currentTimeMillis()));
}
@@ -439,7 +457,6 @@ public class TestClusterSetup extends ZkUnitTestBase {
System.out.println("END " + clusterName + " at " + new Date(System.currentTimeMillis()));
}
-
@Test(expectedExceptions = HelixException.class)
public void testAddClusterWithInvalidCloudConfig() throws Exception {
String className = TestHelper.getTestClassName();
@@ -455,7 +472,6 @@ public class TestClusterSetup extends ZkUnitTestBase {
CloudConfig cloudConfigInit = cloudConfigInitBuilder.build();
-
// Since setCloudInfoProcessorName is missing, this add cluster call will throw an exception
_clusterSetup.addCluster(clusterName, false, cloudConfigInit);
}
@@ -490,7 +506,6 @@ public class TestClusterSetup extends ZkUnitTestBase {
Assert.assertEquals(cloudConfigFromZk.getCloudProvider(), CloudProvider.CUSTOMIZED.name());
}
-
@Test(dependsOnMethods = "testAddClusterWithValidCloudConfig")
public void testAddClusterAzureProvider() throws Exception {
String className = TestHelper.getTestClassName();