[GitHub] kkhatua commented on a change in pull request #1437: DRILL-6702: Disable CPU Reporting for non-HotSpot JDKs
kkhatua commented on a change in pull request #1437: DRILL-6702: Disable CPU
Reporting for non-HotSpot JDKs
URL: https://github.com/apache/drill/pull/1437#discussion_r214162611
##
File path: common/src/main/java/org/apache/drill/exec/metrics/CpuGaugeSet.java
##
@@ -32,13 +32,22 @@
*/
@SuppressWarnings("restriction")
public class CpuGaugeSet implements MetricSet {
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(CpuGaugeSet.class);
- private OperatingSystemMXBean osMXBean;
- private RuntimeMXBean rtMXBean;
+ private final OperatingSystemMXBean osMXBean;
+ private final RuntimeMXBean rtMXBean;
public CpuGaugeSet() {
-this.osMXBean = (OperatingSystemMXBean)
ManagementFactory.getOperatingSystemMXBean();
this.rtMXBean = ManagementFactory.getRuntimeMXBean();
+//DRILL-6702: Instead of worrying about compiling with IBM JDK, for now,
we shall provide no CPU metrics for non-HotSpot JVMs
+OperatingSystemMXBean tempOSMXBean;
+try {
+ tempOSMXBean = (OperatingSystemMXBean)
ManagementFactory.getOperatingSystemMXBean();
+} catch (ClassCastException cce) {
Review comment:
I primarily expect only the ClassCastException, but I've converted this to
catch Exception.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] kkhatua commented on a change in pull request #1437: DRILL-6702: Disable CPU Reporting for non-HotSpot JDKs
kkhatua commented on a change in pull request #1437: DRILL-6702: Disable CPU
Reporting for non-HotSpot JDKs
URL: https://github.com/apache/drill/pull/1437#discussion_r213771375
##
File path: common/src/main/java/org/apache/drill/exec/metrics/CpuGaugeSet.java
##
@@ -63,7 +72,7 @@ public OperatingSystemLoad(OperatingSystemMXBean osBean) {
@Override
public Double getValue() {
-return osMXBean.getSystemLoadAverage();
+return (osMXBean != null) ? osMXBean.getSystemLoadAverage(): null;
Review comment:
For readability. Helps delineate the condition and the 2 values.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] kkhatua commented on a change in pull request #1437: DRILL-6702: Disable CPU Reporting for non-HotSpot JDKs
kkhatua commented on a change in pull request #1437: DRILL-6702: Disable CPU
Reporting for non-HotSpot JDKs
URL: https://github.com/apache/drill/pull/1437#discussion_r213418278
##
File path: common/src/main/java/org/apache/drill/exec/metrics/CpuGaugeSet.java
##
@@ -32,13 +32,22 @@
*/
@SuppressWarnings("restriction")
public class CpuGaugeSet implements MetricSet {
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(CpuGaugeSet.class);
- private OperatingSystemMXBean osMXBean;
- private RuntimeMXBean rtMXBean;
+ private final OperatingSystemMXBean osMXBean;
+ private final RuntimeMXBean rtMXBean;
public CpuGaugeSet() {
-this.osMXBean = (OperatingSystemMXBean)
ManagementFactory.getOperatingSystemMXBean();
this.rtMXBean = ManagementFactory.getRuntimeMXBean();
+//DRILL-6702: Instead of worrying about compiling with IBM JDK, for now,
we shall provide no CPU metrics for non-HotSpot JVMs
+OperatingSystemMXBean tempOSMXBean;
+try {
+ tempOSMXBean = (OperatingSystemMXBean)
ManagementFactory.getOperatingSystemMXBean();
+} catch (ClassCastException cce) {
+ logger.warn("{}. Detected non-Supported JVM [{}]. CPU Metrics in the
WebUI will not be available!", cce.getMessage(),
System.getProperty("java.vm.name"));
Review comment:
@arina-ielchiieva the only message you get is
``` java.lang.ClassCastException:
com.ibm.lang.management.ExtendedOperatingSystem incompatible with
com.sun.management.OperatingSystemMXBean```
Not very useful, but you still get the simplified message without stack
trace. There isn't much value in adding more information than what is already
there, because the issue only emerged when JDBC drivers were being built using
the IBM JDK, something that a regular user will not face. (my testing showed
that an OracleJDK compiled class ran without issue for an IBM JDK Runtime).
I'll apply Vlad's suggestion & squash the commits.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] kkhatua commented on a change in pull request #1437: DRILL-6702: Disable CPU Reporting for non-HotSpot JDKs
kkhatua commented on a change in pull request #1437: DRILL-6702: Disable CPU
Reporting for non-HotSpot JDKs
URL: https://github.com/apache/drill/pull/1437#discussion_r213193819
##
File path: common/src/main/java/org/apache/drill/exec/metrics/CpuGaugeSet.java
##
@@ -32,13 +32,23 @@
*/
@SuppressWarnings("restriction")
public class CpuGaugeSet implements MetricSet {
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(CpuGaugeSet.class);
- private OperatingSystemMXBean osMXBean;
- private RuntimeMXBean rtMXBean;
+ private final OperatingSystemMXBean osMXBean;
+ private final RuntimeMXBean rtMXBean;
public CpuGaugeSet() {
-this.osMXBean = (OperatingSystemMXBean)
ManagementFactory.getOperatingSystemMXBean();
this.rtMXBean = ManagementFactory.getRuntimeMXBean();
+//DRILL-6702: Instead of worrying about compiling with IBM JDK, for now,
we shall provide no CPU metrics for non-HotSpot JVMs
+OperatingSystemMXBean tempOSMXBean = null;
+try {
+ tempOSMXBean = (OperatingSystemMXBean)
ManagementFactory.getOperatingSystemMXBean();
+} catch (ClassCastException cce) {
+ logger.warn("Detected non-HotSpot JVM [{}]. CPU Metrics in the WebUI
will not be available!", System.getProperty("java.vm.name"));
+ tempOSMXBean = null;
+}
+this.osMXBean = tempOSMXBean;
+System.out.println("osMXBean::" + osMXBean);
Review comment:
Agreed. The fix has been verified and I've done the clean up.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] kkhatua commented on a change in pull request #1437: DRILL-6702: Disable CPU Reporting for non-HotSpot JDKs
kkhatua commented on a change in pull request #1437: DRILL-6702: Disable CPU
Reporting for non-HotSpot JDKs
URL: https://github.com/apache/drill/pull/1437#discussion_r212839403
##
File path: common/src/main/java/org/apache/drill/exec/metrics/CpuGaugeSet.java
##
@@ -32,13 +32,23 @@
*/
@SuppressWarnings("restriction")
public class CpuGaugeSet implements MetricSet {
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(CpuGaugeSet.class);
- private OperatingSystemMXBean osMXBean;
- private RuntimeMXBean rtMXBean;
+ private final OperatingSystemMXBean osMXBean;
+ private final RuntimeMXBean rtMXBean;
public CpuGaugeSet() {
-this.osMXBean = (OperatingSystemMXBean)
ManagementFactory.getOperatingSystemMXBean();
this.rtMXBean = ManagementFactory.getRuntimeMXBean();
+//DRILL-6702: Instead of worrying about compiling with IBM JDK, for now,
we shall provide no CPU metrics for non-HotSpot JVMs
+OperatingSystemMXBean tempOSMXBean = null;
+try {
+ tempOSMXBean = (OperatingSystemMXBean)
ManagementFactory.getOperatingSystemMXBean();
+} catch (ClassCastException cce) {
+ logger.warn("Detected non-HotSpot JVM [{}]. CPU Metrics in the WebUI
will not be available!", System.getProperty("java.vm.name"));
Review comment:
Ok.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] kkhatua commented on a change in pull request #1437: DRILL-6702: Disable CPU Reporting for non-HotSpot JDKs
kkhatua commented on a change in pull request #1437: DRILL-6702: Disable CPU
Reporting for non-HotSpot JDKs
URL: https://github.com/apache/drill/pull/1437#discussion_r212839399
##
File path: common/src/main/java/org/apache/drill/exec/metrics/CpuGaugeSet.java
##
@@ -32,13 +32,23 @@
*/
@SuppressWarnings("restriction")
public class CpuGaugeSet implements MetricSet {
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(CpuGaugeSet.class);
- private OperatingSystemMXBean osMXBean;
- private RuntimeMXBean rtMXBean;
+ private final OperatingSystemMXBean osMXBean;
+ private final RuntimeMXBean rtMXBean;
public CpuGaugeSet() {
-this.osMXBean = (OperatingSystemMXBean)
ManagementFactory.getOperatingSystemMXBean();
this.rtMXBean = ManagementFactory.getRuntimeMXBean();
+//DRILL-6702: Instead of worrying about compiling with IBM JDK, for now,
we shall provide no CPU metrics for non-HotSpot JVMs
+OperatingSystemMXBean tempOSMXBean = null;
+try {
+ tempOSMXBean = (OperatingSystemMXBean)
ManagementFactory.getOperatingSystemMXBean();
+} catch (ClassCastException cce) {
+ logger.warn("Detected non-HotSpot JVM [{}]. CPU Metrics in the WebUI
will not be available!", System.getProperty("java.vm.name"));
+ tempOSMXBean = null;
+}
+this.osMXBean = tempOSMXBean;
+System.out.println("osMXBean::" + osMXBean);
Review comment:
Yes. Waiting for verification from the original JIRA poster if it fixes the
issue, and if not, they'll share this output.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] kkhatua commented on a change in pull request #1437: DRILL-6702: Disable CPU Reporting for non-HotSpot JDKs
kkhatua commented on a change in pull request #1437: DRILL-6702: Disable CPU
Reporting for non-HotSpot JDKs
URL: https://github.com/apache/drill/pull/1437#discussion_r212789862
##
File path: common/src/main/java/org/apache/drill/exec/metrics/CpuGaugeSet.java
##
@@ -32,13 +32,20 @@
*/
@SuppressWarnings("restriction")
public class CpuGaugeSet implements MetricSet {
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(CpuGaugeSet.class);
- private OperatingSystemMXBean osMXBean;
- private RuntimeMXBean rtMXBean;
+ private final OperatingSystemMXBean osMXBean;
+ private final RuntimeMXBean rtMXBean;
public CpuGaugeSet() {
-this.osMXBean = (OperatingSystemMXBean)
ManagementFactory.getOperatingSystemMXBean();
this.rtMXBean = ManagementFactory.getRuntimeMXBean();
+//DRILL-6702: Instead of worrying about compiling with IBM JDK, for now,
we shall provide no CPU metrics for non-HotSpot JVMs
+if (System.getProperty("java.vm.name").contains("HotSpot")) {
Review comment:
Ok. I'm not able to repro the ClassCastException with the IBM JDK running an
Oracle compilation.
The IBM JDK on my Linux box is using
`com.ibm.lang.management.internal.UnixExtendedOperatingSystem` which implements
the `com.sun.management.OperatingSystemMXBean` interface. The JIRA reports a
class cast exception from `com.ibm.lang.management.ExtendedOperatingSystem`.
So, it might be an issue with the platform's implementation.
I'll make the change however, and see if the original JIRA filer can confirm
the fix.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] kkhatua commented on a change in pull request #1437: DRILL-6702: Disable CPU Reporting for non-HotSpot JDKs
kkhatua commented on a change in pull request #1437: DRILL-6702: Disable CPU
Reporting for non-HotSpot JDKs
URL: https://github.com/apache/drill/pull/1437#discussion_r212525602
##
File path: common/src/main/java/org/apache/drill/exec/metrics/CpuGaugeSet.java
##
@@ -32,13 +32,20 @@
*/
@SuppressWarnings("restriction")
public class CpuGaugeSet implements MetricSet {
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(CpuGaugeSet.class);
- private OperatingSystemMXBean osMXBean;
- private RuntimeMXBean rtMXBean;
+ private final OperatingSystemMXBean osMXBean;
+ private final RuntimeMXBean rtMXBean;
public CpuGaugeSet() {
-this.osMXBean = (OperatingSystemMXBean)
ManagementFactory.getOperatingSystemMXBean();
this.rtMXBean = ManagementFactory.getRuntimeMXBean();
+//DRILL-6702: Instead of worrying about compiling with IBM JDK, for now,
we shall provide no CPU metrics for non-HotSpot JVMs
+if (System.getProperty("java.vm.name").contains("HotSpot")) {
Review comment:
I like the `try/catch` approach. The instanceOf actually passed for the IBM
JDK and failed in the cast. The `java.vm.name` appears to be standard, so I
doubt it would be missing. Other JDKs might not implement it because it is a
`com.sun. ...` package. (Remembering in context of Oracle vs Google, over
Android)
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] kkhatua commented on a change in pull request #1437: DRILL-6702: Disable CPU Reporting for non-HotSpot JDKs
kkhatua commented on a change in pull request #1437: DRILL-6702: Disable CPU
Reporting for non-HotSpot JDKs
URL: https://github.com/apache/drill/pull/1437#discussion_r212038433
##
File path: common/src/main/java/org/apache/drill/exec/metrics/CpuGaugeSet.java
##
@@ -32,13 +32,24 @@
*/
@SuppressWarnings("restriction")
public class CpuGaugeSet implements MetricSet {
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(CpuGaugeSet.class);
private OperatingSystemMXBean osMXBean;
private RuntimeMXBean rtMXBean;
+ private static String nonHotSpotJVMName;
public CpuGaugeSet() {
-this.osMXBean = (OperatingSystemMXBean)
ManagementFactory.getOperatingSystemMXBean();
+//DRILL-6702: Instead of worrying about compiling with IBM JDK, for now,
we shall provide no CPU metrics for non-HotSpot JVMs
+if (System.getProperty("java.vm.name").contains("HotSpot")) {
+ this.osMXBean = (OperatingSystemMXBean)
ManagementFactory.getOperatingSystemMXBean();
+} else {
+ this.osMXBean = null;
+ nonHotSpotJVMName = System.getProperty("java.vm.name");
+}
this.rtMXBean = ManagementFactory.getRuntimeMXBean();
+if (nonHotSpotJVMName != null) {
+ logger.warn("Detected non-HotSpot JVM [{}]. CPU Metrics in the WebUI
will not be available!", nonHotSpotJVMName);
+}
Review comment:
@arina-ielchiieva
Strangely enough, the instance check worked for the IBM JDK as well.
However, if I were to extract the object's classname (
`operatingSystemMXBean.getClass()` ) , I get the following:
Oracle JDK: `sun.management.OperatingSystemImpl`
IBM JDK: `com.ibm.lang.management.internal.UnixExtendedOperatingSystem`
I can do a check for the former, instead of the HotSpot name (though, I
prefer the latter).
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
