-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-25228][CORE]Add executor CPU time metric. #22218
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
c715096
438bf90
807119b
d522fa2
b7fdec2
95d31f6
e72966e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,11 +17,13 @@ | |
|
|
||
| package org.apache.spark.executor | ||
|
|
||
| import java.lang.management.ManagementFactory | ||
| import java.util.concurrent.ThreadPoolExecutor | ||
|
|
||
| import scala.collection.JavaConverters._ | ||
|
|
||
| import com.codahale.metrics.{Gauge, MetricRegistry} | ||
| import com.sun.management.OperatingSystemMXBean | ||
| import org.apache.hadoop.fs.FileSystem | ||
|
|
||
| import org.apache.spark.metrics.source.Source | ||
|
|
@@ -73,6 +75,13 @@ class ExecutorSource(threadPool: ThreadPoolExecutor, executorId: String) extends | |
| registerFileSystemStat(scheme, "write_ops", _.getWriteOps(), 0) | ||
| } | ||
|
|
||
| // Dropwizard metrics gauge measuring the executor's process (JVM) CPU time. | ||
| // The value is returned in nanoseconds, the method return -1 if this operation is not supported. | ||
| val osMXBean = ManagementFactory.getOperatingSystemMXBean.asInstanceOf[OperatingSystemMXBean] | ||
| metricRegistry.register(MetricRegistry.name("executorCPUTime" ), new Gauge[Long] { | ||
|
||
| override def getValue: Long = osMXBean.getProcessCpuTime() | ||
|
||
| }) | ||
|
|
||
| // Expose executor task metrics using the Dropwizard metrics system. | ||
| // The list is taken from TaskMetrics.scala | ||
| val METRIC_CPU_TIME = metricRegistry.counter(MetricRegistry.name("cpuTime")) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this com.sun class going to be available in all JDKs? Thinking of OpenJDK and IBM JDKs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point.
This class cannot be loaded at least on IBM JDK as reported here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed this is a very good point that I had overlooked. I have now directly checked and this appears to work OK on OpenJDK (and on Oracle JVM of course). In addition, I tested manually with IBM JDK (IBM J9 VM, Java 1.8.0_181, where one would indeed suspect incompatibilities and surprisingly this appears to work in that case too. I believe this may come from recent work by IBM to make
com.ibm.lang.management.OperatingSystemMXBean.getProcessCpuTimecompatible withcom.sun.management.OperatingSystemMXBean.getProcessCpuTime? See also this linkI guess that if this is confirmed, we should be fine with a large fraction of the commonly used JDKs. In addition, we could handle the exception in case getProcessCpuTime is not available on a particular platform where the executor is running, for example returning the value -1 for this gauge in that case. Any thoughts/suggestions on this proposal?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's safest to a little reflection here to make sure this doesn't cause the whole app to crash every time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have refactored the code with a different approach using the BeanServer which should address the comments about avialability of com.sun.management.OperatingSystemMXBean across different JDKs.