Skip to content

[SPARK-25865][CORE] Add GC information to ExecutorMetrics #22874

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.

Sign up for GitHub

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

Closed
LantaoJin wants to merge 14 commits into from
Closed

[SPARK-25865][CORE] Add GC information to ExecutorMetrics #22874

Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
6ad48d6
[SPARK-25865][CORE] Add GC information to ExecutorMetrics
LantaoJin Dec 17, 2018
b8c3382
add UT
LantaoJin Jan 2, 2019
098a246
address comments
LantaoJin Jan 2, 2019
1dbc68a
address comments
LantaoJin Jan 2, 2019
94fd962
make configurable
LantaoJin Jan 2, 2019
e4d9acd
address comments
LantaoJin Jan 2, 2019
aa3cf3c
address comments
LantaoJin Jan 7, 2019
544749d
address comments
LantaoJin Jan 8, 2019
fe5a04c
address rat check
LantaoJin Jan 10, 2019
12fc5cc
address comments
LantaoJin Jan 11, 2019
6d5643c
remove the unclear gc metric configuration
LantaoJin Jan 20, 2019
4d1021c
fix ut
LantaoJin Feb 20, 2019
ba500fe
address the comments
LantaoJin Feb 21, 2019
958a728
make log clear
LantaoJin Feb 25, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
address the comments
  • Loading branch information
@LantaoJin
LantaoJin committed Feb 21, 2019
commit ba500fe816954ff9e82b4cb92215d8eaecc1dd4f
14 changes: 10 additions & 4 deletions core/src/main/scala/org/apache/spark/metrics/ExecutorMetricType.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ case object ProcessTreeMetrics extends ExecutorMetricType {
}

case object GarbageCollectionMetrics extends ExecutorMetricType with Logging {
private var nonBuiltInCollectors: Seq[String] = Nil

override val names = Seq(
"MinorGCCount",
"MinorGCTime",
Expand Down Expand Up @@ -135,17 +137,21 @@ 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
ManagementFactory.getGarbageCollectorMXBeans.asScala.foreach { mxBean =>
ManagementFactory.getGarbageCollectorMXBeans.asScala.foreach { mxBean =>
if (youngGenerationGarbageCollector.contains(mxBean.getName)) {
gcMetrics(0) = mxBean.getCollectionCount
Copy link
Contributor

@squito squito Jan 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it ever possible that multiple collectors are considered young generation (or old generation)? If so, you'd want to use += here.

I'm not sure if there is a configuration where that might happen, but I'm also not super familiar with all variants here. @kiszk maybe you know?

Copy link
Contributor Author

@LantaoJin LantaoJin Jan 23, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK, one collector per generation is supported only.

Copy link
Member

@kiszk kiszk Feb 18, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for being late. I agree with one collector per generation.

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.key} " +
} 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 " +
Copy link
Member

@kiszk kiszk Feb 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: to me, the subject of this sentence is unclear. At first, I interpreted as (Spark) add the ..... Can we make it clear?
For example, Please add ... or Users should add ....

Copy link
Contributor Author

@LantaoJin LantaoJin Feb 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kiszk Yes, I've modified it now.

s"to ${config.EVENT_LOG_GC_METRICS_YOUNG_GENERATION_GARBAGE_COLLECTORS.key} " +
s"or ${config.EVENT_LOG_GC_METRICS_OLD_GENERATION_GARBAGE_COLLECTORS.key} to enable.")
} else {
// do nothing
}
}
gcMetrics
Expand Down