[SPARK-25277][YARN] YARN applicationMaster metrics should not register static metrics #22279

LucaCanali · 2018-08-30T07:27:19Z

What changes were proposed in this pull request?

YARN applicationMaster metrics registration introduced in SPARK-24594 causes further registration of static metrics (Codegenerator and HiveExternalCatalog) and of JVM metrics, which I believe do not belong in this context.
This looks like an unintended side effect of using the start method of [[MetricsSystem]].
A possible solution proposed here, is to introduce startNoRegisterSources to avoid these additional registrations of static sources and of JVM sources in the case of YARN applicationMaster metrics (this could be useful for other metrics that may be added in the future).

How was this patch tested?

Manually tested on a YARN cluster,

jerryshao · 2018-08-31T03:02:55Z

Hi @LucaCanali do you have an output current AM metrics? I would like to know what kind of metrics will be output for now.

jerryshao · 2018-08-31T03:04:06Z

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

    sinks.foreach(_.start)
  }

+  // Same as start but this method only registers sinks


Would you please explain why only registering sinks could solve the problem here?

What I am trying to do is to avoid the registration of the "static metrics", for CodeGeneration and HiveExternalCatalog and also for JVM.

LucaCanali · 2018-08-31T09:04:44Z

Hi @jerryshao you can find here below an example of metrics currently reported by applicationMaster, illustrating the issue reported here. You can find there the list of AM metrics reported (with the application ID as a prefix by default). In addition metrics for CodeGeneration and HiveExternalCatalog are also reported, these metrics do not make sense in this context, in addition they have no prefix. Metrics for JVM are reported too (without application_id prefix), which I am not sure it is wanted either.

bin/spark-shell --master yarn \
--conf "spark.metrics.conf.applicationMaster.sink.graphite.class"="org.apache.spark.metrics.sink.GraphiteSink" \
--conf "spark.metrics.conf.*.sink.graphite.host"=lc-mytest5 \
--conf "spark.metrics.conf.*.sink.graphite.port"=2003 \
--conf "spark.metrics.conf.*.sink.graphite.period"=10 \
--conf "spark.metrics.conf.*.sink.graphite.unit"=seconds \
--conf "spark.metrics.conf.*.sink.graphite.prefix"="luca" \
--conf "spark.metrics.conf.*.source.jvm.class"="org.apache.spark.metrics.source.JvmSource"

I have used InfluxDB to collect the metrics. This is the output of "show measurements" in InfluxDB:

