Github user ash211 commented on a diff in the pull request:
https://github.com/apache/spark/pull/22485#discussion_r219916796
--- Diff:
common/network-yarn/src/main/java/org/apache/spark/network/yarn/YarnShuffleService.java
---
@@ -168,6 +170,15 @@ protected void serviceInit(Configuration conf) throws
Exception {
TransportConf transportConf = new TransportConf("shuffle", new
HadoopConfigProvider(conf));
blockHandler = new ExternalShuffleBlockHandler(transportConf,
registeredExecutorFile);
+ // register metrics on the block handler into the Node Manager's
metrics system.
+ YarnShuffleServiceMetrics serviceMetrics =
+ new YarnShuffleServiceMetrics(blockHandler.getAllMetrics());
+
+ MetricsSystemImpl metricsSystem = (MetricsSystemImpl)
DefaultMetricsSystem.instance();
+ metricsSystem.register(
--- End diff --
Sorry for the delay.
My explanation for using reflection was here:
https://github.com/palantir/spark/pull/149/files#r107770857 -- basically
`registerSource` isn't accessible from here.
As to why use `registerSource` vs `register`, I don't think I knew about
`register` at the time. Looking at
https://github.com/apache/hadoop/blame/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/impl/MetricsSystemImpl.java
`registerSource` ends up being called both ways.
If you can get the right behavior using the accessible method rather than
doing reflection, I'd say we should go with that way.
cc @robert3005 @mccheah fysa for potential future merge conflict
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
