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 <[email protected]> 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 <[email protected]> --- .../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); }
