narendly commented on a change in pull request #1007:
URL: https://github.com/apache/helix/pull/1007#discussion_r425535304



##########
File path: 
helix-core/src/test/java/org/apache/helix/manager/zk/TestZkHelixAdmin.java
##########
@@ -560,6 +560,82 @@ public void testLegacyEnableDisablePartition() {
         2);
   }
 
+  @Test
+  public void testResetPartition() throws Exception {
+    String className = TestHelper.getTestClassName();
+    String methodName = TestHelper.getTestMethodName();
+    String clusterName = className + "_" + methodName;
+    String instanceName = "TestInstance";
+    String testResource = "TestResource";
+    System.out.println("START " + clusterName + " at " + new 
Date(System.currentTimeMillis()));
+    HelixAdmin admin = new ZKHelixAdmin(_gZkClient);
+    admin.addCluster(clusterName, true);
+    admin.addInstance(clusterName, new InstanceConfig(instanceName));
+    admin.enableInstance(clusterName, instanceName, true);
+
+    InstanceConfig instanceConfig = admin.getInstanceConfig(clusterName, 
instanceName);
+
+    // Test the sanity check for resetPartition.
+    // resetPartition is expected to throw an exception when provided with a 
nonexist instance.

Review comment:
       nonexistent

##########
File path: 
helix-core/src/test/java/org/apache/helix/manager/zk/TestZkHelixAdmin.java
##########
@@ -560,6 +560,82 @@ public void testLegacyEnableDisablePartition() {
         2);
   }
 
+  @Test
+  public void testResetPartition() throws Exception {
+    String className = TestHelper.getTestClassName();
+    String methodName = TestHelper.getTestMethodName();
+    String clusterName = className + "_" + methodName;
+    String instanceName = "TestInstance";
+    String testResource = "TestResource";
+    System.out.println("START " + clusterName + " at " + new 
Date(System.currentTimeMillis()));
+    HelixAdmin admin = new ZKHelixAdmin(_gZkClient);
+    admin.addCluster(clusterName, true);
+    admin.addInstance(clusterName, new InstanceConfig(instanceName));
+    admin.enableInstance(clusterName, instanceName, true);
+
+    InstanceConfig instanceConfig = admin.getInstanceConfig(clusterName, 
instanceName);
+
+    // Test the sanity check for resetPartition.
+    // resetPartition is expected to throw an exception when provided with a 
nonexist instance.
+    try {
+      admin.resetPartition(clusterName, "WrongTestInstance", testResource, 
Arrays.asList("1", "2"));
+      Assert.fail("Should throw HelixException");
+    } catch (HelixException expected) {
+      // This exception is expected because the instance name is made up.
+      Assert.assertEquals(expected.getMessage(), String.format(
+          "Can't reset state for %s.[1, 2] on WrongTestInstance, because 
WrongTestInstance"
+              + " has never existed in cluster %s", testResource, 
clusterName));
+    }
+
+    // resetPartition is expected to throw an exception when provided with a 
non-live instance.
+    try {
+      admin.resetPartition(clusterName, instanceName, testResource, 
Arrays.asList("1", "2"));
+      Assert.fail("Should throw HelixException");
+    } catch (HelixException expected) {
+      // This exception is expected because the instance is not alive.
+      Assert.assertEquals(expected.getMessage(), String.format(
+          "Can't reset state for %s.[1, 2] on %s, because %s does not live in 
cluster %s anymore",
+          testResource, instanceName, instanceName, clusterName));
+    }
+
+    HelixManager manager = initializeHelixManager(clusterName, 
instanceConfig.getInstanceName());
+    manager.connect();
+
+    // resetPartition is expected to throw an exception when provided with a 
non-exist resource.
+    try {
+      admin.resetPartition(clusterName, instanceName, testResource, 
Arrays.asList("1", "2"));
+      Assert.fail("Should throw HelixException");
+    } catch (HelixException expected) {
+      // This exception is expected because the resource is not added.
+      Assert.assertEquals(expected.getMessage(), String
+          .format("Can't reset state for %s.[1, 2] on %s, because %s is not 
added", testResource,
+              instanceName, testResource));
+    }
+
+    IdealState idealState = new IdealState(testResource);
+    idealState.setNumPartitions(3);
+    admin.addStateModelDef(clusterName, "MasterSlave", new MasterSlaveSMD());
+    idealState.setStateModelDefRef("MasterSlave");
+    idealState.setRebalanceMode(IdealState.RebalanceMode.FULL_AUTO);
+    admin.addResource(clusterName, testResource, idealState);
+
+    admin.enableResource(clusterName, testResource, true);
+    try {
+      admin.resetPartition(clusterName, instanceName, testResource, 
Arrays.asList("1", "2"));
+      Assert.fail("Should throw HelixException");
+    } catch (HelixException expected) {
+      // This exception is expected because partitions do not exist.
+      Assert.assertEquals(expected.getMessage(), String
+          .format("Can't reset state for %s.[1, 2] on %s, because not all [1, 
2] exist",
+              testResource, instanceName));
+    }
+
+    // clean up
+    manager.disconnect();
+    admin.dropCluster(clusterName);

