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-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
