[SPARK-21334][CORE] Add metrics reporting service to External Shuffle Server

[raajay]

What changes were proposed in this pull request?

Add a metrics reporting service, that periodically reports the metrics defined in ExternalShuffleServiceSource. Currently, although the metrics are defined, they are never reported.

How was this patch tested?

Manual tests to ensure that metrics are reported on ExternalShuffleService start.

[AmplabJenkins]

Can one of the admins verify this patch?

[jerryshao]

I think it is because you don't configure metrics sink here, so by default none of the metrics are reported.

For example if I enabled jmx sink (*.sink.jmx.class=org.apache.spark.metrics.sink.JmxSink).

Then using jconsole to connect to shuffle service, then you will see it on jconsole:

Please elaborate more if you have specific issue regarding this.

[raajay]

We were using a custom sink rather than the JmxSink for gathering metrics. The sink did NOT have a "reporter" like the ones JmxSink or CsvSink have. I guess a cleaner design is to implement a metrics reporter in the Sink and not have a reporting service as part of external shuffle service. Thanks!

[jerryshao]

If your custom sink doesn't have a "reporter", then I'm not sure how does this code work in your changes? Since MetricsSystem#report will call report API of each sink. I'm still not clearly sure about your scenario.

  override def run(): Unit = {
     metricsSystem.report()
  }

[raajay]

@jerryshao My CustomSInk has the report function defined. What I did not have was an equivalent of JmxReporter defined in my CustomSink. The reporter essentially periodically invokes the report function defined in CustomSink

[jerryshao]

So I think if you want to connect your custom sink to Spark Metrics System, then you should at least follow what Spark and codahale metrics library did. Adding a feature in Spark specifically works only for your own sink seems not so reasonable, unless it is a general requirement.

[raajay]

I understand. My previous comment was just a clarification to your question: "I'm not sure how does this code work in your changes?". I will close this PR. The JIRA is already closed.