xkrogen commented on a change in pull request #32388:
URL: https://github.com/apache/spark/pull/32388#discussion_r623209427



##########
File path: 
common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/ExternalBlockHandler.java
##########
@@ -98,10 +99,10 @@ public ExternalBlockHandler(
       OneForOneStreamManager streamManager,
       ExternalShuffleBlockResolver blockManager,
       MergedShuffleFileManager mergeManager) {
-    this.metrics = new ShuffleMetrics();
     this.streamManager = streamManager;
     this.blockManager = blockManager;
     this.mergeManager = mergeManager;
+    this.metrics = new ShuffleMetrics();

Review comment:
       Great question! Changing from:
   ```
         allMetrics.put("registeredExecutorsSize",
                        (Gauge<Integer>) () -> 
blockManager.getRegisteredExecutorsSize());
   ```
   to
   ```
         allMetrics.put("registeredExecutorsSize",
             (Gauge<Integer>) blockManager::getRegisteredExecutorsSize);
   ```
   Means that the `blockManager` object is evaluated at the time of `new 
ShuffleMetrics()`, instead of later at the time the metrics are gathered. When 
the instantiation is the first line, `blockManager` is still null. If it's at 
the end, `blockManager` has been initialized by then.




-- 
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]

Reply via email to