Kathy Sun has posted comments on this change. Change subject: IMPALA-3981: Fix crash when accessing statestored / catalogd /memz page ......................................................................
Patch Set 4: (7 comments) http://gerrit.cloudera.org:8080/#/c/3998/4/be/src/catalog/catalogd-main.cc File be/src/catalog/catalogd-main.cc: PS4, Line 71: scoped_ptr<MetricGroup> metrics(new MetricGroup("catalog")); > This is the metric group you should pass in to AddDefaulUrlCallbacks(). Sam Done http://gerrit.cloudera.org:8080/#/c/3998/4/be/src/util/default-path-handlers.cc File be/src/util/default-path-handlers.cc: PS4, Line 112: Value jvm(kObjectType); > move this inside the if statement, to line 115. Done PS4, Line 113: GetChildGroup > FindChildGroup(), right? Done http://gerrit.cloudera.org:8080/#/c/3998/4/be/src/util/default-path-handlers.h File be/src/util/default-path-handlers.h: PS4, Line 27: class MetricGroup; > the reason we do this is that it removes a compilation dependency between t I see. It's my first time to know that. THX! http://gerrit.cloudera.org:8080/#/c/3998/4/be/src/util/metrics.cc File be/src/util/metrics.cc: Line 210: else return NULL; > remove 'else' Done http://gerrit.cloudera.org:8080/#/c/3998/4/be/src/util/metrics.h File be/src/util/metrics.h: PS4, Line 302: GetChildGroup > Can you rename this to GetOrCreateChildGroup() now? Done PS4, Line 304: ; > Better to say something like "Returns a child metric group with name 'name' Done -- To view, visit http://gerrit.cloudera.org:8080/3998 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If59b10f20044d1a468f27810a3029fe18fb19f29 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Kathy Sun <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: Kathy Sun <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
