divijvaidya commented on code in PR #14667:
URL: https://github.com/apache/kafka/pull/14667#discussion_r1376157216
##########
core/src/main/scala/kafka/server/ReplicaManager.scala:
##########
@@ -630,13 +630,17 @@ class ReplicaManager(val config: KafkaConfig,
// Third delete the logs and checkpoint.
val errorMap = new mutable.HashMap[TopicPartition, Throwable]()
+ // `tieredEnabledPartitions` has exclude internal topics
+ val tieredEnabledPartitions = partitionsToStop.filter(sp =>
logManager.getLog(sp.topicPartition).exists(unifiedLog =>
unifiedLog.remoteLogEnabled()))
Review Comment:
This could be moved after the if {} logic in next line and merged after
`partitionsToStop.filterNot(sp => errorMap.contains(sp.topicPartition))`. This
way you won't call a contains() operation per partition.
Also:
1. we want to exclude reserved/internal topics as well.
2. we want to add back the warn that we talked about in
https://github.com/apache/kafka/pull/14329#discussion_r1315675510 since we no
longer expect stopPartitions to be called for non-tiered and reserved topic.
##########
core/src/main/scala/kafka/server/ReplicaManager.scala:
##########
@@ -630,13 +630,17 @@ class ReplicaManager(val config: KafkaConfig,
// Third delete the logs and checkpoint.
val errorMap = new mutable.HashMap[TopicPartition, Throwable]()
+ // `tieredEnabledPartitions` has exclude internal topics
+ val tieredEnabledPartitions = partitionsToStop.filter(sp =>
logManager.getLog(sp.topicPartition).exists(unifiedLog =>
unifiedLog.remoteLogEnabled()))
if (partitionsToDelete.nonEmpty) {
// Delete the logs and checkpoint.
logManager.asyncDelete(partitionsToDelete, isStray = false, (tp, e) =>
errorMap.put(tp, e))
}
remoteLogManager.foreach { rlm =>
// exclude the partitions with offline/error state
Review Comment:
this comment should be updated now
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
