chia7712 commented on code in PR #16353:
URL: https://github.com/apache/kafka/pull/16353#discussion_r1641654171
##########
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:
not sure whether I have caught the context, so please feel free to correct
me.
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?
2. those configs (`remote.log.manager.copy.max.bytes.per.second`,
`remote.log.manager.fetch.max.bytes.per.second`) can be updated by
`reconfigure` process, so it should be fine to initialize them with "stale"
(static) configs after broker restart, right?
##########
core/src/main/java/kafka/log/remote/RemoteLogManager.java:
##########
@@ -412,7 +418,7 @@ public void onLeadershipChange(Set<Partition>
partitionsBecomeLeader,
Map<String, Uuid> topicIds) {
LOGGER.debug("Received leadership changes for leaders: {} and
followers: {}", partitionsBecomeLeader, partitionsBecomeFollower);
- if (this.rlmConfig.isRemoteStorageSystemEnabled() &&
!isRemoteLogManagerConfigured()) {
+ if (config.remoteLogManagerConfig().isRemoteStorageSystemEnabled() &&
!isRemoteLogManagerConfigured()) {
Review Comment:
this is unrelated to this PR, but `config.remoteLogManagerConfig` always
return the same config even though the `config` is updated dynamically. That
seems to be error-prone, since it means the "updated" config can return
"remoteLogManagerConfig" with "previous" configs.
--
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]
