[GitHub] [kafka] hachikuji commented on a diff in pull request #12138: MINOR: Followers should not have any remote replica states left over from previous leadership

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

hachikuji commented on code in PR #12138:
URL: https://github.com/apache/kafka/pull/12138#discussion_r874209109


##########
core/src/main/scala/kafka/cluster/Partition.scala:
##########
@@ -578,7 +579,8 @@ class Partition(val topicPartition: TopicPartition,
       // Updating the assignment and ISR state is safe if the partition epoch is
       // larger or equal to the current partition epoch.
       updateAssignmentAndIsr(
-        assignment = partitionState.replicas.asScala.map(_.toInt),
+        assignment = replicas,
+        followers = replicas.filter(_ != localBrokerId),

Review Comment:
   Why not push this computation down to `updateAssignmentAndIsr`?



##########
core/src/main/scala/kafka/cluster/Partition.scala:
##########
@@ -673,7 +675,8 @@ class Partition(val topicPartition: TopicPartition,
 
       updateAssignmentAndIsr(
         assignment = partitionState.replicas.asScala.iterator.map(_.toInt).toSeq,
-        isr = Set.empty[Int],
+        followers = Seq.empty,
+        isr = Set.empty,

Review Comment:
   Perhaps a separate issue, but any reason not to update ISR? 



##########
core/src/main/scala/kafka/cluster/Partition.scala:
##########
@@ -578,7 +579,8 @@ class Partition(val topicPartition: TopicPartition,
       // Updating the assignment and ISR state is safe if the partition epoch is
       // larger or equal to the current partition epoch.
       updateAssignmentAndIsr(
-        assignment = partitionState.replicas.asScala.map(_.toInt),
+        assignment = replicas,

Review Comment:
   Would it make sense to change `assignment` to `replicas`?



-- 
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-unsubscribe@kafka.apache.org

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