gaurav-narula commented on code in PR #17499:
URL: https://github.com/apache/kafka/pull/17499#discussion_r1799875723
##########
storage/src/main/java/org/apache/kafka/server/log/remote/storage/RemoteLogManagerConfig.java:
##########
@@ -257,13 +257,13 @@ public static ConfigDef configDef() {
atLeast(1),
MEDIUM,
REMOTE_LOG_MANAGER_THREAD_POOL_SIZE_DOC)
-
.defineInternal(REMOTE_LOG_MANAGER_COPIER_THREAD_POOL_SIZE_PROP,
+ .define(REMOTE_LOG_MANAGER_COPIER_THREAD_POOL_SIZE_PROP,
INT,
DEFAULT_REMOTE_LOG_MANAGER_COPIER_THREAD_POOL_SIZE,
Review Comment:
Unrelated to this change, but the KIP mentions:
> If no thread values are set for the two new configurations presented later
on in the document we will default to using the same number of threads in each
pool as detailed by remote.log.manager.thread.pool.size.
Perhaps I'm mistaken, but this gives the impression that the default for the
new config is falling back to `remote.log.manager.thread.pool.size`.
[Configuration
section](https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=263424654#KIP950:TieredStorageDisablement-Configuration)
however specifies a default of 10 for the new configs. Seems a bit ambiguous
and I wonder if the default is correct?
--
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
