ahuang98 commented on code in PR #12479:
URL: https://github.com/apache/kafka/pull/12479#discussion_r940527628
##########
core/src/test/scala/integration/kafka/api/RackAwareAutoTopicCreationTest.scala:
##########
@@ -37,29 +43,57 @@ class RackAwareAutoTopicCreationTest extends
KafkaServerTestHarness with RackAwa
def generateConfigs =
(0 until numServers) map { node =>
- TestUtils.createBrokerConfig(node, zkConnect, enableControlledShutdown =
false, rack = Some((node / 2).toString))
+ TestUtils.createBrokerConfig(node, zkConnectOrNull,
enableControlledShutdown = false, rack = Some((node / 2).toString))
} map (KafkaConfig.fromProps(_, overridingProps))
private val topic = "topic"
- @Test
- def testAutoCreateTopic(): Unit = {
+ @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumName)
+ @ValueSource(strings = Array("zk", "kraft"))
+ def testAutoCreateTopic(quorum: String): Unit = {
val producer = TestUtils.createProducer(bootstrapServers())
+ val props = new Properties()
+ props.put(CommonClientConfigs.BOOTSTRAP_SERVERS_CONFIG, bootstrapServers())
+ val adminClient = Admin.create(props)
+
+ TestUtils.waitUntilTrue(
+ () => brokers.head.metadataCache.getAliveBrokers().size == numServers,
+ "Timed out waiting for all brokers to become unfenced")
Review Comment:
This check is to confirm that all KRaft brokers are alive before the
controller attempts to assign replicas, isn't it enough to just check that this
is the case from any broker? (In [zk
mode](https://github.com/apache/kafka/blob/d9b898b678158626bd2872bbfef883ca60a41c43/core/src/main/scala/kafka/admin/AdminUtils.scala#L103)
I actually didn't see exclusion of dead brokers in the replica assignment
logic, so this check seems unnecessary)
--
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
