showuon commented on code in PR #12405:
URL: https://github.com/apache/kafka/pull/12405#discussion_r922932963


##########
core/src/test/scala/unit/kafka/controller/ControllerContextTest.scala:
##########
@@ -203,4 +203,21 @@ class ControllerContextTest {
     context.removeTopic(tp3.topic)
     assertEquals(0, context.preferredReplicaImbalanceCount)
   }
+
+  @Test
+  def testPreferredReplicaImbalanceMetricOnConcurrentTopicDeletion(): Unit = {
+    context.updatePartitionFullReplicaAssignment(tp1, ReplicaAssignment(Seq(1, 
2, 3)))
+    context.updatePartitionFullReplicaAssignment(tp3, ReplicaAssignment(Seq(1, 
2, 3)))
+    assertEquals(0, context.preferredReplicaImbalanceCount)
+
+    context.queueTopicDeletion(Set(tp1.topic))
+    // All partitions in topic will be marked as Offline during deletion 
procedure
+    context.putPartitionLeadershipInfo(tp1, 
LeaderIsrAndControllerEpoch(LeaderAndIsr(LeaderAndIsr.NoLeader, List(1, 2, 3)), 
0))
+    assertEquals(0, context.preferredReplicaImbalanceCount)
+
+    // Initiate tp3's topic deletion before tp1's deletion completes.
+    // Since tp1's delete-topic ZK node still exists, 
context.queueTopicDeletion will be called with Set(tp1, tp3)

Review Comment:
   It's a little confusing we sometimes call `tp1's` deletion, sometimes call 
`tp3's topic` deletion. Could we create 2 variables for these 2 topics? ex: 
`topicA`, `topicB`? So, in this test case, we can make it much clear like this:
   ```
   context.queueTopicDeletion(Set(topicA))
   
   // Initiate topicB's deletion before topicA's deletion completes.
   ...
   ```



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