-
Notifications
You must be signed in to change notification settings - Fork 15.4k
KAFKA-10040; Make computing the PreferredReplicaImbalanceCount metric more efficient #8724
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -74,6 +74,7 @@ case class ReplicaAssignment private (replicas: Seq[Int], | |
| class ControllerContext { | ||
| val stats = new ControllerStats | ||
| var offlinePartitionCount = 0 | ||
| var preferredReplicaImbalanceCount = 0 | ||
| val shuttingDownBrokerIds = mutable.Set.empty[Int] | ||
| private val liveBrokers = mutable.Set.empty[Broker] | ||
| private val liveBrokerEpochs = mutable.Map.empty[Int, Long] | ||
|
|
@@ -82,7 +83,7 @@ class ControllerContext { | |
|
|
||
| val allTopics = mutable.Set.empty[String] | ||
| val partitionAssignments = mutable.Map.empty[String, mutable.Map[Int, ReplicaAssignment]] | ||
| val partitionLeadershipInfo = mutable.Map.empty[TopicPartition, LeaderIsrAndControllerEpoch] | ||
| private val partitionLeadershipInfo = mutable.Map.empty[TopicPartition, LeaderIsrAndControllerEpoch] | ||
| val partitionsBeingReassigned = mutable.Set.empty[TopicPartition] | ||
| val partitionStates = mutable.Map.empty[TopicPartition, PartitionState] | ||
| val replicaStates = mutable.Map.empty[PartitionAndReplica, ReplicaState] | ||
|
|
@@ -120,6 +121,7 @@ class ControllerContext { | |
| replicasOnOfflineDirs.clear() | ||
| partitionStates.clear() | ||
| offlinePartitionCount = 0 | ||
| preferredReplicaImbalanceCount = 0 | ||
| replicaStates.clear() | ||
| } | ||
|
|
||
|
|
@@ -137,7 +139,10 @@ class ControllerContext { | |
|
|
||
| def updatePartitionFullReplicaAssignment(topicPartition: TopicPartition, newAssignment: ReplicaAssignment): Unit = { | ||
| val assignments = partitionAssignments.getOrElseUpdate(topicPartition.topic, mutable.Map.empty) | ||
| assignments.put(topicPartition.partition, newAssignment) | ||
| val previous = assignments.put(topicPartition.partition, newAssignment) | ||
| val leadershipInfo = partitionLeadershipInfo.get(topicPartition) | ||
| updatePreferredReplicaImbalanceMetric(topicPartition, previous, leadershipInfo, | ||
| Some(newAssignment), leadershipInfo) | ||
| } | ||
|
|
||
| def partitionReplicaAssignmentForTopic(topic : String): Map[TopicPartition, Seq[Int]] = { | ||
|
|
@@ -281,16 +286,24 @@ class ControllerContext { | |
| } | ||
|
|
||
| def removeTopic(topic: String): Unit = { | ||
| // Metric is cleaned when the topic is queued up for deletion so | ||
| // we don't clean it twice. We clean it only if it is deleted | ||
| // directly. | ||
| if (!topicsToBeDeleted.contains(topic)) | ||
| cleanPreferredReplicaImbalanceMetric(topic) | ||
| topicsToBeDeleted -= topic | ||
| topicsWithDeletionStarted -= topic | ||
| allTopics -= topic | ||
| partitionAssignments.remove(topic) | ||
| partitionLeadershipInfo.foreach { case (topicPartition, _) => | ||
| if (topicPartition.topic == topic) | ||
| partitionLeadershipInfo.remove(topicPartition) | ||
| partitionAssignments.remove(topic).foreach { assignments => | ||
| assignments.keys.foreach { partition => | ||
| partitionLeadershipInfo.remove(new TopicPartition(topic, partition)) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| def queueTopicDeletion(topics: Set[String]): Unit = { | ||
| topicsToBeDeleted ++= topics | ||
| topics.foreach(cleanPreferredReplicaImbalanceMetric) | ||
| } | ||
|
|
||
| def beginTopicDeletion(topics: Set[String]): Unit = { | ||
|
|
@@ -391,6 +404,90 @@ class ControllerContext { | |
| partitionsForTopic(topic).filter { partition => states.contains(partitionState(partition)) }.toSet | ||
| } | ||
|
|
||
| def putPartitionLeadershipInfo(partition: TopicPartition, | ||
| leaderIsrAndControllerEpoch: LeaderIsrAndControllerEpoch): Unit = { | ||
| val previous = partitionLeadershipInfo.put(partition, leaderIsrAndControllerEpoch) | ||
| val replicaAssignment = partitionFullReplicaAssignment(partition) | ||
| updatePreferredReplicaImbalanceMetric(partition, Some(replicaAssignment), previous, | ||
| Some(replicaAssignment), Some(leaderIsrAndControllerEpoch)) | ||
| } | ||
|
|
||
| def partitionLeadershipInfo(partition: TopicPartition): Option[LeaderIsrAndControllerEpoch] = { | ||
| partitionLeadershipInfo.get(partition) | ||
| } | ||
|
|
||
| def partitionsLeadershipInfo(): Iterable[(TopicPartition, LeaderIsrAndControllerEpoch)] = { | ||
| partitionLeadershipInfo | ||
| } | ||
|
|
||
| def partitionsWithLeaders(): Set[TopicPartition] = { | ||
| partitionLeadershipInfo.keys.filter(tp => !isTopicQueuedUpForDeletion(tp.topic)).toSet | ||
| } | ||
|
|
||
| def partitionsWithOfflineLeader(): Set[TopicPartition] = { | ||
| partitionLeadershipInfo.filter { case (topicPartition, leaderIsrAndControllerEpoch) => | ||
| !isReplicaOnline(leaderIsrAndControllerEpoch.leaderAndIsr.leader, topicPartition) && | ||
| !isTopicQueuedUpForDeletion(topicPartition.topic) | ||
| }.keySet | ||
| } | ||
|
|
||
| def partitionLeadersOnBroker(brokerId: Int): Set[TopicPartition] = { | ||
| partitionLeadershipInfo.filter { case (topicPartition, leaderIsrAndControllerEpoch) => | ||
| !isTopicQueuedUpForDeletion(topicPartition.topic) && | ||
| leaderIsrAndControllerEpoch.leaderAndIsr.leader == brokerId && | ||
| partitionReplicaAssignment(topicPartition).size > 1 | ||
| }.keySet | ||
| } | ||
|
|
||
| def clearPartitionLeadershipInfo(): Unit = { | ||
| partitionLeadershipInfo.clear() | ||
| } | ||
|
|
||
| def partitionWithLeadersCount(): Int = { | ||
| partitionLeadershipInfo.size | ||
| } | ||
|
|
||
| private def updatePreferredReplicaImbalanceMetric(partition: TopicPartition, | ||
| oldReplicaAssignment: Option[ReplicaAssignment], | ||
| oldLeadershipInfo: Option[LeaderIsrAndControllerEpoch], | ||
| newReplicaAssignment: Option[ReplicaAssignment], | ||
| newLeadershipInfo: Option[LeaderIsrAndControllerEpoch]): Unit = { | ||
| if (!isTopicQueuedUpForDeletion(partition.topic)) { | ||
| oldReplicaAssignment.foreach { replicaAssignment => | ||
| oldLeadershipInfo.foreach { leadershipInfo => | ||
| if (!hasPreferredLeader(replicaAssignment, leadershipInfo)) | ||
| preferredReplicaImbalanceCount -= 1 | ||
| } | ||
| } | ||
|
|
||
| newReplicaAssignment.foreach { replicaAssignment => | ||
| newLeadershipInfo.foreach { leadershipInfo => | ||
| if (!hasPreferredLeader(replicaAssignment, leadershipInfo)) | ||
| preferredReplicaImbalanceCount += 1 | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private def cleanPreferredReplicaImbalanceMetric(topic: String): Unit = { | ||
| partitionAssignments.getOrElse(topic, mutable.Map.empty).foreach { case (partition, replicaAssignment) => | ||
| partitionLeadershipInfo.get(new TopicPartition(topic, partition)).foreach { leadershipInfo => | ||
| if (!hasPreferredLeader(replicaAssignment, leadershipInfo)) | ||
| preferredReplicaImbalanceCount -= 1 | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private def hasPreferredLeader(replicaAssignment: ReplicaAssignment, | ||
| leadershipInfo: LeaderIsrAndControllerEpoch): Boolean = { | ||
| val preferredReplica = replicaAssignment.replicas.head | ||
| if (replicaAssignment.isBeingReassigned && replicaAssignment.addingReplicas.contains(preferredReplica)) | ||
| // reassigning partitions are not counted as imbalanced until the new replica joins the ISR (completes reassignment) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not something to change here, but it's a little curious that we only do this when a reassignment is in progress. I'm not sure it's useful to distinguish that case as opposed to a broker that is catching up after a restart. If the preferred leader is not in the ISR, then we can't elect it anyway. Seems like it might be more useful if this metric captured the "eligible imbalance".
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with you. |
||
| !leadershipInfo.leaderAndIsr.isr.contains(preferredReplica) | ||
| else | ||
| leadershipInfo.leaderAndIsr.leader == preferredReplica | ||
| } | ||
|
|
||
| private def isValidReplicaStateTransition(replica: PartitionAndReplica, targetState: ReplicaState): Boolean = | ||
| targetState.validPreviousStates.contains(replicaStates(replica)) | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I think this exact argument doesn't need to be an Option
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, in
updatePartitionFullReplicaAssignment,partitionLeadershipInfo.get(topicPartition)returns anOptionso it is more convenient to keep it as anOption. Ok for you?