showuon commented on code in PR #16284: URL: https://github.com/apache/kafka/pull/16284#discussion_r1634856945
########## core/src/test/scala/unit/kafka/integration/UncleanLeaderElectionTest.scala: ########## @@ -247,17 +258,18 @@ class UncleanLeaderElectionTest extends QuorumTestHarness { //remove any previous unclean election metric val uncleanLeaderElectionsPerSecGauge = getGauge("UncleanLeaderElectionsPerSec") @volatile var uncleanLeaderElectionsPerSec = uncleanLeaderElectionsPerSecGauge.count() - assert(uncleanLeaderElectionsPerSec == 0) + assertEquals(0, uncleanLeaderElectionsPerSec) // shutdown leader and then restart follower brokers.filter(_.config.brokerId == leaderId).map(shutdownBroker) val followerServer = brokers.find(_.config.brokerId == followerId).get followerServer.startup() - // verify that unclean election to non-ISR follower does not occur - awaitLeaderChange(brokers, topicPartition, expectedLeaderOpt = Some(leaderId)) + // verify that unclean election to non-ISR follower does not occur. + // That is, leader should be NO_LEADER(-1) and the ISR should has only old leaderId. + waitForNoLeaderAndIsrHasOldLeaderId(followerServer.replicaManager.metadataCache, leaderId) Review Comment: For the `awaitLeaderChange`, we verify the leader not change by checking the leader node info, which is already shutdown. That doesn't make sense. Changed it to check the alive node with the expected leader and ISR info. -- 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