Henry Robinson has posted comments on this change. Change subject: IMPALA-3715: Include total usage of JVM memory ......................................................................
Patch Set 1: (7 comments) http://gerrit.cloudera.org:8080/#/c/3625/1/be/src/util/default-path-handlers.cc File be/src/util/default-path-handlers.cc: PS1, Line 30: #include "runtime/exec-env.h" keep these ordered alphabetically, so this would go on line 26 Line 31: Remove this extra blank line PS1, Line 110: Value jvm; : jvm.SetObject(); Think you can just do: Value jvm(kObjectType); PS1, Line 115: for(SizeType i = 0; i < jvm["metrics"].Size() ;i++){ format this as follows: for (SizeType i = 0; i < jvm["metrics"].Size(); ++i) { Line 116: if (strstr(jvm["metrics"][i]["name"].GetString(),"total") != nullptr){ space after ',' http://gerrit.cloudera.org:8080/#/c/3625/1/www/memz.tmpl File www/memz.tmpl: PS1, Line 22: 2 Could you fix this while you're here? Line 36: {{#jvm}} nit: align with <tr> below -- To view, visit http://gerrit.cloudera.org:8080/3625 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib44c25eb5a5d70f10a6a120501eec2d50fad5ce9 Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Kathy Sun <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-HasComments: Yes
