kamalcph commented on code in PR #16353:
URL: https://github.com/apache/kafka/pull/16353#discussion_r1641760409
##########
core/src/main/java/kafka/log/remote/RemoteLogManager.java:
##########
@@ -226,7 +227,8 @@ public RemoteLogManager(RemoteLogManagerConfig rlmConfig,
rlmCopyQuotaManager = createRLMCopyQuotaManager();
rlmFetchQuotaManager = createRLMFetchQuotaManager();
- indexCache = new
RemoteIndexCache(rlmConfig.remoteLogIndexFileCacheTotalSizeBytes(),
remoteLogStorageManager, logDir);
+ RemoteLogManagerConfig rlmConfig = config.remoteLogManagerConfig();
+ indexCache = new
RemoteIndexCache(config.remoteLogIndexFileCacheTotalSizeBytes(),
remoteLogStorageManager, logDir);
Review Comment:
> 1. IIRC, the dynamical configs are loaded by another thread, and hence we
may NOT see the latest configs, which were updated dynamically, in creating
RemoteLogManager, right?
No, KafkaServer/BrokerServer does `config.dynamicConfig.initialize` before
creating the RemoteLogManager instance so the dynamic configs gets updated in
the `KafkaConfig` object but not in the `KafkaConfig.remoteLogManagerConfig()`.
I have tested the patch only with ZooKeeper. I think the behavior should be
similar of KRaftMetadataCache/ConfigRepository.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
