You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@helix.apache.org by hz...@apache.org on 2020/08/28 19:33:20 UTC
[helix] branch master updated: Fix empty host list for instance
stoppable response (#1237)
This is an automated email from the ASF dual-hosted git repository.
hzlu 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 54a96c0 Fix empty host list for instance stoppable response (#1237)
54a96c0 is described below
commit 54a96c038479737e813b7351a75849a2925ccc89
Author: Huizhi Lu <ih...@gmail.com>
AuthorDate: Fri Aug 28 12:33:10 2020 -0700
Fix empty host list for instance stoppable response (#1237)
Health check stoppable responds with empty hostname if persist assignment config is disabled.
This commit adds INVALID_CONFIG to the response for users script to parse and understand.
---
.../apache/helix/util/InstanceValidationUtil.java | 25 ++++++++++++----------
.../helix/util/TestInstanceValidationUtil.java | 2 +-
.../helix/rest/server/service/InstanceService.java | 6 +++---
.../rest/server/service/InstanceServiceImpl.java | 19 ++++++++++++----
.../rest/server/service/TestInstanceService.java | 1 -
5 files changed, 33 insertions(+), 20 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 530077d..bc3b48f 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
@@ -146,7 +146,9 @@ public class InstanceValidationUtil {
}
/**
- * Method to check if the instance has valid configuration
+ * Method to check if the instance has valid configuration.
+ * Instance stability check requires PERSIST_INTERMEDIATE_ASSIGNMENT turned on!
+ *
* @param dataAccessor
* @param clusterId
* @param instanceName
@@ -154,7 +156,17 @@ public class InstanceValidationUtil {
*/
public static boolean hasValidConfig(HelixDataAccessor dataAccessor, String clusterId,
String instanceName) {
- PropertyKey propertyKey = dataAccessor.keyBuilder().instanceConfig(instanceName);
+ PropertyKey.Builder keyBuilder = dataAccessor.keyBuilder();
+ ClusterConfig clusterConfig = dataAccessor.getProperty(keyBuilder.clusterConfig());
+ if (clusterConfig == null) {
+ throw new HelixException("Cluster config is missing in cluster " + clusterId);
+ }
+ if (!clusterConfig.isPersistIntermediateAssignment()) {
+ throw new HelixException(String.format(
+ "Cluster config %s is not turned on, which is required for instance stability check.",
+ ClusterConfig.ClusterConfigProperty.PERSIST_INTERMEDIATE_ASSIGNMENT.toString()));
+ }
+ PropertyKey propertyKey = keyBuilder.instanceConfig(instanceName);
InstanceConfig instanceConfig = dataAccessor.getProperty(propertyKey);
return instanceConfig != null && instanceConfig.isValid();
}
@@ -249,21 +261,12 @@ public class InstanceValidationUtil {
/**
* Check instance is already in the stable state. Here stable means all the ideal state mapping
* matches external view (view of current state).
- * It requires PERSIST_INTERMEDIATE_ASSIGNMENT turned on!
* @param dataAccessor
* @param instanceName
* @return
*/
public static boolean isInstanceStable(HelixDataAccessor dataAccessor, String instanceName) {
PropertyKey.Builder keyBuilder = dataAccessor.keyBuilder();
- ClusterConfig clusterConfig = dataAccessor.getProperty(keyBuilder.clusterConfig());
- if (clusterConfig == null) {
- throw new HelixException("Missing cluster config!");
- }
- if (!clusterConfig.isPersistIntermediateAssignment()) {
- throw new HelixException("isInstanceStable needs persist assignment on!");
- }
-
List<String> idealStateNames = dataAccessor.getChildNames(keyBuilder.idealStates());
for (String idealStateName : idealStateNames) {
IdealState idealState = dataAccessor.getProperty(keyBuilder.idealStates(idealStateName));
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 65a9f4b..f8de6ae 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
@@ -279,7 +279,7 @@ public class TestInstanceValidationUtil {
clusterConfig.setPersistIntermediateAssignment(false);
when(mock.dataAccessor.getProperty(any(PropertyKey.class))).thenReturn(clusterConfig);
- InstanceValidationUtil.isInstanceStable(mock.dataAccessor, TEST_INSTANCE);
+ InstanceValidationUtil.hasValidConfig(mock.dataAccessor, TEST_CLUSTER, TEST_INSTANCE);
}
@Test(expectedExceptions = HelixException.class)
diff --git a/helix-rest/src/main/java/org/apache/helix/rest/server/service/InstanceService.java b/helix-rest/src/main/java/org/apache/helix/rest/server/service/InstanceService.java
index eae947f..ce8bc5e 100644
--- a/helix-rest/src/main/java/org/apache/helix/rest/server/service/InstanceService.java
+++ b/helix-rest/src/main/java/org/apache/helix/rest/server/service/InstanceService.java
@@ -72,9 +72,9 @@ public interface InstanceService {
/**
* Pre-defined list of checks to test if an instance is in healthy running state
*/
- public static List<HealthCheck> STARTED_AND_HEALTH_CHECK_LIST =
- ImmutableList.of(HealthCheck.INSTANCE_NOT_ALIVE, HealthCheck.INSTANCE_NOT_ENABLED,
- HealthCheck.INSTANCE_NOT_STABLE, HealthCheck.EMPTY_RESOURCE_ASSIGNMENT);
+ public static List<HealthCheck> STARTED_AND_HEALTH_CHECK_LIST = ImmutableList
+ .of(INVALID_CONFIG, INSTANCE_NOT_ALIVE, INSTANCE_NOT_ENABLED, INSTANCE_NOT_STABLE,
+ EMPTY_RESOURCE_ASSIGNMENT);
}
/**
diff --git a/helix-rest/src/main/java/org/apache/helix/rest/server/service/InstanceServiceImpl.java b/helix-rest/src/main/java/org/apache/helix/rest/server/service/InstanceServiceImpl.java
index d4bbff6..5099ec8 100644
--- a/helix-rest/src/main/java/org/apache/helix/rest/server/service/InstanceServiceImpl.java
+++ b/helix-rest/src/main/java/org/apache/helix/rest/server/service/InstanceServiceImpl.java
@@ -278,12 +278,23 @@ public class InstanceServiceImpl implements InstanceService {
for (HealthCheck healthCheck : healthChecks) {
switch (healthCheck) {
case INVALID_CONFIG:
- healthStatus.put(HealthCheck.INVALID_CONFIG.name(),
- InstanceValidationUtil.hasValidConfig(_dataAccessor, clusterId, instanceName));
- if (!healthStatus.get(HealthCheck.INVALID_CONFIG.name())) {
- LOG.error("The instance {} doesn't have valid configuration", instanceName);
+ boolean validConfig;
+ try {
+ validConfig =
+ InstanceValidationUtil.hasValidConfig(_dataAccessor, clusterId, instanceName);
+ } catch (HelixException e) {
+ validConfig = false;
+ LOG.warn("Cluster {} instance {} doesn't have valid config: {}", clusterId, instanceName,
+ e.getMessage());
+ }
+
+ // TODO: should add reason to request response
+ healthStatus.put(HealthCheck.INVALID_CONFIG.name(), validConfig);
+ if (!validConfig) {
+ // No need to do remaining health checks.
return healthStatus;
}
+ break;
case INSTANCE_NOT_ENABLED:
healthStatus.put(HealthCheck.INSTANCE_NOT_ENABLED.name(),
InstanceValidationUtil.isEnabled(_dataAccessor, instanceName));
diff --git a/helix-rest/src/test/java/org/apache/helix/rest/server/service/TestInstanceService.java b/helix-rest/src/test/java/org/apache/helix/rest/server/service/TestInstanceService.java
index 94b2129..f785124 100644
--- a/helix-rest/src/test/java/org/apache/helix/rest/server/service/TestInstanceService.java
+++ b/helix-rest/src/test/java/org/apache/helix/rest/server/service/TestInstanceService.java
@@ -59,7 +59,6 @@ import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyZeroInteractions;
import static org.mockito.Mockito.when;
-
public class TestInstanceService {
private static final String TEST_CLUSTER = "TestCluster";
private static final String TEST_INSTANCE = "instance0.linkedin.com_1235";