name: measurements
name
----
CodeGenerator.compilationTime.count
CodeGenerator.compilationTime.max
CodeGenerator.compilationTime.mean
CodeGenerator.compilationTime.min
CodeGenerator.compilationTime.p50
CodeGenerator.compilationTime.p75
CodeGenerator.compilationTime.p95
CodeGenerator.compilationTime.p98
CodeGenerator.compilationTime.p99
CodeGenerator.compilationTime.p999
CodeGenerator.compilationTime.stddev
CodeGenerator.generatedClassSize.count
CodeGenerator.generatedClassSize.max
CodeGenerator.generatedClassSize.mean
CodeGenerator.generatedClassSize.min
CodeGenerator.generatedClassSize.p50
CodeGenerator.generatedClassSize.p75
CodeGenerator.generatedClassSize.p95
CodeGenerator.generatedClassSize.p98
CodeGenerator.generatedClassSize.p99
CodeGenerator.generatedClassSize.p999
CodeGenerator.generatedClassSize.stddev
CodeGenerator.generatedMethodSize.count
CodeGenerator.generatedMethodSize.max
CodeGenerator.generatedMethodSize.mean
CodeGenerator.generatedMethodSize.min
CodeGenerator.generatedMethodSize.p50
CodeGenerator.generatedMethodSize.p75
CodeGenerator.generatedMethodSize.p95
CodeGenerator.generatedMethodSize.p98
CodeGenerator.generatedMethodSize.p99
CodeGenerator.generatedMethodSize.p999
CodeGenerator.generatedMethodSize.stddev
CodeGenerator.sourceCodeSize.count
CodeGenerator.sourceCodeSize.max
CodeGenerator.sourceCodeSize.mean
CodeGenerator.sourceCodeSize.min
CodeGenerator.sourceCodeSize.p50
CodeGenerator.sourceCodeSize.p75
CodeGenerator.sourceCodeSize.p95
CodeGenerator.sourceCodeSize.p98
CodeGenerator.sourceCodeSize.p99
CodeGenerator.sourceCodeSize.p999
CodeGenerator.sourceCodeSize.stddev
HiveExternalCatalog.fileCacheHits.count
HiveExternalCatalog.filesDiscovered.count
HiveExternalCatalog.hiveClientCalls.count
HiveExternalCatalog.parallelListingJobCount.count
HiveExternalCatalog.partitionsFetched.count
application_1516620698330_110908.applicationMaster.numContainersPendingAllocate
application_1516620698330_110908.applicationMaster.numExecutorsFailed
application_1516620698330_110908.applicationMaster.numExecutorsRunning
application_1516620698330_110908.applicationMaster.numLocalityAwareTasks
application_1516620698330_110908.applicationMaster.numReleasedContainers
jvm.PS-MarkSweep.count
jvm.PS-MarkSweep.time
jvm.PS-Scavenge.count
jvm.PS-Scavenge.time
jvm.direct.capacity
jvm.direct.count
jvm.direct.used
jvm.heap.committed
jvm.heap.init
jvm.heap.max
jvm.heap.usage
jvm.heap.used
jvm.mapped.capacity
jvm.mapped.count
jvm.mapped.used
jvm.non-heap.committed
jvm.non-heap.init
jvm.non-heap.max
jvm.non-heap.usage
jvm.non-heap.used
jvm.pools.Code-Cache.committed
jvm.pools.Code-Cache.init
jvm.pools.Code-Cache.max
jvm.pools.Code-Cache.usage
jvm.pools.Code-Cache.used
jvm.pools.Compressed-Class-Space.committed
jvm.pools.Compressed-Class-Space.init
jvm.pools.Compressed-Class-Space.max
jvm.pools.Compressed-Class-Space.usage
jvm.pools.Compressed-Class-Space.used
jvm.pools.Metaspace.committed
jvm.pools.Metaspace.init
jvm.pools.Metaspace.max
jvm.pools.Metaspace.usage
jvm.pools.Metaspace.used
jvm.pools.PS-Eden-Space.committed
jvm.pools.PS-Eden-Space.init
jvm.pools.PS-Eden-Space.max
jvm.pools.PS-Eden-Space.usage
jvm.pools.PS-Eden-Space.used
jvm.pools.PS-Old-Gen.committed
jvm.pools.PS-Old-Gen.init
jvm.pools.PS-Old-Gen.max
jvm.pools.PS-Old-Gen.usage
jvm.pools.PS-Old-Gen.used
jvm.pools.PS-Survivor-Space.committed
jvm.pools.PS-Survivor-Space.init
jvm.pools.PS-Survivor-Space.max
jvm.pools.PS-Survivor-Space.usage
jvm.pools.PS-Survivor-Space.used
jvm.total.committed
jvm.total.init
jvm.total.max
jvm.total.used

LucaCanali · 2018-10-05T11:59:48Z

@jerryshao would you have any additional comments on this?

LucaCanali · 2018-10-15T06:53:10Z

@attilapiros would you be interested to review this as a follow-up of your work on [SPARK-24594][YARN] Introducing metrics for YARN ?

attilapiros · 2018-10-15T08:56:00Z

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

    sinks.foreach(_.start)
  }

+  // Same as start but this method only registers sinks


As I see it is a duplication of the start() method without the registerSources() but what about extending the start() method with a new boolean flag (like registerStaticSources:Boolean = true) and using this flag to decide registerSources() should be called or not. Then in AM you can call it with false. This way there is no code duplication and more clear what is the different between the two usage.

Thanks @attilapiros for looking at this. I agree with your proposal. I'll provide an update to the PR.

I was thinking a bit more about this PR and in case of client mode it could make sense to have some of this static metrics as the driver and AM are separated. What about (assuming the boolean flag is there) calling the start method with false only in case of cluster mode?

I would split the case in 2 topics:

I belive that CodeGenerator and HiveExternalCatalog metrics don't make sense in the context of AM, so they can be safely removed.

The JVM metrics may be relevant as you mentioned. Although in the current version I see the problem that the JVM metrics for the AM appear without any application id nor prefix, so they are difficult to process. I guess this part can be improved if we think JVM metrics for AM can be of interest.

I can just fully agree with you.

Just to close the loop here: I like the extra flag to start instead of the separate method.

As for AM vs. driver, the way I understand things, in cluster mode, there will be two MetricSystem instances - one for the AM and one for the driver, so you shouldn't lose any driver metrics.

JVM metrics for the client-mode AM can be interesting, but that's generally not a source of problems that I've noticed, so we can probably punt on it for now.

I have implemented the extra flag to start and tested the patch on a YARN cluster using a graphite sink for the metrics.

vanzin · 2018-12-10T23:58:42Z

