asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael Blow (Code Review)" <do-not-re...@asterixdb.incubator.apache.org>
Subject Change in asterixdb[master]: [ASTERIXDB-2142][*DB][API] Guard against unavailable nc detail
Date Tue, 24 Oct 2017 19:44:54 GMT
Michael Blow has submitted this change and it was merged.

Change subject: [ASTERIXDB-2142][*DB][API] Guard against unavailable nc detail
......................................................................


[ASTERIXDB-2142][*DB][API] Guard against unavailable nc detail

hcc.getNodeDetailsJSON() & hcc.getThreadDump() APIs returns the null
string in case of unknown or not connected node- handle these in the
HTTP API.

Change-Id: I06a00f191812d25a6fef6c7cae7d693b258a6b6d
Reviewed-on: https://asterix-gerrit.ics.uci.edu/2096
Sonar-Qube: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Tested-by: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Contrib: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Integration-Tests: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Reviewed-by: Murtadha Hubail <mhubail@apache.org>
---
M asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ClusterControllerDetailsApiServlet.java
M asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/DiagnosticsApiServlet.java
M asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/NodeControllerDetailsApiServlet.java
M asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ShutdownApiServlet.java
M hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/client/IHyracksClientConnection.java
5 files changed, 55 insertions(+), 28 deletions(-)

Approvals:
  Anon. E. Moose #1000171: 
  Jenkins: Verified; No violations found; ; Verified
  Murtadha Hubail: Looks good to me, approved



diff --git a/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ClusterControllerDetailsApiServlet.java
b/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ClusterControllerDetailsApiServlet.java
index 1faa316..dcd43cb 100644
--- a/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ClusterControllerDetailsApiServlet.java
+++ b/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ClusterControllerDetailsApiServlet.java
@@ -81,9 +81,9 @@
         } else if (parts.length == 1) {
             switch (parts[0]) {
                 case "config":
-                    return OBJECT_MAPPER.readValue(hcc.getNodeDetailsJSON(null, false, true),
ObjectNode.class);
+                    return OBJECT_MAPPER.readValue(processNodeDetails(hcc, false, true),
ObjectNode.class);
                 case "stats":
-                    return OBJECT_MAPPER.readValue(hcc.getNodeDetailsJSON(null, true, false),
ObjectNode.class);
+                    return OBJECT_MAPPER.readValue(processNodeDetails(hcc, true, false),
ObjectNode.class);
                 case "threaddump":
                     return processCCThreadDump(hcc);
 
@@ -96,10 +96,19 @@
         }
     }
 
+    private String processNodeDetails(IHyracksClientConnection hcc, boolean includeStats,
boolean includeConfig)
+            throws Exception {
+        final String details = hcc.getNodeDetailsJSON(null, includeStats, includeConfig);
+        if (details == null) {
+            throw new IllegalStateException("unable to retrieve details for CC");
+        }
+        return details;
+    }
+
     private ObjectNode processCCThreadDump(IHyracksClientConnection hcc) throws Exception
{
         String dump = hcc.getThreadDump(null);
         if (dump == null) {
-            throw new IllegalArgumentException();
+            throw new IllegalStateException("unable to retrieve thread dump for CC");
         }
         return (ObjectNode) OBJECT_MAPPER.readTree(dump);
     }
diff --git a/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/DiagnosticsApiServlet.java
b/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/DiagnosticsApiServlet.java
index eafda09..a79b137 100644
--- a/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/DiagnosticsApiServlet.java
+++ b/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/DiagnosticsApiServlet.java
@@ -106,9 +106,9 @@
         Map<String, Future<JsonNode>> ncData;
         ncData = new HashMap<>();
         ncData.put("threaddump",
-                executor.submit(() -> fixupKeys((ObjectNode) OBJECT_MAPPER.readTree(hcc.getThreadDump(nc)))));
+                executor.submit(() -> fixupKeys((ObjectNode) OBJECT_MAPPER.readTree(processThreadDump(nc)))));
         ncData.put("config", executor
-                .submit(() -> fixupKeys((ObjectNode) OBJECT_MAPPER.readTree(hcc.getNodeDetailsJSON(nc,
false, true)))));
+                .submit(() -> fixupKeys((ObjectNode) OBJECT_MAPPER.readTree(processNodeDetails(nc,
false, true)))));
         ncData.put("stats", executor.submit(() -> fixupKeys(processNodeStats(hcc, nc))));
         return ncData;
     }
@@ -117,11 +117,11 @@
         Map<String, Future<JsonNode>> ccFutureData;
         ccFutureData = new HashMap<>();
         ccFutureData.put("threaddump",
-                executor.submit(() -> fixupKeys((ObjectNode) OBJECT_MAPPER.readTree(hcc.getThreadDump(null)))));
+                executor.submit(() -> fixupKeys((ObjectNode) OBJECT_MAPPER.readTree(processThreadDump(null)))));
         ccFutureData.put("config", executor.submit(
-                () -> fixupKeys((ObjectNode) OBJECT_MAPPER.readTree(hcc.getNodeDetailsJSON(null,
false, true)))));
+                () -> fixupKeys((ObjectNode) OBJECT_MAPPER.readTree(processNodeDetails(null,
false, true)))));
         ccFutureData.put("stats", executor.submit(
-                () -> fixupKeys((ObjectNode) OBJECT_MAPPER.readTree(hcc.getNodeDetailsJSON(null,
true, false)))));
+                () -> fixupKeys((ObjectNode) OBJECT_MAPPER.readTree(processNodeDetails(null,
true, false)))));
         return ccFutureData;
     }
 
@@ -143,4 +143,13 @@
             }
         }
     }
