clolov commented on code in PR #16502:
URL: https://github.com/apache/kafka/pull/16502#discussion_r1679077395
##########
checkstyle/suppressions.xml:
##########
@@ -40,6 +40,7 @@
files="core[\\/]src[\\/](generated|generated-test)[\\/].+.java$"/>
<suppress checks="NPathComplexity"
files="(ClusterTestExtensions|KafkaApisBuilder|SharePartition).java"/>
<suppress
checks="NPathComplexity|ClassFanOutComplexity|ClassDataAbstractionCoupling"
files="(RemoteLogManager|RemoteLogManagerTest).java"/>
+ <suppress checks="MethodLength" files="RemoteLogManager.java"/>
Review Comment:
For my understanding, what is causing this? I am somewhat surprised that
given you are breaking down methods into smaller ones to see this popping up
##########
core/src/main/java/kafka/log/remote/RemoteLogManager.java:
##########
@@ -241,12 +246,17 @@ public RemoteLogManager(RemoteLogManagerConfig rlmConfig,
indexCache = new
RemoteIndexCache(rlmConfig.remoteLogIndexFileCacheTotalSizeBytes(),
remoteLogStorageManager, logDir);
delayInMs = rlmConfig.remoteLogManagerTaskIntervalMs();
- rlmScheduledThreadPool = new
RLMScheduledThreadPool(rlmConfig.remoteLogManagerThreadPoolSize());
+ rlmCopyThreadPool = new
RLMScheduledThreadPool(rlmConfig.remoteLogManagerCopierThreadPoolSize(),
+ "RLMCopyThreadPool", "kafka-rlm-copy-thread-pool-");
+ rlmExpirationThreadPool = new
RLMScheduledThreadPool(rlmConfig.remoteLogManagerExpirationThreadPoolSize(),
+ "RLMExpirationThreadPool", "kafka-rlm-expiration-thread-pool-");
+ followerThreadPool = new
RLMScheduledThreadPool(rlmConfig.remoteLogManagerThreadPoolSize(),
+ "RLMFollowerScheduledThreadPool",
"kafka-rlm-follower-thread-pool-");
metricsGroup.newGauge(REMOTE_LOG_MANAGER_TASKS_AVG_IDLE_PERCENT_METRIC, new
Gauge<Double>() {
@Override
public Double value() {
- return rlmScheduledThreadPool.getIdlePercent();
+ return rlmCopyThreadPool.getIdlePercent();
Review Comment:
This is a miss on my side - I am happy to circle back to the KIP to mention
that this metric will either a) no longer show a reasonable value or b) will
show the value of the copy pool only. Also another miss on my side is that we
should surface the same metrics per thread pool that we have now for the new
thread pools. But I am happy to pick this up once this PR is closed 😊
--
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]
