tinaselenge commented on code in PR #17524:
URL: https://github.com/apache/kafka/pull/17524#discussion_r1830721304
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -615,6 +615,42 @@ class PlaintextAdminIntegrationTest extends
BaseAdminIntegrationTest {
assertEquals(brokerStrs.mkString(","), nodeStrs.mkString(","))
}
+ @ParameterizedTest
+ @ValueSource(strings = Array("kraft"))
+ def testListNodesNotIncludingFencedBrokers(quorum: String): Unit = {
+ client = createAdminClient
+ val fencedBrokerId = brokers.last.config.brokerId
+ killBroker(fencedBrokerId)
+ Thread.sleep(10000) //sleep is needed to ensure the broker is fenced after
the shutdown
Review Comment:
This is my concern as well. It seems to take at least a few seconds for the
broker to become fenced after killing it and was hoping that 10000ms would be
more than enough (need to rerun it a number of times in Jenkins to confirm). I
understand there is a risk of it not being enough resulting in flakiness. I see
that some of the integration tests also use TestUtils.retry() to retry when
there is an assertion error, until the max timeout is reached. This still does
not guarantee, but I wonder if that's a better solution with a larger max
timeout, than the sleep.
--
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
