TaiJuWu commented on code in PR #19286:
URL: https://github.com/apache/kafka/pull/19286#discussion_r2014001146


##########
core/src/main/java/kafka/log/remote/RemoteLogManager.java:
##########
@@ -407,14 +413,17 @@ private void configureRLMM() {
         rlmmProps.put(LOG_DIR_CONFIG, logDir);
         rlmmProps.put("cluster.id", clusterId);
 
-        remoteLogMetadataManager.configure(rlmmProps);
+        remoteLogMetadataManagerPlugin.get().configure(rlmmProps);
     }
 
     public void startup() {
         // Initialize and configure RSM and RLMM. This will start RSM, RLMM 
resources which may need to start resources
         // in connecting to the brokers or remote storages.
         configureRSM();
         configureRLMM();
+        // the withPluginMetrics() method will be called when the plugin is 
instantiated (after configure() if the plugin also implements Configurable)

Review Comment:
   
https://github.com/apache/kafka/pull/19286/commits/108129df77529aa53cfa42cdaae5a40f47459ff9
 is the commit I try moving `configure` to constructor but I finally gave it up.
   
   The most important consideration is `RemoteLogMetadataManager` and 
`RemoteStorageManager` are public plugin and we should not change this flow 
without any KIP.
   
   If I misunderstood, please correct me.  I will wait other feedback and 
continue to work later.



-- 
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: jira-unsubscr...@kafka.apache.org

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

Reply via email to