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

Reply via email to