ok to test

SparkQA · 2018-12-11T04:20:15Z

Test build #99939 has finished for PR 22279 at commit a990758.

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

LucaCanali · 2018-12-11T07:40:28Z

Thanks @vanzin for looking at this.

SparkQA · 2018-12-12T13:35:52Z

Test build #100017 has finished for PR 22279 at commit fd11730.

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

vanzin · 2018-12-12T17:02:56Z

retest this please

vanzin · 2018-12-12T18:30:33Z

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

    running = true
-    StaticSources.allSources.foreach(registerSource)
-    registerSources()
+    if(registerStaticSources) {


nit: space before (

SparkQA · 2018-12-12T21:02:12Z

Test build #100038 has finished for PR 22279 at commit fd11730.

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

SparkQA · 2018-12-13T00:07:20Z

Test build #100044 has finished for PR 22279 at commit 9c21f16.

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

vanzin · 2018-12-13T00:17:58Z

Giving up on flaky tests... interesting ones all passed. Merging to master.

…r static metrics ## What changes were proposed in this pull request? YARN applicationMaster metrics registration introduced in SPARK-24594 causes further registration of static metrics (Codegenerator and HiveExternalCatalog) and of JVM metrics, which I believe do not belong in this context. This looks like an unintended side effect of using the start method of [[MetricsSystem]]. A possible solution proposed here, is to introduce startNoRegisterSources to avoid these additional registrations of static sources and of JVM sources in the case of YARN applicationMaster metrics (this could be useful for other metrics that may be added in the future). ## How was this patch tested? Manually tested on a YARN cluster, Closes apache#22279 from LucaCanali/YarnMetricsRemoveExtraSourceRegistration. Lead-authored-by: Luca Canali <[email protected]> Co-authored-by: LucaCanali <[email protected]> Signed-off-by: Marcelo Vanzin <[email protected]>

dongjoon-hyun · 2019-09-16T06:54:26Z

Hi, All. Sorry for commenting on this old PR. This looks safe and worthy for backporting.
Since branch-2.4 is our LTS branch for 2.4.x, I'll test and backport this to branch-2.4.

…r static metrics ## What changes were proposed in this pull request? YARN applicationMaster metrics registration introduced in SPARK-24594 causes further registration of static metrics (Codegenerator and HiveExternalCatalog) and of JVM metrics, which I believe do not belong in this context. This looks like an unintended side effect of using the start method of [[MetricsSystem]]. A possible solution proposed here, is to introduce startNoRegisterSources to avoid these additional registrations of static sources and of JVM sources in the case of YARN applicationMaster metrics (this could be useful for other metrics that may be added in the future). ## How was this patch tested? Manually tested on a YARN cluster, Closes #22279 from LucaCanali/YarnMetricsRemoveExtraSourceRegistration. Lead-authored-by: Luca Canali <[email protected]> Co-authored-by: LucaCanali <[email protected]> Signed-off-by: Marcelo Vanzin <[email protected]>

YARN applicationMaster metrics should not register static metrics

a990758

jerryshao reviewed Aug 31, 2018

View reviewed changes

attilapiros reviewed Oct 15, 2018

View reviewed changes

Implemented extra flag to start as in the PR discussion.

fd11730

vanzin reviewed Dec 12, 2018

View reviewed changes

formatting

9c21f16

asfgit closed this in 2920438 Dec 13, 2018

dongjoon-hyun mentioned this pull request Jul 24, 2020

[SPARK-32409][DOC] Document dependency between spark.metrics.staticSources.enabled and JVMSource registration #29203

Closed

[SPARK-25277][YARN] YARN applicationMaster metrics should not register static metrics #22279

[SPARK-25277][YARN] YARN applicationMaster metrics should not register static metrics #22279

Uh oh!

Conversation

LucaCanali commented Aug 30, 2018

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

jerryshao commented Aug 31, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LucaCanali commented Aug 31, 2018

Uh oh!

LucaCanali commented Oct 5, 2018

Uh oh!

LucaCanali commented Oct 15, 2018

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vanzin commented Dec 10, 2018

Uh oh!

SparkQA commented Dec 11, 2018

Uh oh!

LucaCanali commented Dec 11, 2018

Uh oh!

SparkQA commented Dec 12, 2018

Uh oh!

vanzin commented Dec 12, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Dec 12, 2018

Uh oh!

SparkQA commented Dec 13, 2018

Uh oh!

vanzin commented Dec 13, 2018

Uh oh!

dongjoon-hyun commented Sep 16, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

attilapiros Oct 15, 2018 •

edited

Loading