ijuma commented on a change in pull request #10003:
URL: https://github.com/apache/kafka/pull/10003#discussion_r568653073
##########
File path: core/src/main/scala/kafka/server/ReplicaManager.scala
##########
@@ -316,7 +324,7 @@ class ReplicaManager(val config: KafkaConfig,
private def maybeRemoveTopicMetrics(topic: String): Unit = {
val topicHasOnlinePartition = allPartitions.values.exists {
case HostedPartition.Online(partition) => topic == partition.topic
- case HostedPartition.None | HostedPartition.Offline => false
+ case _ => false
Review comment:
@hachikuji Have you considered having a sealed trait with `def
partition: Partition` and then have the relevant case classes inherit from
that? Then, when you want to get a partition, you match against that subtrait
while knowing that the partition is always available in those cases. I
suggested this offline to @rondagostino, but not sure if he didn't think it was
useful or didn't get around to it.
##########
File path: core/src/main/scala/kafka/server/ReplicaManager.scala
##########
@@ -316,7 +324,7 @@ class ReplicaManager(val config: KafkaConfig,
private def maybeRemoveTopicMetrics(topic: String): Unit = {
val topicHasOnlinePartition = allPartitions.values.exists {
case HostedPartition.Online(partition) => topic == partition.topic
- case HostedPartition.None | HostedPartition.Offline => false
+ case _ => false
Review comment:
After this change, we can probably keep `HostedPartition.None |
HostedPartition.Offline` as it was before, right?
##########
File path: core/src/main/scala/kafka/server/ReplicaManager.scala
##########
@@ -891,7 +944,7 @@ class ReplicaManager(val config: KafkaConfig,
def processFailedRecord(topicPartition: TopicPartition, t: Throwable) = {
val logStartOffset = getPartition(topicPartition) match {
case HostedPartition.Online(partition) => partition.logStartOffset
- case HostedPartition.None | HostedPartition.Offline => -1L
+ case _ => -1L
Review comment:
I think you want to call `onlinePartition` instead of `getPartition`.
----------------------------------------------------------------
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
