Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/10998 )
Change subject: IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics ...................................................................... Patch Set 11: (2 comments) http://gerrit.cloudera.org:8080/#/c/10998/11/be/src/util/default-path-handlers.cc File be/src/util/default-path-handlers.cc: http://gerrit.cloudera.org:8080/#/c/10998/11/be/src/util/default-path-handlers.cc@207 PS11, Line 207: // Parse the JSON string returned from fe. > It's pretty weird that we're doing an extra round of parsing. I think it's It is unfortunate. We use two different libraries to parse the same json, once in C++ and one in Java and there is no intermediate state that is compatible with both to avoid re-parsing. Yea, the whole rendering framework relies on the JSON content underneath that requires the content to be populated that way for it to parse. Clarified. http://gerrit.cloudera.org:8080/#/c/10998/11/be/src/util/default-path-handlers.cc@261 PS11, Line 261: webserver->RegisterUrlCallback("/jmx", "raw_text.tmpl", JmxHandler); > Do you think raw_text.tmpl is ever ever used? I think because we set the "_ You are right, the template here is of no use since we do a non-HTML rendering anyway. I thought about this but it turns out we set a PLAIN content-type for RawCallbacks. Now we need to again fix the following codepath to infer from the arguments map whether it is a JSON and fix the content type accordingly. I felt it was not worth the effort. Thoughts? if (!url_handler->use_templates()) { content_type = PLAIN; url_handler->raw_callback()(arguments, &output); <----- } else { RenderUrlWithTemplate(arguments, *url_handler, &output, &content_type); } -- To view, visit http://gerrit.cloudera.org:8080/10998 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I30d897b7e063846ad6d8f88243e2c04264da0341 Gerrit-Change-Number: 10998 Gerrit-PatchSet: 11 Gerrit-Owner: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Philip Zeyliger <[email protected]> Gerrit-Reviewer: Tianyi Wang <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Reviewer: Vuk Ercegovac <[email protected]> Gerrit-Comment-Date: Wed, 08 Aug 2018 00:26:48 +0000 Gerrit-HasComments: Yes
