[helix] 06/15: Fix check for disabled partitions

[hu...@apache.org]

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

commit 8abfe67fa5612cae39f591c2c0bd7a41b48b91e7
Author: Junkai Xue <jx...@linkedin.com>
AuthorDate: Wed May 29 18:19:43 2019 -0700

    Fix check for disabled partitions
    
    For the map field of disabled partitions, even they are all enabled, there could be some key left over for resources. We cannot just check if there is any resource entries. With this fix, Helix loops all the resource entries of disabled map to see whether there is a parition list is not empty.
    
    In addition, fix failed tests in REST.
    
    RB=1683071
    BUG=HELIX-1910
    G=helix-reviewers
    A=hulee
    
    Signed-off-by: Hunter Lee <hu...@linkedin.com>
---
 .../org/apache/helix/util/InstanceValidationUtil.java     | 11 ++++++++++-
 .../org/apache/helix/util/TestInstanceValidationUtil.java | 15 ++++++++++++++-
 .../apache/helix/rest/server/TestInstancesAccessor.java   |  9 ++++++---
 3 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/helix-core/src/main/java/org/apache/helix/util/InstanceValidationUtil.java b/helix-core/src/main/java/org/apache/helix/util/InstanceValidationUtil.java
index 7239eeb..b373469 100644
--- a/helix-core/src/main/java/org/apache/helix/util/InstanceValidationUtil.java
+++ b/helix-core/src/main/java/org/apache/helix/util/InstanceValidationUtil.java
@@ -130,7 +130,16 @@ public class InstanceValidationUtil {
     PropertyKey propertyKey = dataAccessor.keyBuilder().instanceConfig(instanceName);
     InstanceConfig instanceConfig = dataAccessor.getProperty(propertyKey);
     if (instanceConfig != null) {
-      return !instanceConfig.getDisabledPartitionsMap().isEmpty();
+      // Originally, Helix only checks whether disabled partition map has entries or not. But this
+      // could cause problem when some of partitions disabled and enabled back, the resource entries
+      // are still left there. For detailed check, we shall check the whether partition list is empty
+      // or not
+      for (List<String> disabledPartitions : instanceConfig.getDisabledPartitionsMap().values()) {
+        if (disabledPartitions != null && disabledPartitions.size() > 0) {
+          return true;
+        }
+      }
+      return false;
     }
 
     throw new HelixException("Fail to get instance config for " + instanceName);
diff --git a/helix-core/src/test/java/org/apache/helix/util/TestInstanceValidationUtil.java b/helix-core/src/test/java/org/apache/helix/util/TestInstanceValidationUtil.java
index 2b193b4..be5d136 100644
--- a/helix-core/src/test/java/org/apache/helix/util/TestInstanceValidationUtil.java
+++ b/helix-core/src/test/java/org/apache/helix/util/TestInstanceValidationUtil.java
@@ -2,6 +2,7 @@ package org.apache.helix.util;
 
 import static org.mockito.Mockito.*;
 
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 
@@ -158,11 +159,23 @@ public class TestInstanceValidationUtil {
   }
 
   @Test
+  public void TestHasDisabledPartitions_with_only_names() {
+    Mock mock = new Mock();
+    InstanceConfig instanceConfig = mock(InstanceConfig.class);
+    when(instanceConfig.getDisabledPartitionsMap())
+        .thenReturn(ImmutableMap.of("db0", Collections.emptyList()));
+    when(mock.dataAccessor.getProperty(any(PropertyKey.class))).thenReturn(instanceConfig);
+
+    Assert.assertFalse(InstanceValidationUtil.hasDisabledPartitions(mock.dataAccessor, TEST_CLUSTER,
+        TEST_INSTANCE));
+  }
+
+  @Test
   public void TestHasDisabledPartitions_true() {
     Mock mock = new Mock();
     InstanceConfig instanceConfig = mock(InstanceConfig.class);
     when(instanceConfig.getDisabledPartitionsMap())
-        .thenReturn(ImmutableMap.of("db0", Collections.<String> emptyList()));
+        .thenReturn(ImmutableMap.of("db0", Arrays.asList("p1")));
     when(mock.dataAccessor.getProperty(any(PropertyKey.class))).thenReturn(instanceConfig);
 
     Assert.assertTrue(InstanceValidationUtil.hasDisabledPartitions(mock.dataAccessor, TEST_CLUSTER,
diff --git a/helix-rest/src/test/java/org/apache/helix/rest/server/TestInstancesAccessor.java b/helix-rest/src/test/java/org/apache/helix/rest/server/TestInstancesAccessor.java
index c3c8691..0e86584 100644
--- a/helix-rest/src/test/java/org/apache/helix/rest/server/TestInstancesAccessor.java
+++ b/helix-rest/src/test/java/org/apache/helix/rest/server/TestInstancesAccessor.java
@@ -50,7 +50,7 @@ public class TestInstancesAccessor extends AbstractTestClass {
   }
 
   @Test(dependsOnMethods = "testInstancesStoppable_zoneBased")
-  public void testInstancesStoppable_disableOneInstance() {
+  public void testInstancesStoppable_disableOneInstance() throws InterruptedException {
     // Disable one selected instance0, it should failed to check
     String instance = "instance0";
     InstanceConfig instanceConfig = _configAccessor.getInstanceConfig(STOPPABLE_CLUSTER, instance);
@@ -58,6 +58,11 @@ public class TestInstancesAccessor extends AbstractTestClass {
     instanceConfig.setInstanceEnabledForPartition("FakeResource", "FakePartition", false);
     _configAccessor.setInstanceConfig(STOPPABLE_CLUSTER, instance, instanceConfig);
 
+    // It takes time to reflect the changes.
+    BestPossibleExternalViewVerifier verifier =
+        new BestPossibleExternalViewVerifier.Builder(STOPPABLE_CLUSTER).setZkAddr(ZK_ADDR).build();
+    Assert.assertTrue(verifier.verifyByPolling());
+
     Entity entity = Entity.entity("", MediaType.APPLICATION_JSON_TYPE);
     Response response = new JerseyUriRequestBuilder("clusters/{}/instances/{}/stoppable")
         .format(STOPPABLE_CLUSTER, instance).post(this, entity);
@@ -69,8 +74,6 @@ public class TestInstancesAccessor extends AbstractTestClass {
     instanceConfig.setInstanceEnabled(true);
     instanceConfig.setInstanceEnabledForPartition("FakeResource", "FakePartition", true);
     _configAccessor.setInstanceConfig(STOPPABLE_CLUSTER, instance, instanceConfig);
-    BestPossibleExternalViewVerifier verifier =
-        new BestPossibleExternalViewVerifier.Builder(STOPPABLE_CLUSTER).setZkAddr(ZK_ADDR).build();
     Assert.assertTrue(verifier.verifyByPolling());
 
     entity = Entity.entity("", MediaType.APPLICATION_JSON_TYPE);