chia7712 commented on code in PR #12174: URL: https://github.com/apache/kafka/pull/12174#discussion_r1549916567
########## core/src/test/scala/unit/kafka/integration/KafkaServerTestHarness.scala: ########## @@ -260,9 +274,12 @@ abstract class KafkaServerTestHarness extends QuorumTestHarness { } def killBroker(index: Int): Unit = { Review Comment: maybe we should keep origin implementation since it expect to await shutdown. ########## core/src/test/java/kafka/test/junit/ZkClusterInvocationContext.java: ########## @@ -323,7 +324,7 @@ public void rollingBrokerRestart() { throw new IllegalStateException("Tried to restart brokers but the cluster has not been started!"); } for (int i = 0; i < clusterReference.get().brokerCount(); i++) { - clusterReference.get().killBroker(i); + clusterReference.get().killBroker(i, Duration.ofSeconds(5)); Review Comment: what is the purpose of this change? ########## core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala: ########## @@ -197,12 +198,11 @@ class ServerShutdownTest extends KafkaServerTestHarness { verifyNonDaemonThreadsStatus() } - @Disabled @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumName) @ValueSource(strings = Array("kraft")) def testCleanShutdownWithKRaftControllerUnavailable(quorum: String): Unit = { Review Comment: the shutdown with timeout is a kind of dirty shutdown so we should rename the test to `testDirtyShutdownWithKRaftControllerUnavailable` ########## core/src/test/scala/unit/kafka/integration/KafkaServerTestHarness.scala: ########## @@ -260,9 +274,12 @@ abstract class KafkaServerTestHarness extends QuorumTestHarness { } def killBroker(index: Int): Unit = { - if (alive(index)) { - _brokers(index).shutdown() - _brokers(index).awaitShutdown() + killBroker(index, Duration.ofSeconds(5)) + } + + def killBroker(index: Int, timeout: Duration): Unit = { Review Comment: we need to document the difference of this variety. -- 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