rondagostino commented on code in PR #14790:
URL: https://github.com/apache/kafka/pull/14790#discussion_r1418016489
##########
core/src/main/scala/kafka/server/ReplicaAlterLogDirsThread.scala:
##########
@@ -76,13 +83,69 @@ class ReplicaAlterLogDirsThread(name: String,
futureLog.updateHighWatermark(partitionData.highWatermark)
futureLog.maybeIncrementLogStartOffset(partitionData.logStartOffset,
LogStartOffsetIncrementReason.LeaderOffsetIncremented)
- if (partition.maybeReplaceCurrentWithFutureReplica())
- removePartitions(Set(topicPartition))
+ directoryEventHandler match {
+ case DirectoryEventHandler.NOOP =>
+ if (partition.maybeReplaceCurrentWithFutureReplica())
+ removePartitions(Set(topicPartition))
+ case _ =>
+ maybePromoteFutureReplica(topicPartition, partition)
+ }
quota.record(records.sizeInBytes)
logAppendInfo
}
+ override def removePartitions(topicPartitions: Set[TopicPartition]):
Map[TopicPartition, PartitionFetchState] = {
+ // Schedule assignment request to revert any queued request before
cancelling
+ for {
+ topicPartition <- topicPartitions
+ partitionState <- partitionAssignmentRequestState(topicPartition)
+ if partitionState == QUEUED
+ partition = replicaMgr.getPartitionOrException(topicPartition)
+ topicId <- partition.topicId
+ directoryId <- partition.logDirectoryId()
+ topicIdPartition = new TopicIdPartition(topicId,
topicPartition.partition())
+ } yield directoryEventHandler.handleAssignment(topicIdPartition,
directoryId)
+
+ super.removePartitions(topicPartitions)
+ }
+
+ // Visible for testing
+ def updatedAssignmentRequestState(topicPartition: TopicPartition)(state:
ReplicaAlterLogDirsThread.DirectoryEventRequestState): Unit = {
+ assignmentRequestStates.put(topicPartition, state)
+ }
+ private def maybePromoteFutureReplica(topicPartition: TopicPartition,
partition: Partition) = {
Review Comment:
nit: missing newline separator between defs
##########
core/src/main/scala/kafka/server/ReplicaAlterLogDirsThread.scala:
##########
@@ -76,13 +83,69 @@ class ReplicaAlterLogDirsThread(name: String,
futureLog.updateHighWatermark(partitionData.highWatermark)
futureLog.maybeIncrementLogStartOffset(partitionData.logStartOffset,
LogStartOffsetIncrementReason.LeaderOffsetIncremented)
- if (partition.maybeReplaceCurrentWithFutureReplica())
- removePartitions(Set(topicPartition))
+ directoryEventHandler match {
+ case DirectoryEventHandler.NOOP =>
+ if (partition.maybeReplaceCurrentWithFutureReplica())
+ removePartitions(Set(topicPartition))
+ case _ =>
+ maybePromoteFutureReplica(topicPartition, partition)
+ }
quota.record(records.sizeInBytes)
logAppendInfo
}
+ override def removePartitions(topicPartitions: Set[TopicPartition]):
Map[TopicPartition, PartitionFetchState] = {
+ // Schedule assignment request to revert any queued request before
cancelling
+ for {
+ topicPartition <- topicPartitions
+ partitionState <- partitionAssignmentRequestState(topicPartition)
+ if partitionState == QUEUED
+ partition = replicaMgr.getPartitionOrException(topicPartition)
+ topicId <- partition.topicId
+ directoryId <- partition.logDirectoryId()
+ topicIdPartition = new TopicIdPartition(topicId,
topicPartition.partition())
+ } yield directoryEventHandler.handleAssignment(topicIdPartition,
directoryId)
Review Comment:
Seems odd to use `yield` when `directoryEventHandler.handleAssignment()` is
a void method. Is there a reason to use `yield`?
##########
core/src/test/scala/unit/kafka/server/ReplicaAlterLogDirsThreadTest.scala:
##########
@@ -268,6 +268,200 @@ class ReplicaAlterLogDirsThreadTest {
assertEquals(0, thread.partitionCount)
}
+ def updateAssignmentRequestState(thread: ReplicaAlterLogDirsThread,
partitionId:Int, newState:
ReplicaAlterLogDirsThread.DirectoryEventRequestState) = {
+ topicNames.get(topicId).map(topicName => {
+ thread.updatedAssignmentRequestState(new TopicPartition(topicName,
partitionId))(newState)
+ })
+ }
+ @Test
Review Comment:
nit: missing newline separator
##########
core/src/main/scala/kafka/server/ReplicaAlterLogDirsThread.scala:
##########
@@ -76,13 +83,69 @@ class ReplicaAlterLogDirsThread(name: String,
futureLog.updateHighWatermark(partitionData.highWatermark)
futureLog.maybeIncrementLogStartOffset(partitionData.logStartOffset,
LogStartOffsetIncrementReason.LeaderOffsetIncremented)
- if (partition.maybeReplaceCurrentWithFutureReplica())
- removePartitions(Set(topicPartition))
+ directoryEventHandler match {
+ case DirectoryEventHandler.NOOP =>
+ if (partition.maybeReplaceCurrentWithFutureReplica())
+ removePartitions(Set(topicPartition))
+ case _ =>
+ maybePromoteFutureReplica(topicPartition, partition)
+ }
quota.record(records.sizeInBytes)
logAppendInfo
}
+ override def removePartitions(topicPartitions: Set[TopicPartition]):
Map[TopicPartition, PartitionFetchState] = {
+ // Schedule assignment request to revert any queued request before
cancelling
+ for {
+ topicPartition <- topicPartitions
+ partitionState <- partitionAssignmentRequestState(topicPartition)
+ if partitionState == QUEUED
+ partition = replicaMgr.getPartitionOrException(topicPartition)
+ topicId <- partition.topicId
+ directoryId <- partition.logDirectoryId()
+ topicIdPartition = new TopicIdPartition(topicId,
topicPartition.partition())
+ } yield directoryEventHandler.handleAssignment(topicIdPartition,
directoryId)
+
+ super.removePartitions(topicPartitions)
+ }
+
+ // Visible for testing
+ def updatedAssignmentRequestState(topicPartition: TopicPartition)(state:
ReplicaAlterLogDirsThread.DirectoryEventRequestState): Unit = {
Review Comment:
`s/def/private[server] def/`
--
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-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
