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

LantaoJin · 2018-10-29T09:16:31Z

What changes were proposed in this pull request?

Only memory usage without GC information could not help us to determinate the proper settings of memory. We need the GC metrics about frequency of major & minor GC. For example, two cases, their configured memory for executor are all 10GB and their usages are all near 10GB. So should we increase or decrease the configured memory for them? This metrics may be helpful. We can increase configured memory for the first one if it has very frequency major GC and decrease the second one if only some minor GC and none major GC.
GC metrics are only useful in entire lifetime of executors instead of separated stages.

How was this patch tested?

Adding UT.

squito · 2018-12-11T20:43:21Z

@LantaoJin #22612 has been merged (and unfortunately seems like there are many merge conflicts now), would you like to update this?

LantaoJin · 2018-12-13T13:45:20Z

@squito Yes, I will update when I get a chance.

LantaoJin · 2018-12-17T10:07:54Z

Updated. cc @squito @rezasafi

rezasafi · 2018-12-17T16:01:08Z

@LantaoJin Thank you very much for the work. Is that possible for you to add an actual spark event log with these new GC metrics under the core/test/resources/spark-events and then update HistoryServerSuite and other places accordingly? (similar to what was done for SPARK-23429(application_1506645932520_24630151) and SPARK-24958(application_1538416563558_0014)). It would be nice to have that here as well.

core/src/main/scala/org/apache/spark/metrics/ExecutorMetricType.scala

squito

I'm not entirely sure these metrics make sense with ExecutorMetrics, or maybe it just needs extra handling. The memory metrics make sense as instaneous snapshots, but gc count & gc time are constantly increasing. They make sense over the entire lifetime of the executor, but not when viewed within one stage -- you'd want to subtract out the value at the beginning of the stage.

BTW spark already tracks total gc time as a task metrics -- of course that's a little misleading since gc is not assignable to just one task, but its at least available, and can already be aggregated for the whole executor. So we should think about what is the value add over that existing metric.

Sorry for only commenting about this now, I hadn't thought carefully about what these metrics mean before this.

core/src/main/scala/org/apache/spark/metrics/ExecutorMetricType.scala

core/src/test/scala/org/apache/spark/util/JsonProtocolSuite.scala

core/src/main/scala/org/apache/spark/metrics/ExecutorMetricType.scala

LantaoJin · 2019-01-02T04:10:11Z

They make sense over the entire lifetime of the executor, but not when viewed within one stage -- you'd want to subtract out the value at the beginning of the stage.

You are right. They only make sense over the entire lifetime. I don't want to separate this metrics to multi-stage. I will check the current implementation. One of purposes to add this is determining the frequency of major & minor GC. Only memory usage couldn't tell us the rationality of memory allocation. For example, two cases, their configured memory for executor are all 10GB and their usages are all near 10GB. So should we increase or decrease the configured memory for them? This metrics may be helpful. We can increase configured memory for the first one if it has very frequency major GC and decrease the second one if only some minor GC and none major GC.

LantaoJin · 2019-01-02T14:05:42Z

retest this please

core/src/main/scala/org/apache/spark/metrics/ExecutorMetricType.scala

squito

can you add a smaller event log file? you really just need one, maybe two executors with this update, right? This one is really too big to be checked in.

-rwxr-xr-x  1 irashid  staff    34M Jan  7 15:43 application_1536831636016_56998
-rw-r--r--  1 irashid  staff   336K Jan  3 14:52 application_1538416563558_0014
-rw-r--r--  1 irashid  staff   148K Jan  3 14:52 application_1506645932520_24630151
-rwxr-xr-x  1 irashid  staff   135K Oct 14  2016 local-1430917381534
-rwxr-xr-x  1 irashid  staff   126K Aug  6 12:13 app-20161115172038-0000
-rwxr-xr-x  1 irashid  staff   121K Aug  6 12:13 app-20161116163331-0000
-rwxr-xr-x  1 irashid  staff   120K Nov 28 16:47 application_1516285256255_0012
-rwxr-xr-x  1 irashid  staff   111K Nov 28 16:47 app-20180109111548-0000
-rwxr-xr-x  1 irashid  staff    67K Oct 26  2016 local-1422981780767
-rwxr-xr-x  1 irashid  staff    65K Oct 26  2016 local-1422981759269
-rwxr-xr-x  1 irashid  staff    65K Oct 26  2016 local-1425081759269
-rwxr-xr-x  1 irashid  staff    17K Oct 26  2016 local-1426533911241
-rwxr-xr-x  1 irashid  staff    17K Oct 26  2016 local-1426633911242
-rw-r--r--  1 irashid  staff   5.1K Oct 26  2016 local-1430917381535_1
-rw-r--r--  1 irashid  staff   5.1K Oct 26  2016 local-1430917381535_2

core/src/main/scala/org/apache/spark/metrics/ExecutorMetricType.scala

LantaoJin · 2019-01-08T07:12:43Z

@squito Replaced with a small one:

285K Jan 8 12:42 core/src/test/resources/spark-events/application_1536831636016_59384_1

squito · 2019-01-08T16:17:32Z

