manuzhang commented on a change in pull request #28416:
URL: https://github.com/apache/spark/pull/28416#discussion_r418357988
##########
File path:
common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/ExternalBlockHandler.java
##########
@@ -242,7 +240,6 @@ public ShuffleMetrics() {
allMetrics.put("registeredExecutorsSize",
(Gauge<Integer>) () ->
blockManager.getRegisteredExecutorsSize());
allMetrics.put("numActiveConnections", activeConnections);
- allMetrics.put("numRegisteredConnections", registeredConnections);
Review comment:
This line looks suspicious and confusing since
`numRegisteredConnections` is registered twice for `YarnShuffleService`. After
digging into it, I find it's [the Counter from
TransportContext](https://github.com/apache/spark/blob/master/common/network-common/src/main/java/org/apache/spark/network/TransportContext.java#L71)
that's really [counting the
numbers](https://github.com/apache/spark/blob/master/common/network-common/src/main/java/org/apache/spark/network/server/TransportChannelHandler.java#L189).
That created here in `ExternalBlockHandler#ShuffleMetrics` is never used
anywhere.
I think it's misleading for people who want to add new metrics. That said,
I'm ok to leave it as is if you still suggest so.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