Review comment:
       Please add a TestHelper.verify() to verify that the cluster has indeed 
been fully deleted at this point.

##########
File path: 
helix-core/src/test/java/org/apache/helix/manager/zk/TestZkHelixAdmin.java
##########
@@ -560,6 +560,82 @@ public void testLegacyEnableDisablePartition() {
         2);
   }
 
+  @Test
+  public void testResetPartition() throws Exception {
+    String className = TestHelper.getTestClassName();
+    String methodName = TestHelper.getTestMethodName();
+    String clusterName = className + "_" + methodName;
+    String instanceName = "TestInstance";
+    String testResource = "TestResource";
+    System.out.println("START " + clusterName + " at " + new 
Date(System.currentTimeMillis()));
+    HelixAdmin admin = new ZKHelixAdmin(_gZkClient);
+    admin.addCluster(clusterName, true);
+    admin.addInstance(clusterName, new InstanceConfig(instanceName));
+    admin.enableInstance(clusterName, instanceName, true);
+
+    InstanceConfig instanceConfig = admin.getInstanceConfig(clusterName, 
instanceName);
+
+    // Test the sanity check for resetPartition.
+    // resetPartition is expected to throw an exception when provided with a 
nonexist instance.
+    try {
+      admin.resetPartition(clusterName, "WrongTestInstance", testResource, 
Arrays.asList("1", "2"));

Review comment:
       Make "WrongTestInstance" a string constant throughout?

##########
File path: 
helix-core/src/test/java/org/apache/helix/manager/zk/TestZkHelixAdmin.java
##########
@@ -560,6 +560,82 @@ public void testLegacyEnableDisablePartition() {
         2);
   }
 
+  @Test
+  public void testResetPartition() throws Exception {
+    String className = TestHelper.getTestClassName();
+    String methodName = TestHelper.getTestMethodName();
+    String clusterName = className + "_" + methodName;
+    String instanceName = "TestInstance";
+    String testResource = "TestResource";
+    System.out.println("START " + clusterName + " at " + new 
Date(System.currentTimeMillis()));
+    HelixAdmin admin = new ZKHelixAdmin(_gZkClient);
+    admin.addCluster(clusterName, true);
+    admin.addInstance(clusterName, new InstanceConfig(instanceName));
+    admin.enableInstance(clusterName, instanceName, true);
+
+    InstanceConfig instanceConfig = admin.getInstanceConfig(clusterName, 
instanceName);
+
+    // Test the sanity check for resetPartition.
+    // resetPartition is expected to throw an exception when provided with a 
nonexist instance.
+    try {
+      admin.resetPartition(clusterName, "WrongTestInstance", testResource, 
Arrays.asList("1", "2"));
+      Assert.fail("Should throw HelixException");
+    } catch (HelixException expected) {
+      // This exception is expected because the instance name is made up.
+      Assert.assertEquals(expected.getMessage(), String.format(
+          "Can't reset state for %s.[1, 2] on WrongTestInstance, because 
WrongTestInstance"
+              + " has never existed in cluster %s", testResource, 
clusterName));
+    }
+
+    // resetPartition is expected to throw an exception when provided with a 
non-live instance.
+    try {
+      admin.resetPartition(clusterName, instanceName, testResource, 
Arrays.asList("1", "2"));
+      Assert.fail("Should throw HelixException");
+    } catch (HelixException expected) {
+      // This exception is expected because the instance is not alive.
+      Assert.assertEquals(expected.getMessage(), String.format(
+          "Can't reset state for %s.[1, 2] on %s, because %s does not live in 
cluster %s anymore",
+          testResource, instanceName, instanceName, clusterName));
+    }
+
+    HelixManager manager = initializeHelixManager(clusterName, 
instanceConfig.getInstanceName());
+    manager.connect();
+
+    // resetPartition is expected to throw an exception when provided with a 
non-exist resource.
+    try {
+      admin.resetPartition(clusterName, instanceName, testResource, 
Arrays.asList("1", "2"));
+      Assert.fail("Should throw HelixException");
+    } catch (HelixException expected) {
+      // This exception is expected because the resource is not added.
+      Assert.assertEquals(expected.getMessage(), String
+          .format("Can't reset state for %s.[1, 2] on %s, because %s is not 
added", testResource,
+              instanceName, testResource));
+    }
+
+    IdealState idealState = new IdealState(testResource);
+    idealState.setNumPartitions(3);
+    admin.addStateModelDef(clusterName, "MasterSlave", new MasterSlaveSMD());
+    idealState.setStateModelDefRef("MasterSlave");
+    idealState.setRebalanceMode(IdealState.RebalanceMode.FULL_AUTO);
+    admin.addResource(clusterName, testResource, idealState);
+
+    admin.enableResource(clusterName, testResource, true);

Review comment:
       Do you need to add a verifier check here? It might take some time for 
the controller to rebalance.

##########
File path: 
helix-core/src/test/java/org/apache/helix/manager/zk/TestZkHelixAdmin.java
##########
@@ -560,6 +560,82 @@ public void testLegacyEnableDisablePartition() {
         2);
   }
 
+  @Test
+  public void testResetPartition() throws Exception {
+    String className = TestHelper.getTestClassName();
+    String methodName = TestHelper.getTestMethodName();
+    String clusterName = className + "_" + methodName;
+    String instanceName = "TestInstance";
+    String testResource = "TestResource";
+    System.out.println("START " + clusterName + " at " + new 
Date(System.currentTimeMillis()));
+    HelixAdmin admin = new ZKHelixAdmin(_gZkClient);
+    admin.addCluster(clusterName, true);
+    admin.addInstance(clusterName, new InstanceConfig(instanceName));
+    admin.enableInstance(clusterName, instanceName, true);
+
+    InstanceConfig instanceConfig = admin.getInstanceConfig(clusterName, 
instanceName);
+
+    // Test the sanity check for resetPartition.
+    // resetPartition is expected to throw an exception when provided with a 
nonexist instance.
+    try {
+      admin.resetPartition(clusterName, "WrongTestInstance", testResource, 
Arrays.asList("1", "2"));
+      Assert.fail("Should throw HelixException");
+    } catch (HelixException expected) {
+      // This exception is expected because the instance name is made up.
+      Assert.assertEquals(expected.getMessage(), String.format(
+          "Can't reset state for %s.[1, 2] on WrongTestInstance, because 
WrongTestInstance"
+              + " has never existed in cluster %s", testResource, 
clusterName));
+    }
+
+    // resetPartition is expected to throw an exception when provided with a 
non-live instance.
+    try {
+      admin.resetPartition(clusterName, instanceName, testResource, 
Arrays.asList("1", "2"));
+      Assert.fail("Should throw HelixException");
+    } catch (HelixException expected) {
+      // This exception is expected because the instance is not alive.
+      Assert.assertEquals(expected.getMessage(), String.format(
+          "Can't reset state for %s.[1, 2] on %s, because %s does not live in 
cluster %s anymore",
+          testResource, instanceName, instanceName, clusterName));
+    }
+
+    HelixManager manager = initializeHelixManager(clusterName, 
instanceConfig.getInstanceName());
+    manager.connect();
+
+    // resetPartition is expected to throw an exception when provided with a 
non-exist resource.

Review comment:
       nonexistent

##########
File path: 
helix-core/src/test/java/org/apache/helix/manager/zk/TestZkHelixAdmin.java
##########
@@ -560,6 +560,82 @@ public void testLegacyEnableDisablePartition() {
         2);
   }
 
+  @Test
+  public void testResetPartition() throws Exception {
+    String className = TestHelper.getTestClassName();
+    String methodName = TestHelper.getTestMethodName();
+    String clusterName = className + "_" + methodName;
+    String instanceName = "TestInstance";
+    String testResource = "TestResource";
+    System.out.println("START " + clusterName + " at " + new 
Date(System.currentTimeMillis()));
+    HelixAdmin admin = new ZKHelixAdmin(_gZkClient);
+    admin.addCluster(clusterName, true);
+    admin.addInstance(clusterName, new InstanceConfig(instanceName));
+    admin.enableInstance(clusterName, instanceName, true);

Review comment:
       Do we need a verifier check after this?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to