Jenkins, ok to test

SparkQA · 2019-01-08T16:21:25Z

Test build #100934 has finished for PR 22874 at commit c3faf61.

This patch fails RAT tests.
This patch merges cleanly.
This patch adds no public classes.

LantaoJin · 2019-01-09T02:37:32Z

retest this please

gaborgsomogyi · 2019-01-09T16:53:29Z

@LantaoJin it won't work even with retest because of the following:

Could not find Apache license headers in the following files:
 !????? /home/jenkins/workspace/SparkPullRequestBuilder/core/src/test/resources/spark-events/application_1536831636016_59384_1

rezasafi · 2019-01-09T17:29:38Z

@LantaoJin you should add "application_1536831636016_59384_1 "to /dev/.rat-excludes

LantaoJin · 2019-01-10T03:26:11Z

@gaborgsomogyi @rezasafi Thank you for the kindly reminding.

LantaoJin · 2019-01-10T03:26:52Z

Jenkins, ok to test

SparkQA · 2019-01-10T07:18:44Z

Test build #101001 has finished for PR 22874 at commit 3d9a02c.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

LantaoJin · 2019-01-10T07:42:19Z

Fails by Flaky Test: HiveClientSuites

LantaoJin · 2019-01-10T07:42:33Z

retest this please

core/src/main/scala/org/apache/spark/internal/config/package.scala

LantaoJin · 2019-01-11T09:55:29Z

Jenkins, ok to test

SparkQA · 2019-01-11T14:33:41Z

Test build #101079 has finished for PR 22874 at commit 6dede3e.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

LantaoJin · 2019-01-15T09:24:01Z

Gentle ping @squito @kiszk

squito

generally looks good, a couple of minor things

core/src/main/scala/org/apache/spark/metrics/ExecutorMetricType.scala

LantaoJin · 2019-02-19T04:32:18Z

Fix the conflicts of master.
retest this please

SparkQA · 2019-02-19T20:54:45Z

Test build #102507 has finished for PR 22874 at commit 6d5643c.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

LantaoJin · 2019-02-20T06:34:48Z

retest this please

SparkQA · 2019-02-20T08:05:02Z

Test build #102534 has finished for PR 22874 at commit 4d1021c.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

LantaoJin · 2019-02-20T08:17:08Z

retest this please

squito

other than a nit, lgtm, assuming tests pass.

Can you please update the description, as that becomes the commit msg? I would add some of your prior explanation for the utility of this as a metric over the lifetime of the executor #22874 (comment), though its not so useful per-stage.

core/src/main/scala/org/apache/spark/metrics/ExecutorMetricType.scala

SparkQA · 2019-02-21T01:39:34Z

Test build #4561 has finished for PR 22874 at commit 4d1021c.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2019-02-21T07:47:39Z

Test build #102572 has finished for PR 22874 at commit ba500fe.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

kiszk · 2019-02-24T16:37:39Z

core/src/main/scala/org/apache/spark/metrics/ExecutorMetricType.scala

+      } 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 " +


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 ....

@kiszk Yes, I've modified it now.

kiszk · 2019-02-24T16:38:24Z

LGTM except one minor comment.

SparkQA · 2019-02-25T14:20:46Z

Test build #102745 has finished for PR 22874 at commit 958a728.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

LantaoJin · 2019-02-27T04:34:42Z

Gently ping @squito @kiszk

squito · 2019-03-04T20:26:52Z

merged to master, thanks @LantaoJin -- sorry for the delay from me, was out last week

gatorsmile

@LantaoJin Could you submit another PR to document these new metrics in https://spark.apache.org/docs/latest/monitoring.html

If we do not document them, the Spark users will not be aware of them.

LantaoJin · 2019-03-14T08:56:08Z

@LantaoJin Could you submit another PR to document these new metrics in https://spark.apache.org/docs/latest/monitoring.html

If we do not document them, the Spark users will not be aware of them.

@gatorsmile PR: #24090 , Could you help to review when you have time?

drauschenbach · 2020-07-27T17:54:20Z

I use the IBM java sdk 8.0, and after upgrading to Spark 3 I see these warnings on every single job run:

20/07/26 11:40:38 WARN GarbageCollectionMetrics: To enable non-built-in garbage collector(s) List(scavenge), users should configure it(them) to spark.eventLog.gcMetrics.youngGenerationGarbageCollectors or spark.eventLog.gcMetrics.oldGenerationGarbageCollectors
20/07/26 11:40:38 WARN GarbageCollectionMetrics: To enable non-built-in garbage collector(s) List(global, scavenge), users should configure it(them) to spark.eventLog.gcMetrics.youngGenerationGarbageCollectors or spark.eventLog.gcMetrics.oldGenerationGarbageCollectors

Does anyone know what to do?

I'm using the Baizaicloud Prometheus push gateway metrics sink, if that's relevant.

LantaoJin · 2020-07-28T02:46:48Z

