mimaison commented on code in PR #19286:
URL: https://github.com/apache/kafka/pull/19286#discussion_r2013965436
##########
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:
This is annoying.
Is there a good reason RSM and RLMM are not configured when they are created?
Currently the logic in `BrokerServer` first creates the `RemoteLogManager`
instance, then optionally calls `onEndPointCreated()`, before finally calling
`startup()`.
It looks like we could compute the `Endpoint` object before creating the
`RemoteLogManager` instance and pass it directly in the constructor as
`Optional<Endpoint>` instead of using `onEndPointCreated()`. That way in the
`RemoteLogManager` constructor, we could instantiate and immediately configure
the RSM and RLMM instances.
This would allow us to delete `onEndPointCreated()` (it's only once used in
`BrokerServer`) and streamline the creation of `RemoteLogManager`.
@showuon @satishd WDYT?
--
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
