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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
