[GitHub] [kafka] cmccabe commented on a change in pull request #10561: KAFKA-12686 AlterIsr and LeaderAndIsr race condition

[GitBox <gi...@apache.org>]

cmccabe commented on a change in pull request #10561:
URL: https://github.com/apache/kafka/pull/10561#discussion_r616115604



##########
File path: core/src/main/scala/kafka/cluster/Partition.scala
##########
@@ -1341,13 +1335,15 @@ class Partition(val topicPartition: TopicPartition,
     isrState = proposedIsrState
 
     if (!alterIsrManager.submit(alterIsrItem)) {
-      // If the ISR manager did not accept our update, we need to revert back to previous state
+      // If the ISR manager did not accept our update, we need to revert back to previous state.

Review comment:
       "previous state" seems like the wrong description of what we're reverting to here.  After all, we're NOT reverting back to the state that existed before the ISR was updated by whatever change invalidated our AlterIsrRequest.  "current state" seems more accurate.
   
   Also, this comment seems unnecessarily ZK-specific.  Can we mention "ChangePartitionRecord or LeaderAndIsrRequest" as candidates for altering the ISR?




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org