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 2019/05/25 01:20:04 UTC

[helix] 30/44: Bug fix: reuse the stable logics to verfiy the difference between idealStates and externalViews

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 745868b3f6fb35d1bdd7d39b62132eae85279783
Author: Yi Wang <yw...@linkedin.com>
AuthorDate: Fri May 3 16:03:37 2019 -0700

    Bug fix: reuse the stable logics to verfiy the difference between idealStates and externalViews
    
    RB=1654700
    G=helix-reviewers
    A=jxue
    
    Signed-off-by: Hunter Lee <hu...@linkedin.com>
---
 .../apache/helix/util/InstanceValidationUtil.java  | 39 +++++++++++---------
 .../helix/util/TestInstanceValidationUtil.java     | 41 +++++++++++++++++-----
 2 files changed, 55 insertions(+), 25 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 7d23117..2d4d2ba 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
@@ -19,20 +19,28 @@ package org.apache.helix.util;
  * under the License.
  */
 
-import java.util.*;
-import java.util.stream.Collectors;
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
 
 import org.apache.helix.HelixDataAccessor;
 import org.apache.helix.HelixDefinedState;
 import org.apache.helix.HelixException;
 import org.apache.helix.PropertyKey;
-import org.apache.helix.model.*;
+import org.apache.helix.model.ClusterConfig;
+import org.apache.helix.model.CurrentState;
+import org.apache.helix.model.ExternalView;
+import org.apache.helix.model.IdealState;
+import org.apache.helix.model.InstanceConfig;
+import org.apache.helix.model.LiveInstance;
+import org.apache.helix.model.StateModelDefinition;
 import org.apache.helix.task.TaskConstants;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Sets;
 
 /**
  * Utility class for validating Helix properties
@@ -279,26 +287,25 @@ public class InstanceValidationUtil {
    */
   public static boolean siblingNodesActiveReplicaCheck(HelixDataAccessor dataAccessor, String instanceName) {
     PropertyKey.Builder propertyKeyBuilder = dataAccessor.keyBuilder();
-    List<String> idealStates = dataAccessor.getChildNames(propertyKeyBuilder.idealStates());
-    List<String> externalViews = dataAccessor.getChildNames(propertyKeyBuilder.externalViews());
-    if (idealStates.size() != externalViews.size()) {
-      throw new HelixException(
-          "The following resources in IdealStates are not found in ExternalViews: "
-              + Sets.difference(new HashSet<>(idealStates), new HashSet<>(externalViews)));
-    }
+    List<String> resources = dataAccessor.getChildNames(propertyKeyBuilder.idealStates());
 
-    for (String externalViewName : externalViews) {
+    for (String resourceName : resources) {
+      IdealState idealState = dataAccessor.getProperty(propertyKeyBuilder.idealStates(resourceName));
+      if (idealState == null || !idealState.isEnabled() || !idealState.isValid()
+          || TaskConstants.STATE_MODEL_NAME.equals(idealState.getStateModelDefRef())) {
+        continue;
+      }
       ExternalView externalView =
-          dataAccessor.getProperty(propertyKeyBuilder.externalView(externalViewName));
+          dataAccessor.getProperty(propertyKeyBuilder.externalView(resourceName));
       if (externalView == null) {
-        _logger.error("ExternalView for {} doesn't exist", externalViewName);
-        continue;
+        throw new HelixException(
+            String.format("Resource %s does not have external view!", resourceName));
       }
       // Get the minActiveReplicas constraint for the resource
       int minActiveReplicas = externalView.getMinActiveReplicas();
       if (minActiveReplicas == -1) {
         throw new HelixException(
-            "ExternalView " + externalViewName + " is missing minActiveReplica field");
+            "ExternalView " + resourceName + " is missing minActiveReplica field");
       }
       String stateModeDef = externalView.getStateModelDefRef();
       StateModelDefinition stateModelDefinition =
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 cbbbfd6..38b54f1 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
@@ -331,8 +331,14 @@ public class TestInstanceValidationUtil {
     Mock mock = new Mock();
     doReturn(ImmutableList.of(resource)).when(mock.dataAccessor)
         .getChildNames(argThat(new PropertyKeyArgument(PropertyType.EXTERNALVIEW)));
-    doReturn(ImmutableList.of(resource)).when(mock.dataAccessor)
-        .getChildNames(argThat(new PropertyKeyArgument(PropertyType.IDEALSTATES)));
+    // set ideal state
+    IdealState idealState = mock(IdealState.class);
+    when(idealState.isEnabled()).thenReturn(true);
+    when(idealState.isValid()).thenReturn(true);
+    when(idealState.getStateModelDefRef()).thenReturn("MasterSlave");
+    doReturn(idealState).when(mock.dataAccessor).getProperty(argThat(new PropertyKeyArgument(PropertyType.IDEALSTATES)));
+
+    // set external view
     ExternalView externalView = mock(ExternalView.class);
     when(externalView.getMinActiveReplicas()).thenReturn(2);
     when(externalView.getStateModelDefRef()).thenReturn("MasterSlave");
@@ -358,8 +364,13 @@ public class TestInstanceValidationUtil {
     Mock mock = new Mock();
     doReturn(ImmutableList.of(resource)).when(mock.dataAccessor)
         .getChildNames(argThat(new PropertyKeyArgument(PropertyType.IDEALSTATES)));
-    doReturn(ImmutableList.of(resource)).when(mock.dataAccessor)
-        .getChildNames(argThat(new PropertyKeyArgument(PropertyType.EXTERNALVIEW)));
+    // set ideal state
+    IdealState idealState = mock(IdealState.class);
+    when(idealState.isEnabled()).thenReturn(true);
+    when(idealState.isValid()).thenReturn(true);
+    when(idealState.getStateModelDefRef()).thenReturn("MasterSlave");
+    doReturn(idealState).when(mock.dataAccessor).getProperty(argThat(new PropertyKeyArgument(PropertyType.IDEALSTATES)));
+
     ExternalView externalView = mock(ExternalView.class);
     when(externalView.getMinActiveReplicas()).thenReturn(3);
     when(externalView.getStateModelDefRef()).thenReturn("MasterSlave");
@@ -385,8 +396,13 @@ public class TestInstanceValidationUtil {
     Mock mock = new Mock();
     doReturn(ImmutableList.of(resource)).when(mock.dataAccessor)
         .getChildNames(argThat(new PropertyKeyArgument(PropertyType.IDEALSTATES)));
-    doReturn(Collections.emptyList()).when(mock.dataAccessor)
-        .getChildNames(argThat(new PropertyKeyArgument(PropertyType.EXTERNALVIEW)));
+    // set ideal state
+    IdealState idealState = mock(IdealState.class);
+    when(idealState.isEnabled()).thenReturn(true);
+    when(idealState.isValid()).thenReturn(true);
+    when(idealState.getStateModelDefRef()).thenReturn("MasterSlave");
+    doReturn(idealState).when(mock.dataAccessor).getProperty(argThat(new PropertyKeyArgument(PropertyType.IDEALSTATES)));
+    //set externalView
     ExternalView externalView = mock(ExternalView.class);
     when(externalView.getMinActiveReplicas()).thenReturn(-1);
     doReturn(externalView).when(mock.dataAccessor)
@@ -396,13 +412,20 @@ public class TestInstanceValidationUtil {
   }
 
   @Test(expectedExceptions = HelixException.class)
-  public void TestSiblingNodesActiveReplicaCheck_exception_whenMissingMinActiveReplicas() {
+  public void TestSiblingNodesActiveReplicaCheck_exception_whenExternalViewUnavailable() {
     String resource = "resource";
     Mock mock = new Mock();
     doReturn(ImmutableList.of(resource)).when(mock.dataAccessor)
         .getChildNames(argThat(new PropertyKeyArgument(PropertyType.IDEALSTATES)));
-    doReturn(Collections.emptyList()).when(mock.dataAccessor)
-        .getChildNames(argThat(new PropertyKeyArgument(PropertyType.EXTERNALVIEW)));
+    // set ideal state
+    IdealState idealState = mock(IdealState.class);
+    when(idealState.isEnabled()).thenReturn(true);
+    when(idealState.isValid()).thenReturn(true);
+    when(idealState.getStateModelDefRef()).thenReturn("MasterSlave");
+    doReturn(idealState).when(mock.dataAccessor).getProperty(argThat(new PropertyKeyArgument(PropertyType.IDEALSTATES)));
+
+    doReturn(null).when(mock.dataAccessor)
+        .getProperty(argThat(new PropertyKeyArgument(PropertyType.EXTERNALVIEW)));
 
     InstanceValidationUtil.siblingNodesActiveReplicaCheck(mock.dataAccessor, TEST_INSTANCE);
   }