[GitHub] LantaoJin commented on a change in pull request #22874: [SPARK-25865][CORE] Add GC information to ExecutorMetrics
LantaoJin commented on a change in pull request #22874: [SPARK-25865][CORE] Add
GC information to ExecutorMetrics
URL: https://github.com/apache/spark/pull/22874#discussion_r259741437
##
File path:
core/src/main/scala/org/apache/spark/metrics/ExecutorMetricType.scala
##
@@ -99,6 +102,62 @@ case object ProcessTreeMetrics extends ExecutorMetricType {
}
}
+case object GarbageCollectionMetrics extends ExecutorMetricType with Logging {
+ private var nonBuiltInCollectors: Seq[String] = Nil
+
+ override val names = Seq(
+"MinorGCCount",
+"MinorGCTime",
+"MajorGCCount",
+"MajorGCTime"
+ )
+
+ /* We builtin some common GC collectors which categorized as young
generation and old */
+ private[spark] val YOUNG_GENERATION_BUILTIN_GARBAGE_COLLECTORS = Seq(
+"Copy",
+"PS Scavenge",
+"ParNew",
+"G1 Young Generation"
+ )
+
+ private[spark] val OLD_GENERATION_BUILTIN_GARBAGE_COLLECTORS = Seq(
+"MarkSweepCompact",
+"PS MarkSweep",
+"ConcurrentMarkSweep",
+"G1 Old Generation"
+ )
+
+ private lazy val youngGenerationGarbageCollector: Seq[String] = {
+
SparkEnv.get.conf.get(config.EVENT_LOG_GC_METRICS_YOUNG_GENERATION_GARBAGE_COLLECTORS)
+ }
+
+ private lazy val oldGenerationGarbageCollector: Seq[String] = {
+
SparkEnv.get.conf.get(config.EVENT_LOG_GC_METRICS_OLD_GENERATION_GARBAGE_COLLECTORS)
+ }
+
+ override private[spark] def getMetricValues(memoryManager: MemoryManager):
Array[Long] = {
+val gcMetrics = new Array[Long](names.length) // minorCount, minorTime,
majorCount, majorTime
+ManagementFactory.getGarbageCollectorMXBeans.asScala.foreach { mxBean =>
+ if (youngGenerationGarbageCollector.contains(mxBean.getName)) {
+gcMetrics(0) = mxBean.getCollectionCount
+gcMetrics(1) = mxBean.getCollectionTime
+ } else if (oldGenerationGarbageCollector.contains(mxBean.getName)) {
+gcMetrics(2) = mxBean.getCollectionCount
+gcMetrics(3) = mxBean.getCollectionTime
+ } else if (!nonBuiltInCollectors.contains(mxBean.getName)) {
+nonBuiltInCollectors = mxBean.getName +: nonBuiltInCollectors
+// log it when first seen
+logWarning(s"Add the non-built-in garbage collector(s)
$nonBuiltInCollectors " +
Review comment:
@kiszk Yes, I've modified it now.
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
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] LantaoJin commented on a change in pull request #22874: [SPARK-25865][CORE] Add GC information to ExecutorMetrics
LantaoJin commented on a change in pull request #22874: [SPARK-25865][CORE] Add
GC information to ExecutorMetrics
URL: https://github.com/apache/spark/pull/22874#discussion_r258762490
##
File path:
core/src/main/scala/org/apache/spark/metrics/ExecutorMetricType.scala
##
@@ -99,6 +102,56 @@ case object ProcessTreeMetrics extends ExecutorMetricType {
}
}
+case object GarbageCollectionMetrics extends ExecutorMetricType with Logging {
+ override val names = Seq(
+"MinorGCCount",
+"MinorGCTime",
+"MajorGCCount",
+"MajorGCTime"
+ )
+
+ /* We builtin some common GC collectors which categorized as young
generation and old */
+ private[spark] val YOUNG_GENERATION_BUILTIN_GARBAGE_COLLECTORS = Seq(
+"Copy",
+"PS Scavenge",
+"ParNew",
+"G1 Young Generation"
+ )
+
+ private[spark] val OLD_GENERATION_BUILTIN_GARBAGE_COLLECTORS = Seq(
+"MarkSweepCompact",
+"PS MarkSweep",
+"ConcurrentMarkSweep",
+"G1 Old Generation"
+ )
+
+ private lazy val youngGenerationGarbageCollector: Seq[String] = {
+
SparkEnv.get.conf.get(config.EVENT_LOG_GC_METRICS_YOUNG_GENERATION_GARBAGE_COLLECTORS)
+ }
+
+ private lazy val oldGenerationGarbageCollector: Seq[String] = {
+
SparkEnv.get.conf.get(config.EVENT_LOG_GC_METRICS_OLD_GENERATION_GARBAGE_COLLECTORS)
+ }
+
+ override private[spark] def getMetricValues(memoryManager: MemoryManager):
Array[Long] = {
+val gcMetrics = new Array[Long](names.length) // minorCount, minorTime,
majorCount, majorTime
+ ManagementFactory.getGarbageCollectorMXBeans.asScala.foreach { mxBean =>
Review comment:
fixed
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
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] LantaoJin commented on a change in pull request #22874: [SPARK-25865][CORE] Add GC information to ExecutorMetrics
LantaoJin commented on a change in pull request #22874: [SPARK-25865][CORE] Add
GC information to ExecutorMetrics
URL: https://github.com/apache/spark/pull/22874#discussion_r258762360
##
File path:
core/src/main/scala/org/apache/spark/metrics/ExecutorMetricType.scala
##
@@ -99,6 +102,56 @@ case object ProcessTreeMetrics extends ExecutorMetricType {
}
}
+case object GarbageCollectionMetrics extends ExecutorMetricType with Logging {
+ override val names = Seq(
+"MinorGCCount",
+"MinorGCTime",
+"MajorGCCount",
+"MajorGCTime"
+ )
+
+ /* We builtin some common GC collectors which categorized as young
generation and old */
+ private[spark] val YOUNG_GENERATION_BUILTIN_GARBAGE_COLLECTORS = Seq(
+"Copy",
+"PS Scavenge",
+"ParNew",
+"G1 Young Generation"
+ )
+
+ private[spark] val OLD_GENERATION_BUILTIN_GARBAGE_COLLECTORS = Seq(
+"MarkSweepCompact",
+"PS MarkSweep",
+"ConcurrentMarkSweep",
+"G1 Old Generation"
+ )
+
+ private lazy val youngGenerationGarbageCollector: Seq[String] = {
+
SparkEnv.get.conf.get(config.EVENT_LOG_GC_METRICS_YOUNG_GENERATION_GARBAGE_COLLECTORS)
+ }
+
+ private lazy val oldGenerationGarbageCollector: Seq[String] = {
+
SparkEnv.get.conf.get(config.EVENT_LOG_GC_METRICS_OLD_GENERATION_GARBAGE_COLLECTORS)
+ }
+
+ override private[spark] def getMetricValues(memoryManager: MemoryManager):
Array[Long] = {
+val gcMetrics = new Array[Long](names.length) // minorCount, minorTime,
majorCount, majorTime
+ ManagementFactory.getGarbageCollectorMXBeans.asScala.foreach { mxBean =>
+ if (youngGenerationGarbageCollector.contains(mxBean.getName)) {
+gcMetrics(0) = mxBean.getCollectionCount
+gcMetrics(1) = mxBean.getCollectionTime
+ } else if (oldGenerationGarbageCollector.contains(mxBean.getName)) {
+gcMetrics(2) = mxBean.getCollectionCount
+gcMetrics(3) = mxBean.getCollectionTime
+ } else {
+logDebug(s"${mxBean.getName} is an unsupported garbage collector." +
Review comment:
@squito thanks for this useful comment, I've updated.
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
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] LantaoJin commented on a change in pull request #22874: [SPARK-25865][CORE] Add GC information to ExecutorMetrics
LantaoJin commented on a change in pull request #22874: [SPARK-25865][CORE] Add
GC information to ExecutorMetrics
URL: https://github.com/apache/spark/pull/22874#discussion_r250029543
##
File path:
core/src/main/scala/org/apache/spark/metrics/ExecutorMetricType.scala
##
@@ -135,19 +135,17 @@ case object GarbageCollectionMetrics extends
ExecutorMetricType with Logging {
override private[spark] def getMetricValues(memoryManager: MemoryManager):
Array[Long] = {
val gcMetrics = new Array[Long](names.length) // minorCount, minorTime,
majorCount, majorTime
-if (SparkEnv.get.conf.get(config.EVENT_LOG_GC_METRICS)) {
ManagementFactory.getGarbageCollectorMXBeans.asScala.foreach { mxBean =>
-if (youngGenerationGarbageCollector.contains(mxBean.getName)) {
- gcMetrics(0) = mxBean.getCollectionCount
- gcMetrics(1) = mxBean.getCollectionTime
-} else if (oldGenerationGarbageCollector.contains(mxBean.getName)) {
- gcMetrics(2) = mxBean.getCollectionCount
- gcMetrics(3) = mxBean.getCollectionTime
-} else {
- logDebug(s"${mxBean.getName} is an unsupported garbage collector." +
-s"Add it to
${config.EVENT_LOG_GC_METRICS_YOUNG_GENERATION_GARBAGE_COLLECTORS} " +
-s"or
${config.EVENT_LOG_GC_METRICS_OLD_GENERATION_GARBAGE_COLLECTORS} to enable.")
-}
+ if (youngGenerationGarbageCollector.contains(mxBean.getName)) {
+gcMetrics(0) = mxBean.getCollectionCount
Review comment:
AFAIK, one collector per generation is supported only.
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
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] LantaoJin commented on a change in pull request #22874: [SPARK-25865][CORE] Add GC information to ExecutorMetrics
LantaoJin commented on a change in pull request #22874: [SPARK-25865][CORE] Add
GC information to ExecutorMetrics
URL: https://github.com/apache/spark/pull/22874#discussion_r249644658
##
File path:
core/src/main/scala/org/apache/spark/metrics/ExecutorMetricType.scala
##
@@ -99,6 +102,56 @@ case object ProcessTreeMetrics extends ExecutorMetricType {
}
}
+case object GarbageCollectionMetrics extends ExecutorMetricType with Logging {
+ override val names = Seq(
+"MinorGCCount",
+"MinorGCTime",
+"MajorGCCount",
+"MajorGCTime"
+ )
+
+ /* We builtin some common GC collectors which categorized as young
generation and old */
+ private[spark] val YOUNG_GENERATION_BUILTIN_GARBAGE_COLLECTORS = Seq(
+"Copy",
+"PS Scavenge",
+"ParNew",
+"G1 Young Generation"
+ )
+
+ private[spark] val OLD_GENERATION_BUILTIN_GARBAGE_COLLECTORS = Seq(
+"MarkSweepCompact",
+"PS MarkSweep",
+"ConcurrentMarkSweep",
+"G1 Old Generation"
+ )
+
+ private lazy val youngGenerationGarbageCollector: Seq[String] = {
+
SparkEnv.get.conf.get(config.EVENT_LOG_GC_METRICS_YOUNG_GENERATION_GARBAGE_COLLECTORS)
+ }
+
+ private lazy val oldGenerationGarbageCollector: Seq[String] = {
+
SparkEnv.get.conf.get(config.EVENT_LOG_GC_METRICS_OLD_GENERATION_GARBAGE_COLLECTORS)
+ }
+
+ override private[spark] def getMetricValues(memoryManager: MemoryManager):
Array[Long] = {
+val gcMetrics = new Array[Long](names.length) // minorCount, minorTime,
majorCount, majorTime
+ ManagementFactory.getGarbageCollectorMXBeans.asScala.foreach { mxBean =>
+ if (youngGenerationGarbageCollector.contains(mxBean.getName)) {
+gcMetrics(0) = mxBean.getCollectionCount
+gcMetrics(1) = mxBean.getCollectionTime
+ } else if (oldGenerationGarbageCollector.contains(mxBean.getName)) {
+gcMetrics(2) = mxBean.getCollectionCount
+gcMetrics(3) = mxBean.getCollectionTime
+ } else {
+logDebug(s"${mxBean.getName} is an unsupported garbage collector." +
Review comment:
@kiszk This message is shown only in DEBUG level. In production, I think no
performance degradation since the log level is INFO by default. User should
know DEBUG level always impact performance more or less. And this isn't a high
frequency calling due to heartbeat intervals.
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
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] LantaoJin commented on a change in pull request #22874: [SPARK-25865][CORE] Add GC information to ExecutorMetrics
LantaoJin commented on a change in pull request #22874: [SPARK-25865][CORE] Add
GC information to ExecutorMetrics
URL: https://github.com/apache/spark/pull/22874#discussion_r249267528
##
File path:
core/src/main/scala/org/apache/spark/metrics/ExecutorMetricType.scala
##
@@ -99,6 +102,58 @@ case object ProcessTreeMetrics extends ExecutorMetricType {
}
}
+case object GarbageCollectionMetrics extends ExecutorMetricType with Logging {
+ override val names = Seq(
+"MinorGCCount",
+"MinorGCTime",
+"MajorGCCount",
+"MajorGCTime"
+ )
+
+ /* We builtin some common GC collectors which categorized as young
generation and old */
+ private[spark] val YOUNG_GENERATION_BUILTIN_GARBAGE_COLLECTORS = Seq(
+"Copy",
+"PS Scavenge",
+"ParNew",
+"G1 Young Generation"
+ )
+
+ private[spark] val OLD_GENERATION_BUILTIN_GARBAGE_COLLECTORS = Seq(
+"MarkSweepCompact",
+"PS MarkSweep",
+"ConcurrentMarkSweep",
+"G1 Old Generation"
+ )
+
+ private lazy val youngGenerationGarbageCollector: Seq[String] = {
+
SparkEnv.get.conf.get(config.EVENT_LOG_GC_METRICS_YOUNG_GENERATION_GARBAGE_COLLECTORS)
+ }
+
+ private lazy val oldGenerationGarbageCollector: Seq[String] = {
+
SparkEnv.get.conf.get(config.EVENT_LOG_GC_METRICS_OLD_GENERATION_GARBAGE_COLLECTORS)
+ }
+
+ override private[spark] def getMetricValues(memoryManager: MemoryManager):
Array[Long] = {
+val gcMetrics = new Array[Long](names.length) // minorCount, minorTime,
majorCount, majorTime
+if (SparkEnv.get.conf.get(config.EVENT_LOG_GC_METRICS)) {
Review comment:
Yes. I just keep the same structure like ProcfsMetrics. I will remove it
since it still returns 0 when its off.
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
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] LantaoJin commented on a change in pull request #22874: [SPARK-25865][CORE] Add GC information to ExecutorMetrics
LantaoJin commented on a change in pull request #22874: [SPARK-25865][CORE] Add
GC information to ExecutorMetrics
URL: https://github.com/apache/spark/pull/22874#discussion_r247058061
##
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##
@@ -98,6 +98,27 @@ package object config {
.booleanConf
.createWithDefault(false)
+ private[spark] val EVENT_LOG_GARBAGE_COLLECTION_METRICS =
+ConfigBuilder("spark.eventLog.logStageExecutorGCMetrics.enabled")
+ .booleanConf
+ .createWithDefault(false)
+
+ private[spark] val EVENT_LOG_ADDITIONAL_YOUNG_GENERATION_GARBAGE_COLLECTORS =
+ConfigBuilder("spark.eventLog.additionalYoungGenerationGarbageCollectors")
+ .doc("Names of additional young generation garbage collector, " +
+"usually is the return of GarbageCollectorMXBean.getName, e.g.
ParNew.")
Review comment:
Correct. I improved.
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
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] LantaoJin commented on a change in pull request #22874: [SPARK-25865][CORE] Add GC information to ExecutorMetrics
LantaoJin commented on a change in pull request #22874: [SPARK-25865][CORE] Add
GC information to ExecutorMetrics
URL: https://github.com/apache/spark/pull/22874#discussion_r245867274
##
File path:
core/src/main/scala/org/apache/spark/metrics/ExecutorMetricType.scala
##
@@ -99,6 +102,60 @@ case object ProcessTreeMetrics extends ExecutorMetricType {
}
}
+case object GarbageCollectionMetrics extends ExecutorMetricType with Logging {
+ import GC_TYPE._
+ override val names = Seq(
+"MinorGCCount",
+"MinorGCTime",
+"MajorGCCount",
+"MajorGCTime"
+ )
+
+ private lazy val youngGenGarbageCollector: Seq[String] = {
+Seq(`copy`, `psScavenge`, `parNew`, `g1Young`) ++ /* additional young gc
we added */
+
SparkEnv.get.conf.get(config.EVENT_LOG_ADDITIONAL_YOUNG_GENERATION_GARBAGE_COLLECTORS)
+ }
+
+ private lazy val oldGenGarbageCollector: Seq[String] = {
+Seq(`markSweepCompact`, `psMarkSweep`, `cms`, `g1Old`) ++ /* additional
old gc we added */
+
SparkEnv.get.conf.get(config.EVENT_LOG_ADDITIONAL_OLD_GENERATION_GARBAGE_COLLECTORS)
+ }
+
+ override private[spark] def getMetricValues(memoryManager: MemoryManager):
Array[Long] = {
+val gcMetrics = Array[Long](names.length) // minorCount, minorTime,
majorCount, majorTime
+if (SparkEnv.get.conf.get(config.EVENT_LOG_GARBAGE_COLLECTION_METRICS)) {
+ ManagementFactory.getGarbageCollectorMXBeans.asScala.foreach { mxBean =>
+if (youngGenGarbageCollector.contains(mxBean.getName)) {
+ gcMetrics(0) = mxBean.getCollectionCount
+ gcMetrics(1) = mxBean.getCollectionTime
+} else if (oldGenGarbageCollector.contains(mxBean.getName)) {
+ gcMetrics(2) = mxBean.getCollectionCount
+ gcMetrics(3) = mxBean.getCollectionTime
+} else {
+ logDebug(s"${mxBean.getName} is an unsupported garbage collector." +
+s"Add it to
${config.EVENT_LOG_ADDITIONAL_YOUNG_GENERATION_GARBAGE_COLLECTORS} " +
+s"or
${config.EVENT_LOG_ADDITIONAL_OLD_GENERATION_GARBAGE_COLLECTORS} to enable.")
+}
+ }
+}
+gcMetrics
+ }
+}
+
+private[spark] object GC_TYPE {
+ // Young Gen
+ val copy = "Copy"
+ val psScavenge = "PS Scavenge"
+ val parNew = "ParNew"
+ val g1Young = "G1 Young Generation"
+
+ // Old Gen
+ val markSweepCompact = "MarkSweepCompact"
+ val psMarkSweep = "PS MarkSweep"
+ val cms = "ConcurrentMarkSweep"
+ val g1Old = "G1 Old Generation"
Review comment:
Yes, I will use two Seqs instead.
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
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] LantaoJin commented on a change in pull request #22874: [SPARK-25865][CORE] Add GC information to ExecutorMetrics
LantaoJin commented on a change in pull request #22874: [SPARK-25865][CORE] Add
GC information to ExecutorMetrics
URL: https://github.com/apache/spark/pull/22874#discussion_r245867274
##
File path:
core/src/main/scala/org/apache/spark/metrics/ExecutorMetricType.scala
##
@@ -99,6 +102,60 @@ case object ProcessTreeMetrics extends ExecutorMetricType {
}
}
+case object GarbageCollectionMetrics extends ExecutorMetricType with Logging {
+ import GC_TYPE._
+ override val names = Seq(
+"MinorGCCount",
+"MinorGCTime",
+"MajorGCCount",
+"MajorGCTime"
+ )
+
+ private lazy val youngGenGarbageCollector: Seq[String] = {
+Seq(`copy`, `psScavenge`, `parNew`, `g1Young`) ++ /* additional young gc
we added */
+
SparkEnv.get.conf.get(config.EVENT_LOG_ADDITIONAL_YOUNG_GENERATION_GARBAGE_COLLECTORS)
+ }
+
+ private lazy val oldGenGarbageCollector: Seq[String] = {
+Seq(`markSweepCompact`, `psMarkSweep`, `cms`, `g1Old`) ++ /* additional
old gc we added */
+
SparkEnv.get.conf.get(config.EVENT_LOG_ADDITIONAL_OLD_GENERATION_GARBAGE_COLLECTORS)
+ }
+
+ override private[spark] def getMetricValues(memoryManager: MemoryManager):
Array[Long] = {
+val gcMetrics = Array[Long](names.length) // minorCount, minorTime,
majorCount, majorTime
+if (SparkEnv.get.conf.get(config.EVENT_LOG_GARBAGE_COLLECTION_METRICS)) {
+ ManagementFactory.getGarbageCollectorMXBeans.asScala.foreach { mxBean =>
+if (youngGenGarbageCollector.contains(mxBean.getName)) {
+ gcMetrics(0) = mxBean.getCollectionCount
+ gcMetrics(1) = mxBean.getCollectionTime
+} else if (oldGenGarbageCollector.contains(mxBean.getName)) {
+ gcMetrics(2) = mxBean.getCollectionCount
+ gcMetrics(3) = mxBean.getCollectionTime
+} else {
+ logDebug(s"${mxBean.getName} is an unsupported garbage collector." +
+s"Add it to
${config.EVENT_LOG_ADDITIONAL_YOUNG_GENERATION_GARBAGE_COLLECTORS} " +
+s"or
${config.EVENT_LOG_ADDITIONAL_OLD_GENERATION_GARBAGE_COLLECTORS} to enable.")
+}
+ }
+}
+gcMetrics
+ }
+}
+
+private[spark] object GC_TYPE {
+ // Young Gen
+ val copy = "Copy"
+ val psScavenge = "PS Scavenge"
+ val parNew = "ParNew"
+ val g1Young = "G1 Young Generation"
+
+ // Old Gen
+ val markSweepCompact = "MarkSweepCompact"
+ val psMarkSweep = "PS MarkSweep"
+ val cms = "ConcurrentMarkSweep"
+ val g1Old = "G1 Old Generation"
Review comment:
Yes, I will use a Seq instead.
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
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] LantaoJin commented on a change in pull request #22874: [SPARK-25865][CORE] Add GC information to ExecutorMetrics
LantaoJin commented on a change in pull request #22874: [SPARK-25865][CORE] Add
GC information to ExecutorMetrics
URL: https://github.com/apache/spark/pull/22874#discussion_r245563512
##
File path:
core/src/main/scala/org/apache/spark/metrics/ExecutorMetricType.scala
##
@@ -99,6 +102,68 @@ case object ProcessTreeMetrics extends ExecutorMetricType {
}
}
+case object GarbageCollectionMetrics extends ExecutorMetricType with Logging {
+ import GC_TYPE._
+ override val names = Seq(
+"MinorGCCount",
+"MinorGCTime",
+"MajorGCCount",
+"MajorGCTime"
+ )
+
+ private lazy val supported: Boolean = {
+val shouldLogStageExecutorGCMetrics =
+ SparkEnv.get.conf.get(config.EVENT_LOG_PROCESS_TREE_METRICS)
+val supportedVendor = System.getProperty("java.vendor").contains("Oracle")
||
+ System.getProperty("java.vendor").contains("OpenJDK")
+shouldLogStageExecutorGCMetrics && supportedVendor
Review comment:
Thanks for your review. Also I disabled this feature by default.
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
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] LantaoJin commented on a change in pull request #22874: [SPARK-25865][CORE] Add GC information to ExecutorMetrics
LantaoJin commented on a change in pull request #22874: [SPARK-25865][CORE] Add
GC information to ExecutorMetrics
URL: https://github.com/apache/spark/pull/22874#discussion_r24366
##
File path:
core/src/main/scala/org/apache/spark/metrics/ExecutorMetricType.scala
##
@@ -99,6 +102,68 @@ case object ProcessTreeMetrics extends ExecutorMetricType {
}
}
+case object GarbageCollectionMetrics extends ExecutorMetricType with Logging {
+ import GC_TYPE._
+ override val names = Seq(
+"MinorGCCount",
+"MinorGCTime",
+"MajorGCCount",
+"MajorGCTime"
+ )
+
+ private lazy val supported: Boolean = {
+val shouldLogStageExecutorGCMetrics =
+ SparkEnv.get.conf.get(config.EVENT_LOG_PROCESS_TREE_METRICS)
+val supportedVendor = System.getProperty("java.vendor").contains("Oracle")
||
+ System.getProperty("java.vendor").contains("OpenJDK")
+shouldLogStageExecutorGCMetrics && supportedVendor
Review comment:
I thought it's version specific as well and I am only sure those venders are
safe. I will remove this since this API are standard in Java Lang.
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
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] LantaoJin commented on a change in pull request #22874: [SPARK-25865][CORE] Add GC information to ExecutorMetrics
LantaoJin commented on a change in pull request #22874: [SPARK-25865][CORE] Add
GC information to ExecutorMetrics
URL: https://github.com/apache/spark/pull/22874#discussion_r244741423
##
File path:
core/src/main/scala/org/apache/spark/metrics/ExecutorMetricType.scala
##
@@ -99,6 +100,61 @@ case object ProcessTreeMetrics extends ExecutorMetricType {
}
}
+case object GarbageCollectionMetrics extends ExecutorMetricType {
+ import GC_TYPE._
+ override val names = Seq(
+"MinorGCCount",
+"MinorGCTime",
+"MajorGCCount",
+"MajorGCTime"
+ )
+ override private[spark] def getMetricValues(memoryManager: MemoryManager):
Array[Long] = {
+var allMetrics = GCMetrics(0, 0, 0, 0)
+ManagementFactory.getGarbageCollectorMXBeans.asScala.foreach { mxBean =>
+ val metrics = mxBean.getName match {
+case `copy` | `psScavenge` | `parNew` | `g1Young` =>
Review comment:
@kiszk I added some configurations to support more garbage collectors.
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
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] LantaoJin commented on a change in pull request #22874: [SPARK-25865][CORE] Add GC information to ExecutorMetrics
LantaoJin commented on a change in pull request #22874: [SPARK-25865][CORE] Add
GC information to ExecutorMetrics
URL: https://github.com/apache/spark/pull/22874#discussion_r244698090
##
File path:
core/src/main/scala/org/apache/spark/metrics/ExecutorMetricType.scala
##
@@ -99,6 +100,61 @@ case object ProcessTreeMetrics extends ExecutorMetricType {
}
}
+case object GarbageCollectionMetrics extends ExecutorMetricType {
+ import GC_TYPE._
+ override val names = Seq(
+"MinorGCCount",
+"MinorGCTime",
+"MajorGCCount",
+"MajorGCTime"
+ )
+ override private[spark] def getMetricValues(memoryManager: MemoryManager):
Array[Long] = {
+var allMetrics = GCMetrics(0, 0, 0, 0)
+ManagementFactory.getGarbageCollectorMXBeans.asScala.foreach { mxBean =>
+ val metrics = mxBean.getName match {
+case `copy` | `psScavenge` | `parNew` | `g1Young` =>
Review comment:
I hope it could keep major/minor types for collecting the status of
different generations. I will check more for OpenJDK and how to config it.
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
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] LantaoJin commented on a change in pull request #22874: [SPARK-25865][CORE] Add GC information to ExecutorMetrics
LantaoJin commented on a change in pull request #22874: [SPARK-25865][CORE] Add
GC information to ExecutorMetrics
URL: https://github.com/apache/spark/pull/22874#discussion_r244698090
##
File path:
core/src/main/scala/org/apache/spark/metrics/ExecutorMetricType.scala
##
@@ -99,6 +100,61 @@ case object ProcessTreeMetrics extends ExecutorMetricType {
}
}
+case object GarbageCollectionMetrics extends ExecutorMetricType {
+ import GC_TYPE._
+ override val names = Seq(
+"MinorGCCount",
+"MinorGCTime",
+"MajorGCCount",
+"MajorGCTime"
+ )
+ override private[spark] def getMetricValues(memoryManager: MemoryManager):
Array[Long] = {
+var allMetrics = GCMetrics(0, 0, 0, 0)
+ManagementFactory.getGarbageCollectorMXBeans.asScala.foreach { mxBean =>
+ val metrics = mxBean.getName match {
+case `copy` | `psScavenge` | `parNew` | `g1Young` =>
Review comment:
I will try to use (a) to refactor it. I hope it could keep major/minor types.
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
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] LantaoJin commented on a change in pull request #22874: [SPARK-25865][CORE] Add GC information to ExecutorMetrics
LantaoJin commented on a change in pull request #22874: [SPARK-25865][CORE] Add
GC information to ExecutorMetrics
URL: https://github.com/apache/spark/pull/22874#discussion_r244697492
##
File path:
core/src/main/scala/org/apache/spark/metrics/ExecutorMetricType.scala
##
@@ -99,6 +100,61 @@ case object ProcessTreeMetrics extends ExecutorMetricType {
}
}
+case object GarbageCollectionMetrics extends ExecutorMetricType {
+ import GC_TYPE._
+ override val names = Seq(
+"MinorGCCount",
+"MinorGCTime",
+"MajorGCCount",
+"MajorGCTime"
+ )
+ override private[spark] def getMetricValues(memoryManager: MemoryManager):
Array[Long] = {
+var allMetrics = GCMetrics(0, 0, 0, 0)
Review comment:
Agree
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
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] LantaoJin commented on a change in pull request #22874: [SPARK-25865][CORE] Add GC information to ExecutorMetrics
LantaoJin commented on a change in pull request #22874: [SPARK-25865][CORE] Add
GC information to ExecutorMetrics
URL: https://github.com/apache/spark/pull/22874#discussion_r244690342
##
File path: core/src/test/scala/org/apache/spark/util/JsonProtocolSuite.scala
##
@@ -130,7 +130,7 @@ class JsonProtocolSuite extends SparkFunSuite {
testEvent(executorUnblacklisted, executorUnblacklistedJsonString)
testEvent(nodeBlacklisted, nodeBlacklistedJsonString)
testEvent(nodeUnblacklisted, nodeUnblacklistedJsonString)
-testEvent(executorMetricsUpdate, executorMetricsUpdateJsonString)
+//testEvent(executorMetricsUpdate, executorMetricsUpdateJsonString)
Review comment:
My typo. Updated
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
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
