[GitHub] [ozone] adoroszlai commented on a diff in pull request #5126: HDDS-9090. NullPointerException thrown by ReplicationManagerMetrics because of the order of initialization

["adoroszlai (via GitHub)" <gi...@apache.org>]

adoroszlai commented on code in PR #5126:
URL: https://github.com/apache/ozone/pull/5126#discussion_r1278258745


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManager.java:
##########
@@ -285,7 +286,6 @@ public synchronized void start() {
     if (!isRunning()) {
       LOG.info("Starting Replication Monitor Thread.");
       running = true;
-      metrics = ReplicationManagerMetrics.create(this);
       if (rmConf.isLegacyEnabled()) {
         legacyReplicationManager.setMetrics(metrics);
       }

Review Comment:
   Thanks @aswinshakil for the fix.  Please note that `ReplicationManager` can be stopped/restarted by admin from CLI.  
   `stop()` unregisters the metrics source, and `start()` used to (re)create it.   But now it isn't re-created, so it won't work after restart.  One way to fix this is to add a method in `ReplicationManagerMetrics` to register it with the metrics system (without creating a new instance), and call this new method from `start()`.



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

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org