Todd Lipcon 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 4:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/10998/4/be/src/util/default-path-handlers.h
File be/src/util/default-path-handlers.h:

http://gerrit.cloudera.org:8080/#/c/10998/4/be/src/util/default-path-handlers.h@34
PS4, Line 34:     MetricGroup* metric_group = NULL, bool init_jmx = false);
why did you make the jmx path optional here? Is this because the statestore 
doesn't use Java or something? In that case could we instead just use getJNIEnv 
or something else to determine it rather than adding this param?


http://gerrit.cloudera.org:8080/#/c/10998/4/fe/src/main/java/org/apache/impala/util/JMXJsonUtil.java
File fe/src/main/java/org/apache/impala/util/JMXJsonUtil.java:

http://gerrit.cloudera.org:8080/#/c/10998/4/fe/src/main/java/org/apache/impala/util/JMXJsonUtil.java@71
PS4, Line 71: public class JMXJsonUtil {
I didn't review this file particularly since it's just cross-ported. Any 
particular bits that merit a special look that you had to change?


http://gerrit.cloudera.org:8080/#/c/10998/4/fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java
File fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java:

http://gerrit.cloudera.org:8080/#/c/10998/4/fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java@42
PS4, Line 42: public class JvmPauseMonitor {
same here, any changes worth noting to take a look at?



--
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: 4
Gerrit-Owner: Bharath Vissapragada <[email protected]>
Gerrit-Reviewer: Bharath Vissapragada <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Tianyi Wang <[email protected]>
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Vuk Ercegovac <[email protected]>
Gerrit-Comment-Date: Fri, 27 Jul 2018 22:58:37 +0000
Gerrit-HasComments: Yes

Reply via email to