rondagostino commented on a change in pull request #11606:
URL: https://github.com/apache/kafka/pull/11606#discussion_r787045338



##########
File path: 
core/src/test/scala/unit/kafka/integration/KafkaServerTestHarness.scala
##########
@@ -253,4 +248,32 @@ abstract class KafkaServerTestHarness extends 
QuorumTestHarness {
     getController().kafkaController.controllerContext.topicNames.toMap
   }
 
+  private def createBrokers(startup: Boolean): Unit = {
+    // Add each broker to `servers` buffer as soon as it is created to ensure 
that brokers
+    // are shutdown cleanly in tearDown even if a subsequent broker fails to 
start
+    for (config <- configs) {
+      val broker = createBrokerFromConfig(config)
+      _brokers += broker
+      if (startup) {
+        broker.startup()

Review comment:
       It'd debatable if it is a problem.  It is not a problem if indeed the 
assumption is that the list of alive brokers is only valid if all brokers get 
created and are (now optionally) started -- if it isn't expected to be valid 
then putting in the effort to keep it valid doesn't strictly add anything.  
That being said, I agree the effort is not onerous to do so, and keeping things 
valid wherever possible is best in the long-run, so I'll push a commit to fix 
this.  Thanks for this feedback.




-- 
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