Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/19160#discussion_r139799940
--- Diff:
core/src/main/scala/org/apache/spark/deploy/ExternalShuffleServiceSource.scala
---
@@ -19,19 +19,19 @@ package org.apache.spark.deploy
import javax.annotation.concurrent.ThreadSafe
-import com.codahale.metrics.MetricRegistry
+import com.codahale.metrics.{MetricRegistry, MetricSet}
import org.apache.spark.metrics.source.Source
-import org.apache.spark.network.shuffle.ExternalShuffleBlockHandler
/**
* Provides metrics source for external shuffle service
*/
@ThreadSafe
-private class ExternalShuffleServiceSource
-(blockHandler: ExternalShuffleBlockHandler) extends Source {
+private class ExternalShuffleServiceSource extends Source {
--- End diff --
just for my own understanding, not directly related to this change --
I hadn't realized that the ExternalShuffleBlockHandler had its own
ShuffleMetrics already. Some of those metrics really seem like they should be
part of regular shuffle server, in the executor. Eg.,
[openBlockRequestLatencyMillis](https://github.com/apache/spark/blob/master/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/ExternalShuffleBlockHandler.java#L89).
Do you know why its separate?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