@drauschenbach I am not sure, but looks like scavenge is the young generation collector of IBM JDK and the global may be the old gen. (To confirm it, you'd better to refer the document of IBM JDK). So you can add spark.eventLog.gcMetrics.youngGenerationGarbageCollectors scavenge and spark.eventLog.gcMetrics.oldGenerationGarbageCollectors global in spark-defaults.conf or set --conf spark.eventLog.gcMetrics.youngGenerationGarbageCollectors=scavenge when submitting. This is used to collect GC metrics from Spark executors. If you don't need these GC metrics, just ignore the warn log.

drauschenbach · 2020-07-29T01:12:33Z

@LantaoJin Works great, thanks!

LantaoJin changed the title ~~[WIP][SPARK-25865][CORE] Add GC information to ExecutorMetrics~~ [SPARK-25865][WIP][CORE] Add GC information to ExecutorMetrics Oct 29, 2018

LantaoJin force-pushed the SPARK-25865 branch from a049454 to 6a7d06b Compare December 17, 2018 10:06

LantaoJin changed the title ~~[SPARK-25865][WIP][CORE] Add GC information to ExecutorMetrics~~ [SPARK-25865][CORE] Add GC information to ExecutorMetrics Dec 17, 2018

kiszk reviewed Dec 19, 2018

View reviewed changes

core/src/main/scala/org/apache/spark/metrics/ExecutorMetricType.scala Outdated Show resolved Hide resolved

squito reviewed Dec 19, 2018

View reviewed changes

kiszk reviewed Jan 7, 2019

View reviewed changes

core/src/main/scala/org/apache/spark/metrics/ExecutorMetricType.scala Outdated Show resolved Hide resolved

squito suggested changes Jan 7, 2019

View reviewed changes

core/src/main/scala/org/apache/spark/metrics/ExecutorMetricType.scala Outdated Show resolved Hide resolved

kiszk reviewed Jan 10, 2019

View reviewed changes

core/src/main/scala/org/apache/spark/internal/config/package.scala Outdated Show resolved Hide resolved

kiszk reviewed Jan 10, 2019

View reviewed changes

core/src/main/scala/org/apache/spark/internal/config/package.scala Outdated Show resolved Hide resolved

squito reviewed Jan 17, 2019

View reviewed changes

core/src/main/scala/org/apache/spark/metrics/ExecutorMetricType.scala Outdated Show resolved Hide resolved

core/src/main/scala/org/apache/spark/metrics/ExecutorMetricType.scala Outdated Show resolved Hide resolved

LantaoJin added 7 commits February 19, 2019 12:26

make configurable

94fd962

address comments

e4d9acd

address comments

aa3cf3c

address comments

544749d

address rat check

fe5a04c

address comments

12fc5cc

remove the unclear gc metric configuration

6d5643c

LantaoJin force-pushed the SPARK-25865 branch from 3e8c6b7 to 6d5643c Compare February 19, 2019 04:29

fix ut

4d1021c

squito reviewed Feb 20, 2019

View reviewed changes

core/src/main/scala/org/apache/spark/metrics/ExecutorMetricType.scala Outdated Show resolved Hide resolved

address the comments

ba500fe

kiszk reviewed Feb 24, 2019

View reviewed changes

make log clear

958a728

asfgit closed this in e5c502c Mar 4, 2019

gatorsmile reviewed Mar 13, 2019

View reviewed changes

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

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

Uh oh!

Conversation

LantaoJin commented Oct 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

squito commented Dec 11, 2018

Uh oh!

LantaoJin commented Dec 13, 2018

Uh oh!

LantaoJin commented Dec 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rezasafi commented Dec 17, 2018

Uh oh!

Uh oh!

squito left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

LantaoJin commented Jan 2, 2019

Uh oh!

LantaoJin commented Jan 2, 2019

Uh oh!

Uh oh!

squito left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

LantaoJin commented Jan 8, 2019

Uh oh!

squito commented Jan 8, 2019

Uh oh!

SparkQA commented Jan 8, 2019

Uh oh!

LantaoJin commented Jan 9, 2019

Uh oh!

gaborgsomogyi commented Jan 9, 2019

Uh oh!

rezasafi commented Jan 9, 2019

Uh oh!

LantaoJin commented Jan 10, 2019

Uh oh!

LantaoJin commented Jan 10, 2019

Uh oh!

SparkQA commented Jan 10, 2019

Uh oh!

LantaoJin commented Jan 10, 2019

Uh oh!

LantaoJin commented Jan 10, 2019

Uh oh!

Uh oh!

Uh oh!

LantaoJin commented Jan 11, 2019

Uh oh!

SparkQA commented Jan 11, 2019

Uh oh!

LantaoJin commented Jan 15, 2019

Uh oh!

squito left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

LantaoJin commented Feb 19, 2019

Uh oh!

SparkQA commented Feb 19, 2019

Uh oh!

LantaoJin commented Feb 20, 2019

Uh oh!

SparkQA commented Feb 20, 2019

Uh oh!

LantaoJin commented Feb 20, 2019

Uh oh!

LantaoJin commented Oct 29, 2018 •

edited

Loading

LantaoJin commented Dec 17, 2018 •

edited

Loading

drauschenbach commented Jul 27, 2020 •

edited

Loading

LantaoJin commented Jul 28, 2020 •

edited

Loading