mimaison commented on code in PR #19286: URL: https://github.com/apache/kafka/pull/19286#discussion_r2014493205
########## 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: See my attempt at implementing what I described: https://github.com/apache/kafka/commit/89e9a633cb605bc3a865c5001da704bd29628890 I don't think this changes the contracts we currently have with regards to `RemoteLogMetadataManager` and `RemoteStorageManager`. For both we still call the empty constructor, followed by a call to `configure()`, and now potentially a call to `withPluginMetrics()`, if the implementation is `Monitorable`. -- 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