i3wangyi commented on a change in pull request #365: Fix RoutingTableProvider 
statePropagationLatency metric reporting bug
URL: https://github.com/apache/helix/pull/365#discussion_r308438586
 
 

 ##########
 File path: 
helix-core/src/main/java/org/apache/helix/spectator/RoutingTableProvider.java
 ##########
 @@ -558,23 +558,28 @@ protected void refresh(Map<String, Map<String, 
Map<String, CurrentState>>> curre
 
   private void resetRoutingTableAndNotify(long startTime, RoutingTable 
newRoutingTable) {
     _routingTableRef.set(newRoutingTable);
-    logger.info("Refresh the RoutingTable for cluster {}, takes {} ms.",
-        (_helixManager != null ? _helixManager.getClusterName() : null),
+    String clusterName = _helixManager != null ? 
_helixManager.getClusterName() : null;
+    logger.info("Refresh the RoutingTable for cluster {}, takes {} ms.", 
clusterName,
         (System.currentTimeMillis() - startTime));
-    notifyRoutingTableChange();
+
+    notifyRoutingTableChange(_helixManager != null ? 
_helixManager.getClusterName() : null);
 
 Review comment:
   Is it because you'd like to link the first log info with the following logs? 
   Optional is a good idea but if it's the purpose of logging, why cannot we 
move the 1st log into the private method.
   Then you can skip if it's null

----------------------------------------------------------------
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:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to