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 2020/03/05 22:43:38 UTC

[helix] branch master updated: Add logs and throw exceptions in getInstanceById (#858)

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


The following commit(s) were added to refs/heads/master by this push:
     new 89dfe91  Add logs and throw exceptions in getInstanceById (#858)
89dfe91 is described below

commit 89dfe91345bde73a0f6928118a89bd8338399aa5
Author: Hunter Lee <hu...@linkedin.com>
AuthorDate: Thu Mar 5 14:43:28 2020 -0800

    Add logs and throw exceptions in getInstanceById (#858)
    
    Add logs and throw exceptions in getInstanceById
---
 .../java/org/apache/helix/util/InstanceValidationUtil.java  | 13 +++++++++++++
 .../rest/server/resources/helix/PerInstanceAccessor.java    | 11 +++++++++--
 .../helix/rest/server/service/InstanceServiceImpl.java      |  8 ++++++++
 3 files changed, 30 insertions(+), 2 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 a74d632..055e797 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
@@ -257,6 +257,9 @@ public class InstanceValidationUtil {
   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!");
     }
@@ -276,7 +279,17 @@ public class InstanceValidationUtil {
       }
       for (String partition : idealState.getPartitionSet()) {
         Map<String, String> isPartitionMap = idealState.getInstanceStateMap(partition);
+        if (isPartitionMap == null) {
+          throw new HelixException(String
+              .format("Partition %s of resource %s does not have an ideal state partition map",
+                  partition, idealStateName));
+        }
         Map<String, String> evPartitionMap = externalView.getStateMap(partition);
+        if (evPartitionMap == null) {
+          throw new HelixException(String
+              .format("Partition %s of resource %s does not have an external view partition map",
+                  partition, idealStateName));
+        }
         if (isPartitionMap.containsKey(instanceName) && (!evPartitionMap.containsKey(instanceName)
             || !evPartitionMap.get(instanceName).equals(isPartitionMap.get(instanceName)))) {
           // only checks the state from IS matches EV. Return false when
diff --git a/helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/PerInstanceAccessor.java b/helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/PerInstanceAccessor.java
index 2f861bc..d8d087f 100644
--- a/helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/PerInstanceAccessor.java
+++ b/helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/PerInstanceAccessor.java
@@ -35,6 +35,7 @@ import javax.ws.rs.QueryParam;
 import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;
 
+import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import org.apache.helix.ConfigAccessor;
 import org.apache.helix.HelixAdmin;
@@ -85,7 +86,7 @@ public class PerInstanceAccessor extends AbstractHelixResource {
   @GET
   public Response getInstanceById(@PathParam("clusterId") String clusterId,
       @PathParam("instanceName") String instanceName,
-      @DefaultValue("getInstance") @QueryParam("command") String command) throws IOException {
+      @DefaultValue("getInstance") @QueryParam("command") String command) {
     // Get the command. If not provided, the default would be "getInstance"
     Command cmd;
     try {
@@ -103,7 +104,13 @@ public class PerInstanceAccessor extends AbstractHelixResource {
           new HelixDataAccessorWrapper((ZKHelixDataAccessor) dataAccessor), getConfigAccessor());
       InstanceInfo instanceInfo = instanceService.getInstanceInfo(clusterId, instanceName,
           InstanceService.HealthCheck.STARTED_AND_HEALTH_CHECK_LIST);
-      return OK(objectMapper.writeValueAsString(instanceInfo));
+      String instanceInfoString;
+      try {
+        instanceInfoString = objectMapper.writeValueAsString(instanceInfo);
+      } catch (JsonProcessingException e) {
+        return serverError(e);
+      }
+      return OK(instanceInfoString);
     case validateWeight:
       // Validates instanceConfig for WAGED rebalance
       HelixAdmin admin = getHelixAdmin();
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 9d5749f..9311544 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
@@ -87,6 +87,8 @@ public class InstanceServiceImpl implements InstanceService {
         _dataAccessor.getProperty(_dataAccessor.keyBuilder().liveInstance(instanceName));
     if (instanceConfig != null) {
       instanceInfoBuilder.instanceConfig(instanceConfig.getRecord());
+    } else {
+      LOG.warn("Missing instance config for {}", instanceName);
     }
     if (liveInstance != null) {
       instanceInfoBuilder.liveInstance(liveInstance.getRecord());
@@ -101,9 +103,15 @@ public class InstanceServiceImpl implements InstanceService {
             _dataAccessor.keyBuilder().currentState(instanceName, sessionId, resourceName));
         if (currentState != null && currentState.getPartitionStateMap() != null) {
           partitions.addAll(currentState.getPartitionStateMap().keySet());
+        } else {
+          LOG.warn(
+              "Current state is either null or partitionStateMap is missing. InstanceName: {}, SessionId: {}, ResourceName: {}",
+              instanceName, sessionId, resourceName);
         }
       }
       instanceInfoBuilder.partitions(partitions);
+    } else {
+      LOG.warn("Missing live instance for {}", instanceName);
     }
     try {
       Map<String, Boolean> healthStatus =