divijvaidya commented on code in PR #12669:
URL: https://github.com/apache/kafka/pull/12669#discussion_r978518387
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -169,8 +168,7 @@ class PlaintextAdminIntegrationTest extends
BaseAdminIntegrationTest {
waitForTopics(client, expectedPresent = topics, expectedMissing = List())
val controller = brokers.find(_.config.brokerId ==
brokers.flatMap(_.metadataCache.getControllerId).head).get
- controller.shutdown()
- controller.awaitShutdown()
+ killBroker(controller.config.brokerId)
Review Comment:
> We have already verified metadata propagation in the waitForTopics above
I think this assumption is not correct. `waitForTopics` sends a `metadata`
API call to a broker (not necessarily the controller). In such a situation, it
is not guaranteed that the broker will have the latest metadata in the metadata
cache. (e.g. maybe the broker listener has not updated it's metadata cache from
zk yet). Am I right in saying this?
> I feel like we could also just delete the test since it is probably
covered by every integration test which kills the controller.
The test is meant to validate that the metadata is durable even when
controller is replaced. Other tests in this class that use `killBroker` do not
necessarily kill the controller, hence, I believe that there is value in
keeping this test here.
I agree that we can change the name of the test. I have renamed it to
`testMetadataDurabilityOnControllerFailure`.
--
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