+
+    protected String processNodeDetails(String node, boolean includeStats, boolean includeConfig)
throws Exception {
+        return checkNullDetail(node, hcc.getNodeDetailsJSON(node, includeStats, includeConfig));
+    }
+
+    protected String processThreadDump(String node) throws Exception {
+        return checkNullDetail(node, hcc.getThreadDump(node));
+    }
+
 }
diff --git a/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/NodeControllerDetailsApiServlet.java
b/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/NodeControllerDetailsApiServlet.java
index 2c94f86..f443d09 100644
--- a/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/NodeControllerDetailsApiServlet.java
+++ b/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/NodeControllerDetailsApiServlet.java
@@ -40,6 +40,7 @@
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.node.ArrayNode;
 import com.fasterxml.jackson.databind.node.ObjectNode;
+
 import io.netty.handler.codec.http.HttpResponseStatus;
 
 public class NodeControllerDetailsApiServlet extends ClusterApiServlet {
@@ -139,10 +140,7 @@
     }
 
     protected ObjectNode processNodeStats(IHyracksClientConnection hcc, String node) throws
Exception {
-        final String details = hcc.getNodeDetailsJSON(node, true, false);
-        if (details == null) {
-            throw new IllegalArgumentException();
-        }
+        final String details = checkNullDetail(node, hcc.getNodeDetailsJSON(node, true, false));
         ObjectNode json = (ObjectNode) OBJECT_MAPPER.readTree(details);
         int index = json.get("rrd-ptr").asInt() - 1;
         json.remove("rrd-ptr");
@@ -189,10 +187,7 @@
     }
 
     private ObjectNode processNodeConfig(IHyracksClientConnection hcc, String node) throws
Exception {
-        String config = hcc.getNodeDetailsJSON(node, false, true);
-        if (config == null) {
-            throw new IllegalArgumentException();
-        }
+        String config = checkNullDetail(node, hcc.getNodeDetailsJSON(node, false, true));
         return (ObjectNode) OBJECT_MAPPER.readTree(config);
     }
 
@@ -200,13 +195,22 @@
         if ("cc".equals(node)) {
             return OBJECT_MAPPER.createObjectNode();
         }
-        String dump = hcc.getThreadDump(node);
-        if (dump == null) {
-            // check to see if this is a node that is simply down
-            IClusterStateManager csm = appCtx.getClusterStateManager();
-            ClusterPartition[] cp = csm.getNodePartitions(node);
-            throw cp != null ? new IllegalStateException() : new IllegalArgumentException();
-        }
+        String dump = checkNullDetail(node, hcc.getThreadDump(node));
         return (ObjectNode) OBJECT_MAPPER.readTree(dump);
     }
+
+    protected String checkNullDetail(String node, String value) {
+        if (value != null) {
+            return value;
+        }
+        if (node == null) {
+            // something is seriously wrong if we can't get the cc detail
+            throw new IllegalStateException("unable to obtain detail from cc");
+        }
+        // check to see if this is a node that is simply down
+        IClusterStateManager csm = appCtx.getClusterStateManager();
+        ClusterPartition[] cp = csm.getNodePartitions(node);
+        throw cp != null ? new IllegalStateException("unable to obtain detail from node "
+ node)
+                : new IllegalArgumentException("unknown node " + node);
+    }
 }
diff --git a/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ShutdownApiServlet.java
b/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ShutdownApiServlet.java
index ddd0f1f..ac31e24 100644
--- a/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ShutdownApiServlet.java
+++ b/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ShutdownApiServlet.java
@@ -86,10 +86,15 @@
             for (int i = 0; i < ncs.size(); i++) {
                 ObjectNode nc = (ObjectNode) ncs.get(i);
                 String node = nc.get(NODE_ID_KEY).asText();
-                ObjectNode details = (ObjectNode) OBJECT_MAPPER.readTree(hcc.getNodeDetailsJSON(node,
false, true));
-                nc.set(PID, details.get(PID));
-                if (details.has(INI) && details.get(INI).has(NCSERVICE_PID)) {
-                    nc.put(NCSERVICE_PID, details.get(INI).get(NCSERVICE_PID).asInt());
+                final String detailsString = hcc.getNodeDetailsJSON(node, false, true);
+                if (detailsString != null) {
+                    ObjectNode details = (ObjectNode) OBJECT_MAPPER.readTree(detailsString);
+                    nc.set(PID, details.get(PID));
+                    if (details.has(INI) && details.get(INI).has(NCSERVICE_PID))
{
+                        nc.put(NCSERVICE_PID, details.get(INI).get(NCSERVICE_PID).asInt());
+                    }
+                } else {
+                    LOGGER.warning("Unable to get node details for " + node + " from hcc");
                 }
             }
             jsonObject.set("cluster", clusterState);
diff --git a/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/client/IHyracksClientConnection.java
b/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/client/IHyracksClientConnection.java
index 0189135..a7c1d75 100644
--- a/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/client/IHyracksClientConnection.java
+++ b/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/client/IHyracksClientConnection.java
@@ -211,7 +211,7 @@
      *            id the subject node
      * @param includeStats
      * @param includeConfig
-     * @return serialized JSON containing the node details
+     * @return serialized JSON containing the node details, or null if the details are not
available (e.g. NC down)
      * @throws Exception
      */
     String getNodeDetailsJSON(String nodeId, boolean includeStats, boolean includeConfig)
throws Exception;

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/2096
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I06a00f191812d25a6fef6c7cae7d693b258a6b6d
Gerrit-PatchSet: 2
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Michael Blow <mblow@apache.org>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Blow <mblow@apache.org>
Gerrit-Reviewer: Murtadha Hubail <mhubail@apache.org>
Gerrit-Reviewer: Till Westmann <tillw@apache.org>
Gerrit-Reviewer: abdullah alamoudi <bamousaa@gmail.com>

Mime
View raw message