showuon commented on code in PR #17499:
URL: https://github.com/apache/kafka/pull/17499#discussion_r1802513512
##########
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:
Yes, I think the fallback mechanism is for backward compatibility, so it is
required. Suppose there's a user setting `remote.log.manager.thread.pool.size`
to 1 due to resource limit, and then, after upgrading to 3.9, it'll become 10,
which is not good.
@fvaleri , could you update in the PR, to apply the fallback logic? I'm
thinking we can do:
1. remove the `atLeast(1)` validator for
`remote.log.manager.copier.thread.pool.size` and
`remote.log.manager.expiration.thread.pool.size`.
2. Set the default value of `remote.log.manager.copier.thread.pool.size` and
`remote.log.manager.expiration.thread.pool.size` to 0.
3. When getting the value of `remote.log.manager.copier.thread.pool.size`
and `remote.log.manager.expiration.thread.pool.size`, if we saw `the value is
<= 0`, we set the value of `remote.log.manager.thread.pool.size`.
Does that make sense?
--
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
