You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "FrankYang0529 (via GitHub)" <gi...@apache.org> on 2025/04/22 11:26:07 UTC
[PR] KAFKA-19131: Adjust remote storage reader thread maximum pool size to avoid illegal argument [kafka]
FrankYang0529 opened a new pull request, #19532:
URL: https://github.com/apache/kafka/pull/19532
The remote storage reader thread pool use same count for both maximum and core size. If users adjust the pool size larger than original value, it throws `IllegalArgumentException`. Updated both value to fix the issue.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19131: Adjust remote storage reader thread maximum pool size to avoid illegal argument [kafka]
Posted by "FrankYang0529 (via GitHub)" <gi...@apache.org>.
FrankYang0529 commented on code in PR #19532:
URL: https://github.com/apache/kafka/pull/19532#discussion_r2059546369
##########
storage/src/main/java/org/apache/kafka/server/log/remote/storage/RemoteLogManager.java:
##########
@@ -293,6 +293,7 @@ public void resizeExpirationThreadPool(int newSize) {
public void resizeReaderThreadPool(int newSize) {
int currentSize = remoteStorageReaderThreadPool.getCorePoolSize();
LOGGER.info("Updating remote reader thread pool size from {} to {}", currentSize, newSize);
+ remoteStorageReaderThreadPool.setMaximumPoolSize(newSize);
Review Comment:
Sorry, I missed the decreasing thread count case. Updated `resizeReaderThreadPool` to handle it. Thanks.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19131: Adjust remote storage reader thread maximum pool size to avoid illegal argument [kafka]
Posted by "FrankYang0529 (via GitHub)" <gi...@apache.org>.
FrankYang0529 commented on code in PR #19532:
URL: https://github.com/apache/kafka/pull/19532#discussion_r2059546074
##########
core/src/test/scala/integration/kafka/admin/RemoteTopicCrudTest.scala:
##########
@@ -473,6 +473,40 @@ class RemoteTopicCrudTest extends IntegrationTestHarness {
verifyRemoteLogTopicConfigs(newProps)
}
+ @ParameterizedTest
+ @ValueSource(strings = Array("kraft"))
+ def testUpdateThreadPoolSize(quorum: String): Unit = {
Review Comment:
I forgot that the broker only logs error message when dynamic configuration is broken, so this test can pass even without the fix. Replaced it with unit test. Thanks.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19131: Adjust remote storage reader thread maximum pool size to avoid illegal argument [kafka]
Posted by "kamalcph (via GitHub)" <gi...@apache.org>.
kamalcph commented on PR #19532:
URL: https://github.com/apache/kafka/pull/19532#issuecomment-2845267563
@FrankYang0529
Could you also open this PR against [4.0](https://github.com/apache/kafka/tree/4.0) branch to backport the fix? Thanks!
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19131: Adjust remote storage reader thread maximum pool size to avoid illegal argument [kafka]
Posted by "kamalcph (via GitHub)" <gi...@apache.org>.
kamalcph merged PR #19532:
URL: https://github.com/apache/kafka/pull/19532
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19131: Adjust remote storage reader thread maximum pool size to avoid illegal argument [kafka]
Posted by "kamalcph (via GitHub)" <gi...@apache.org>.
kamalcph commented on code in PR #19532:
URL: https://github.com/apache/kafka/pull/19532#discussion_r2058955813
##########
storage/src/main/java/org/apache/kafka/server/log/remote/storage/RemoteLogManager.java:
##########
@@ -293,6 +293,7 @@ public void resizeExpirationThreadPool(int newSize) {
public void resizeReaderThreadPool(int newSize) {
int currentSize = remoteStorageReaderThreadPool.getCorePoolSize();
LOGGER.info("Updating remote reader thread pool size from {} to {}", currentSize, newSize);
+ remoteStorageReaderThreadPool.setMaximumPoolSize(newSize);
Review Comment:
The patch still fails when we try to decrease the thread count. Confirmed it with below unit test:
```java
@Test
void testUpdateRemoteStorageReaderThreads() {
assertEquals(10, remoteLogManager.readerThreadPoolSize());
remoteLogManager.resizeReaderThreadPool(6);
assertEquals(6, remoteLogManager.readerThreadPoolSize());
remoteLogManager.resizeReaderThreadPool(12);
assertEquals(12, remoteLogManager.readerThreadPoolSize());
}
```
##########
core/src/test/scala/integration/kafka/admin/RemoteTopicCrudTest.scala:
##########
@@ -473,6 +473,40 @@ class RemoteTopicCrudTest extends IntegrationTestHarness {
verifyRemoteLogTopicConfigs(newProps)
}
+ @ParameterizedTest
+ @ValueSource(strings = Array("kraft"))
+ def testUpdateThreadPoolSize(quorum: String): Unit = {
Review Comment:
This test succeeds with and without the fix. The issue is while applying the dynamic configuration and this test does not cover that scenario.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org