chia7712 commented on a change in pull request #10252: URL: https://github.com/apache/kafka/pull/10252#discussion_r588878150
########## File path: core/src/main/scala/kafka/server/metadata/MetadataPartitions.scala ########## @@ -171,25 +200,38 @@ class MetadataPartitionsBuilder(val brokerId: Int, val prevPartition = newPartitionMap.put(partition.partitionIndex, partition) if (partition.isReplicaFor(brokerId)) { _localChanged.add(partition) - } else if (prevPartition != null && prevPartition.isReplicaFor(brokerId)) { - _localRemoved.add(prevPartition) + } else if (prevPartition != null) { + maybeAddToLocalRemoved(prevPartition) } newNameMap.put(partition.topicName, newPartitionMap) } + private def maybeAddToLocalRemoved(partition: MetadataPartition): Unit = { + if (partition.isReplicaFor(brokerId)) { + val currentTopicId = newReverseIdMap.get(partition.topicName) Review comment: Thanks for nice explanation. I have another question for this check. It seems to me three collections in `prevPartitions` should be consistent. For example: a topic which exists one of collection should also exists in other two (vice versa). If above comment is right, why we need this `if-else`? Calling `prevPartitions.contains(partition.topicName)` appears to be enough? ---------------------------------------------------------------- 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: us...@infra.apache.org