You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2021/10/22 09:55:47 UTC

[GitHub] [kafka] JoeCqupt opened a new pull request #11427: MINOR: fix partition state change error msg

JoeCqupt opened a new pull request #11427:
URL: https://github.com/apache/kafka/pull/11427


   MINOR: fix partition state change error msg
   
   previous error msg replicas always empty. it is useless for troubleshooting
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


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

[GitHub] [kafka] dajac merged pull request #11427: MINOR: fix partition state change error msg

Posted by GitBox <gi...@apache.org>.
dajac merged pull request #11427:
URL: https://github.com/apache/kafka/pull/11427


   


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

[GitHub] [kafka] JoeCqupt commented on a change in pull request #11427: MINOR: fix partition state change error msg

Posted by GitBox <gi...@apache.org>.
JoeCqupt commented on a change in pull request #11427:
URL: https://github.com/apache/kafka/pull/11427#discussion_r737441595



##########
File path: core/src/main/scala/kafka/controller/PartitionStateMachine.scala
##########
@@ -269,17 +269,17 @@ class ZkPartitionStateMachine(config: KafkaConfig,
    */
   private def initializeLeaderAndIsrForPartitions(partitions: Seq[TopicPartition]): Seq[TopicPartition] = {
     val successfulInitializations = mutable.Buffer.empty[TopicPartition]
-    val replicasPerPartition = partitions.map(partition => partition -> controllerContext.partitionReplicaAssignment(partition))
+    val replicasPerPartition = partitions.map(partition => partition -> controllerContext.partitionReplicaAssignment(partition)).toMap
     val liveReplicasPerPartition = replicasPerPartition.map { case (partition, replicas) =>
         val liveReplicasForPartition = replicas.filter(replica => controllerContext.isReplicaOnline(replica, partition))
         partition -> liveReplicasForPartition
     }
     val (partitionsWithoutLiveReplicas, partitionsWithLiveReplicas) = liveReplicasPerPartition.partition { case (_, liveReplicas) => liveReplicas.isEmpty }
 
-    partitionsWithoutLiveReplicas.foreach { case (partition, replicas) =>
+    partitionsWithoutLiveReplicas.foreach { case (partition, _) =>
       val failMsg = s"Controller $controllerId epoch ${controllerContext.epoch} encountered error during state change of " +
         s"partition $partition from New to Online, assigned replicas are " +
-        s"[${replicas.mkString(",")}], live brokers are [${controllerContext.liveBrokerIds}]. No assigned " +
+        s"[${replicasPerPartition(partition).mkString(",")}], live brokers are [${controllerContext.liveBrokerIds}]. No assigned " +

Review comment:
       i agree with you. i have modified it




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

[GitHub] [kafka] JoeCqupt commented on pull request #11427: MINOR: fix partition state change error msg

Posted by GitBox <gi...@apache.org>.
JoeCqupt commented on pull request #11427:
URL: https://github.com/apache/kafka/pull/11427#issuecomment-952564114


   Call for review @dajac 


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

[GitHub] [kafka] dajac commented on a change in pull request #11427: MINOR: fix partition state change error msg

Posted by GitBox <gi...@apache.org>.
dajac commented on a change in pull request #11427:
URL: https://github.com/apache/kafka/pull/11427#discussion_r737185965



##########
File path: core/src/main/scala/kafka/controller/PartitionStateMachine.scala
##########
@@ -269,17 +269,17 @@ class ZkPartitionStateMachine(config: KafkaConfig,
    */
   private def initializeLeaderAndIsrForPartitions(partitions: Seq[TopicPartition]): Seq[TopicPartition] = {
     val successfulInitializations = mutable.Buffer.empty[TopicPartition]
-    val replicasPerPartition = partitions.map(partition => partition -> controllerContext.partitionReplicaAssignment(partition))
+    val replicasPerPartition = partitions.map(partition => partition -> controllerContext.partitionReplicaAssignment(partition)).toMap
     val liveReplicasPerPartition = replicasPerPartition.map { case (partition, replicas) =>
         val liveReplicasForPartition = replicas.filter(replica => controllerContext.isReplicaOnline(replica, partition))
         partition -> liveReplicasForPartition
     }
     val (partitionsWithoutLiveReplicas, partitionsWithLiveReplicas) = liveReplicasPerPartition.partition { case (_, liveReplicas) => liveReplicas.isEmpty }
 
-    partitionsWithoutLiveReplicas.foreach { case (partition, replicas) =>
+    partitionsWithoutLiveReplicas.foreach { case (partition, _) =>
       val failMsg = s"Controller $controllerId epoch ${controllerContext.epoch} encountered error during state change of " +
         s"partition $partition from New to Online, assigned replicas are " +
-        s"[${replicas.mkString(",")}], live brokers are [${controllerContext.liveBrokerIds}]. No assigned " +
+        s"[${replicasPerPartition(partition).mkString(",")}], live brokers are [${controllerContext.liveBrokerIds}]. No assigned " +

Review comment:
       I wonder if it would be simpler to just call `controllerContext.partitionReplicaAssignment(partition)` to get the `replicas` for the concerned partitions here instead of building a `Map` at L272. Have you considered this?




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

[GitHub] [kafka] mjsax commented on pull request #11427: MINOR: fix partition state change error msg

Posted by GitBox <gi...@apache.org>.
mjsax commented on pull request #11427:
URL: https://github.com/apache/kafka/pull/11427#issuecomment-949853217


   Call for review @dajac 


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