From cbf877f845dd975a9d6a57e3faeff10cde9eda94 Mon Sep 17 00:00:00 2001 From: Rajini Sivaram Date: Tue, 6 Oct 2020 10:53:46 +0100 Subject: [PATCH 01/11] KAFKA-10554; Perform follower truncation based on diverging epochs in Fetch response --- .../src/main/scala/kafka/api/ApiVersion.scala | 2 + .../kafka/server/AbstractFetcherManager.scala | 4 +- .../kafka/server/AbstractFetcherThread.scala | 76 +++++++--- .../scala/kafka/server/DelayedFetch.scala | 9 +- .../server/ReplicaAlterLogDirsThread.scala | 4 +- .../kafka/server/ReplicaFetcherThread.scala | 13 +- .../scala/kafka/server/ReplicaManager.scala | 26 +++- .../kafka/server/DelayedFetchTest.scala | 7 +- .../server/AbstractFetcherManagerTest.scala | 8 +- .../server/AbstractFetcherThreadTest.scala | 4 + .../ReplicaAlterLogDirsThreadTest.scala | 18 +-- .../server/ReplicaFetcherThreadTest.scala | 139 +++++++++++++++++- .../server/ReplicaManagerQuotasTest.scala | 2 +- .../kafka/server/ReplicaManagerTest.scala | 24 ++- ...ationProtocolAcceptanceWithIbp26Test.scala | 29 ++++ .../ReplicaFetcherThreadBenchmark.java | 4 +- 16 files changed, 315 insertions(+), 54 deletions(-) create mode 100644 core/src/test/scala/unit/kafka/server/epoch/EpochDrivenReplicationProtocolAcceptanceWithIbp26Test.scala diff --git a/core/src/main/scala/kafka/api/ApiVersion.scala b/core/src/main/scala/kafka/api/ApiVersion.scala index 16e6fb9b3d4df..fdebc27c60dc3 100644 --- a/core/src/main/scala/kafka/api/ApiVersion.scala +++ b/core/src/main/scala/kafka/api/ApiVersion.scala @@ -130,6 +130,8 @@ object ApiVersion { val latestVersion: ApiVersion = allVersions.last + def isTruncationOnFetchSupported(version: ApiVersion): Boolean = version >= KAFKA_2_7_IV1 + /** * Return the minimum `ApiVersion` that supports `RecordVersion`. */ diff --git a/core/src/main/scala/kafka/server/AbstractFetcherManager.scala b/core/src/main/scala/kafka/server/AbstractFetcherManager.scala index 5a350e9749bae..b956dfa97f2ba 100755 --- a/core/src/main/scala/kafka/server/AbstractFetcherManager.scala +++ b/core/src/main/scala/kafka/server/AbstractFetcherManager.scala @@ -143,7 +143,7 @@ abstract class AbstractFetcherManager[T <: AbstractFetcherThread](val name: Stri } val initialOffsetAndEpochs = initialFetchOffsets.map { case (tp, brokerAndInitOffset) => - tp -> OffsetAndEpoch(brokerAndInitOffset.initOffset, brokerAndInitOffset.currentLeaderEpoch) + tp -> OffsetAndEpoch(brokerAndInitOffset.initOffset, brokerAndInitOffset.currentLeaderEpoch, brokerAndInitOffset.lastFetchedEpoch) } addPartitionsToFetcherThread(fetcherThread, initialOffsetAndEpochs) @@ -227,6 +227,6 @@ class FailedPartitions { case class BrokerAndFetcherId(broker: BrokerEndPoint, fetcherId: Int) -case class InitialFetchState(leader: BrokerEndPoint, currentLeaderEpoch: Int, initOffset: Long) +case class InitialFetchState(leader: BrokerEndPoint, currentLeaderEpoch: Int, initOffset: Long, lastFetchedEpoch: Option[Int]) case class BrokerIdAndFetcherId(brokerId: Int, fetcherId: Int) diff --git a/core/src/main/scala/kafka/server/AbstractFetcherThread.scala b/core/src/main/scala/kafka/server/AbstractFetcherThread.scala index edaa6b71e5b25..c41d5a78a3a8d 100755 --- a/core/src/main/scala/kafka/server/AbstractFetcherThread.scala +++ b/core/src/main/scala/kafka/server/AbstractFetcherThread.scala @@ -101,6 +101,8 @@ abstract class AbstractFetcherThread(name: String, protected def isOffsetForLeaderEpochSupported: Boolean + protected def isTruncationOnFetchSupported: Boolean + override def shutdown(): Unit = { initiateShutdown() inLock(partitionMapLock) { @@ -225,6 +227,20 @@ abstract class AbstractFetcherThread(name: String, } } + private def truncateOnFetchResponse(responseData: Map[TopicPartition, FetchData]): Unit = { + val epochEndOffsets = responseData + .filter { case (tp, fetchData) => fetchData.error == Errors.NONE && fetchData.divergingEpoch.isPresent } + .map { case (tp, fetchData) => + val divergingEpoch = fetchData.divergingEpoch.get + tp -> new EpochEndOffset(Errors.NONE, divergingEpoch.epoch, divergingEpoch.endOffset) + }.toMap + inLock(partitionMapLock) { + val ResultWithPartitions(fetchOffsets, partitionsWithError) = maybeTruncateToEpochEndOffsets(epochEndOffsets, Map.empty) + handlePartitionsWithErrors(partitionsWithError, "truncateOnFetchResponse") + updateFetchOffsetAndMaybeMarkTruncationComplete(fetchOffsets) + } + } + // Visible for testing private[server] def truncateToHighWatermark(partitions: Set[TopicPartition]): Unit = inLock(partitionMapLock) { val fetchOffsets = mutable.HashMap.empty[TopicPartition, OffsetTruncationState] @@ -253,7 +269,7 @@ abstract class AbstractFetcherThread(name: String, leaderEpochOffset.error match { case Errors.NONE => val offsetTruncationState = getOffsetTruncationState(tp, leaderEpochOffset) - if(doTruncate(tp, offsetTruncationState)) + if (doTruncate(tp, offsetTruncationState)) fetchOffsets.put(tp, offsetTruncationState) case Errors.FENCED_LEADER_EPOCH => @@ -341,7 +357,9 @@ abstract class AbstractFetcherThread(name: String, // ReplicaDirAlterThread may have removed topicPartition from the partitionStates after processing the partition data if (validBytes > 0 && partitionStates.contains(topicPartition)) { // Update partitionStates only if there is no exception during processPartitionData - val newFetchState = PartitionFetchState(nextOffset, Some(lag), currentFetchState.currentLeaderEpoch, state = Fetching) + val newFetchState = PartitionFetchState(nextOffset, Some(lag), + currentFetchState.currentLeaderEpoch, state = Fetching, + Some(currentFetchState.currentLeaderEpoch)) partitionStates.updateAndMoveToEnd(topicPartition, newFetchState) fetcherStats.byteRate.mark(validBytes) } @@ -400,6 +418,8 @@ abstract class AbstractFetcherThread(name: String, } } + if (isTruncationOnFetchSupported) + truncateOnFetchResponse(responseData) if (partitionsWithError.nonEmpty) { handlePartitionsWithErrors(partitionsWithError, "processFetchRequest") } @@ -408,9 +428,12 @@ abstract class AbstractFetcherThread(name: String, def markPartitionsForTruncation(topicPartition: TopicPartition, truncationOffset: Long): Unit = { partitionMapLock.lockInterruptibly() try { + // It is safe to reset `lastFetchedEpoch` here since we don't expect diverging offsets + // immediately after truncation. Subsequent fetches will populate `lastFetchedEpoch`. Option(partitionStates.stateValue(topicPartition)).foreach { state => val newState = PartitionFetchState(math.min(truncationOffset, state.fetchOffset), - state.lag, state.currentLeaderEpoch, state.delay, state = Truncating) + state.lag, state.currentLeaderEpoch, state.delay, state = Truncating, + lastFetchedEpoch = None) partitionStates.updateAndMoveToEnd(topicPartition, newState) partitionMapCond.signalAll() } @@ -432,14 +455,22 @@ abstract class AbstractFetcherThread(name: String, failedPartitions.removeAll(initialFetchStates.keySet) initialFetchStates.forKeyValue { (tp, initialFetchState) => - // We can skip the truncation step iff the leader epoch matches the existing epoch + // For IBP 2.7 onwards, we can rely on truncation based on diverging data returned in fetch responses. + // For older versions, we can skip the truncation step iff the leader epoch matches the existing epoch val currentState = partitionStates.stateValue(tp) - val updatedState = if (currentState != null && currentState.currentLeaderEpoch == initialFetchState.leaderEpoch) { + val updatedState = if (initialFetchState.offset >= 0 && isTruncationOnFetchSupported && initialFetchState.lastFetchedEpoch.nonEmpty) { + if (currentState != null) + currentState + else + PartitionFetchState(initialFetchState.offset, None, initialFetchState.leaderEpoch, + state = Fetching, initialFetchState.lastFetchedEpoch) + } else if (currentState != null && (currentState.currentLeaderEpoch == initialFetchState.leaderEpoch)) { currentState } else if (initialFetchState.offset < 0) { fetchOffsetAndTruncate(tp, initialFetchState.leaderEpoch) } else { - PartitionFetchState(initialFetchState.offset, None, initialFetchState.leaderEpoch, state = Truncating) + PartitionFetchState(initialFetchState.offset, None, initialFetchState.leaderEpoch, + state = Truncating, lastFetchedEpoch = None) } partitionStates.updateAndMoveToEnd(tp, updatedState) } @@ -461,8 +492,9 @@ abstract class AbstractFetcherThread(name: String, val maybeTruncationComplete = fetchOffsets.get(topicPartition) match { case Some(offsetTruncationState) => val state = if (offsetTruncationState.truncationCompleted) Fetching else Truncating + // Resetting `lastFetchedEpoch` since we are truncating and don't expect diverging epoch in the next fetch PartitionFetchState(offsetTruncationState.offset, currentFetchState.lag, - currentFetchState.currentLeaderEpoch, currentFetchState.delay, state) + currentFetchState.currentLeaderEpoch, currentFetchState.delay, state, lastFetchedEpoch = None) case None => currentFetchState } (topicPartition, maybeTruncationComplete) @@ -515,7 +547,7 @@ abstract class AbstractFetcherThread(name: String, // get (leader epoch, end offset) pair that corresponds to the largest leader epoch // less than or equal to the requested epoch. endOffsetForEpoch(tp, leaderEpochOffset.leaderEpoch) match { - case Some(OffsetAndEpoch(followerEndOffset, followerEpoch)) => + case Some(OffsetAndEpoch(followerEndOffset, followerEpoch, _)) => if (followerEpoch != leaderEpochOffset.leaderEpoch) { // the follower does not know about the epoch that leader replied with // we truncate to the end offset of the largest epoch that is smaller than the @@ -596,7 +628,8 @@ abstract class AbstractFetcherThread(name: String, truncate(topicPartition, OffsetTruncationState(leaderEndOffset, truncationCompleted = true)) fetcherLagStats.getAndMaybePut(topicPartition).lag = 0 - PartitionFetchState(leaderEndOffset, Some(0), currentLeaderEpoch, state = Fetching) + PartitionFetchState(leaderEndOffset, Some(0), currentLeaderEpoch, + state = Fetching, lastFetchedEpoch = Some(currentLeaderEpoch)) } else { /** * If the leader's log end offset is greater than the follower's log end offset, there are two possibilities: @@ -629,7 +662,9 @@ abstract class AbstractFetcherThread(name: String, val initialLag = leaderEndOffset - offsetToFetch fetcherLagStats.getAndMaybePut(topicPartition).lag = initialLag - PartitionFetchState(offsetToFetch, Some(initialLag), currentLeaderEpoch, state = Fetching) + // We don't expect diverging epochs from the next fetch request, so resetting `lastFetchedEpoch` + PartitionFetchState(offsetToFetch, Some(initialLag), currentLeaderEpoch, + state = Fetching, lastFetchedEpoch = None) } } @@ -640,7 +675,8 @@ abstract class AbstractFetcherThread(name: String, Option(partitionStates.stateValue(partition)).foreach { currentFetchState => if (!currentFetchState.isDelayed) { partitionStates.updateAndMoveToEnd(partition, PartitionFetchState(currentFetchState.fetchOffset, - currentFetchState.lag, currentFetchState.currentLeaderEpoch, Some(new DelayedItem(delay)), currentFetchState.state)) + currentFetchState.lag, currentFetchState.currentLeaderEpoch, Some(new DelayedItem(delay)), + currentFetchState.state, currentFetchState.lastFetchedEpoch)) } } } @@ -673,7 +709,8 @@ abstract class AbstractFetcherThread(name: String, partitionStates.partitionStateMap.asScala.map { case (topicPartition, currentFetchState) => val initialFetchState = InitialFetchState(sourceBroker, currentLeaderEpoch = currentFetchState.currentLeaderEpoch, - initOffset = currentFetchState.fetchOffset) + initOffset = currentFetchState.fetchOffset, + currentFetchState.lastFetchedEpoch) topicPartition -> initialFetchState } } @@ -687,7 +724,6 @@ abstract class AbstractFetcherThread(name: String, MemoryRecords.readableRecords(buffer) } } - } object AbstractFetcherThread { @@ -769,8 +805,9 @@ case object Truncating extends ReplicaState case object Fetching extends ReplicaState object PartitionFetchState { - def apply(offset: Long, lag: Option[Long], currentLeaderEpoch: Int, state: ReplicaState): PartitionFetchState = { - PartitionFetchState(offset, lag, currentLeaderEpoch, None, state) + def apply(offset: Long, lag: Option[Long], currentLeaderEpoch: Int, state: ReplicaState, + lastFetchedEpoch: Option[Int]): PartitionFetchState = { + PartitionFetchState(offset, lag, currentLeaderEpoch, None, state, lastFetchedEpoch) } } @@ -786,7 +823,8 @@ case class PartitionFetchState(fetchOffset: Long, lag: Option[Long], currentLeaderEpoch: Int, delay: Option[DelayedItem], - state: ReplicaState) { + state: ReplicaState, + lastFetchedEpoch: Option[Int]) { def isReadyForFetch: Boolean = state == Fetching && !isDelayed @@ -799,6 +837,7 @@ case class PartitionFetchState(fetchOffset: Long, override def toString: String = { s"FetchState(fetchOffset=$fetchOffset" + s", currentLeaderEpoch=$currentLeaderEpoch" + + s", lastFetchedEpoch=$lastFetchedEpoch" + s", state=$state" + s", lag=$lag" + s", delay=${delay.map(_.delayMs).getOrElse(0)}ms" + @@ -813,8 +852,9 @@ case class OffsetTruncationState(offset: Long, truncationCompleted: Boolean) { override def toString: String = "offset:%d-truncationCompleted:%b".format(offset, truncationCompleted) } -case class OffsetAndEpoch(offset: Long, leaderEpoch: Int) { +case class OffsetAndEpoch(offset: Long, leaderEpoch: Int, lastFetchedEpoch: Option[Int] = None) { override def toString: String = { - s"(offset=$offset, leaderEpoch=$leaderEpoch)" + val lastFetched = lastFetchedEpoch.map(epoch => s", lastFetchedEpoch=$epoch").getOrElse("") + s"(offset=$offset, leaderEpoch=$leaderEpoch$lastFetched)" } } diff --git a/core/src/main/scala/kafka/server/DelayedFetch.scala b/core/src/main/scala/kafka/server/DelayedFetch.scala index fb07077d1725b..12065ec15d2ae 100644 --- a/core/src/main/scala/kafka/server/DelayedFetch.scala +++ b/core/src/main/scala/kafka/server/DelayedFetch.scala @@ -28,11 +28,12 @@ import org.apache.kafka.common.requests.FetchRequest.PartitionData import scala.collection._ -case class FetchPartitionStatus(startOffsetMetadata: LogOffsetMetadata, fetchInfo: PartitionData) { +case class FetchPartitionStatus(startOffsetMetadata: LogOffsetMetadata, fetchInfo: PartitionData, hasDivergingEpoch: Boolean) { override def toString: String = { "[startOffsetMetadata: " + startOffsetMetadata + ", fetchInfo: " + fetchInfo + + ", hasDivergingEpoch: " + hasDivergingEpoch + "]" } } @@ -98,6 +99,12 @@ class DelayedFetch(delayMs: Long, case FetchTxnCommitted => offsetSnapshot.lastStableOffset } + // Case H: Return diverging epoch in response to trigger truncation + if (fetchStatus.hasDivergingEpoch) { + debug(s"Satisfying fetch $fetchMetadata since it has diverging epoch requiring truncation for partition $topicPartition.") + return forceComplete() + } + // Go directly to the check for Case G if the message offsets are the same. If the log segment // has just rolled, then the high watermark offset will remain the same but be on the old segment, // which would incorrectly be seen as an instance of Case F. diff --git a/core/src/main/scala/kafka/server/ReplicaAlterLogDirsThread.scala b/core/src/main/scala/kafka/server/ReplicaAlterLogDirsThread.scala index 91b873ff17c18..e6580fa450546 100644 --- a/core/src/main/scala/kafka/server/ReplicaAlterLogDirsThread.scala +++ b/core/src/main/scala/kafka/server/ReplicaAlterLogDirsThread.scala @@ -20,7 +20,7 @@ package kafka.server import java.util import java.util.Optional -import kafka.api.Request +import kafka.api.{ApiVersion, Request} import kafka.cluster.BrokerEndPoint import kafka.log.{LeaderOffsetIncremented, LogAppendInfo} import kafka.server.AbstractFetcherThread.ReplicaFetch @@ -183,6 +183,8 @@ class ReplicaAlterLogDirsThread(name: String, override protected def isOffsetForLeaderEpochSupported: Boolean = true + override protected def isTruncationOnFetchSupported: Boolean = ApiVersion.isTruncationOnFetchSupported(brokerConfig.interBrokerProtocolVersion) + /** * Truncate the log for each partition based on current replica's returned epoch and offset. * diff --git a/core/src/main/scala/kafka/server/ReplicaFetcherThread.scala b/core/src/main/scala/kafka/server/ReplicaFetcherThread.scala index 92765c94c51ea..d60b47ba9281f 100644 --- a/core/src/main/scala/kafka/server/ReplicaFetcherThread.scala +++ b/core/src/main/scala/kafka/server/ReplicaFetcherThread.scala @@ -39,6 +39,7 @@ import org.apache.kafka.common.utils.{LogContext, Time} import scala.jdk.CollectionConverters._ import scala.collection.{Map, mutable} +import scala.compat.java8.OptionConverters._ class ReplicaFetcherThread(name: String, fetcherId: Int, @@ -104,6 +105,7 @@ class ReplicaFetcherThread(name: String, private val maxBytes = brokerConfig.replicaFetchResponseMaxBytes private val fetchSize = brokerConfig.replicaFetchMaxBytes private val brokerSupportsLeaderEpochRequest = brokerConfig.interBrokerProtocolVersion >= KAFKA_0_11_0_IV2 + private val brokerSupportsTruncationOnFetch = ApiVersion.isTruncationOnFetchSupported(brokerConfig.interBrokerProtocolVersion) val fetchSessionHandler = new FetchSessionHandler(logContext, sourceBroker.id) override protected def latestEpoch(topicPartition: TopicPartition): Option[Int] = { @@ -267,8 +269,16 @@ class ReplicaFetcherThread(name: String, if (fetchState.isReadyForFetch && !shouldFollowerThrottle(quota, fetchState, topicPartition)) { try { val logStartOffset = this.logStartOffset(topicPartition) + val lastFetchedEpoch = if (isTruncationOnFetchSupported) + fetchState.lastFetchedEpoch.map(_.asInstanceOf[Integer]).asJava + else + Optional.empty[Integer] builder.add(topicPartition, new FetchRequest.PartitionData( - fetchState.fetchOffset, logStartOffset, fetchSize, Optional.of(fetchState.currentLeaderEpoch))) + fetchState.fetchOffset, + logStartOffset, + fetchSize, + Optional.of(fetchState.currentLeaderEpoch), + lastFetchedEpoch)) } catch { case _: KafkaStorageException => // The replica has already been marked offline due to log directory failure and the original failure should have already been logged. @@ -347,6 +357,7 @@ class ReplicaFetcherThread(name: String, override def isOffsetForLeaderEpochSupported: Boolean = brokerSupportsLeaderEpochRequest + override def isTruncationOnFetchSupported: Boolean = brokerSupportsTruncationOnFetch /** * To avoid ISR thrashing, we only throttle a replica on the follower if it's in the throttled replica list, diff --git a/core/src/main/scala/kafka/server/ReplicaManager.scala b/core/src/main/scala/kafka/server/ReplicaManager.scala index b9487fe0a5ee4..1aaf78c2b8e26 100644 --- a/core/src/main/scala/kafka/server/ReplicaManager.scala +++ b/core/src/main/scala/kafka/server/ReplicaManager.scala @@ -769,8 +769,9 @@ class ReplicaManager(val config: KafkaConfig, val futureLog = futureLocalLogOrException(topicPartition) logManager.abortAndPauseCleaning(topicPartition) + val (fetchOffset, lastFetchedEpoch) = initialFetchOffsetAndEpoch(futureLog) val initialFetchState = InitialFetchState(BrokerEndPoint(config.brokerId, "localhost", -1), - partition.getLeaderEpoch, futureLog.highWatermark) + partition.getLeaderEpoch, fetchOffset, lastFetchedEpoch) replicaAlterLogDirsManager.addFetcherForPartitions(Map(topicPartition -> initialFetchState)) } @@ -1088,7 +1089,7 @@ class ReplicaManager(val config: KafkaConfig, fetchInfos.foreach { case (topicPartition, partitionData) => logReadResultMap.get(topicPartition).foreach(logReadResult => { val logOffsetMetadata = logReadResult.info.fetchOffsetMetadata - fetchPartitionStatus += (topicPartition -> FetchPartitionStatus(logOffsetMetadata, partitionData)) + fetchPartitionStatus += (topicPartition -> FetchPartitionStatus(logOffsetMetadata, partitionData, logReadResult.divergingEpoch.nonEmpty)) }) } val fetchMetadata: SFetchMetadata = SFetchMetadata(fetchMinBytes, fetchMaxBytes, hardMaxBytesLimit, @@ -1483,8 +1484,9 @@ class ReplicaManager(val config: KafkaConfig, // replica from source dir to destination dir logManager.abortAndPauseCleaning(topicPartition) + val (fetchOffset, lastFetchedEpoch) = initialFetchOffsetAndEpoch(log) futureReplicasAndInitialOffset.put(topicPartition, InitialFetchState(leader, - partition.getLeaderEpoch, log.highWatermark)) + partition.getLeaderEpoch, fetchOffset, lastFetchedEpoch)) } } } @@ -1667,8 +1669,9 @@ class ReplicaManager(val config: KafkaConfig, val partitionsToMakeFollowerWithLeaderAndOffset = partitionsToMakeFollower.map { partition => val leader = metadataCache.getAliveBrokers.find(_.id == partition.leaderReplicaIdOpt.get).get .brokerEndPoint(config.interBrokerListenerName) - val fetchOffset = partition.localLogOrException.highWatermark - partition.topicPartition -> InitialFetchState(leader, partition.getLeaderEpoch, fetchOffset) + val log = partition.localLogOrException + val (fetchOffset, lastFetchedEpoch) = initialFetchOffsetAndEpoch(log) + partition.topicPartition -> InitialFetchState(leader, partition.getLeaderEpoch, fetchOffset, lastFetchedEpoch) }.toMap replicaFetcherManager.addFetcherForPartitions(partitionsToMakeFollowerWithLeaderAndOffset) @@ -1691,6 +1694,19 @@ class ReplicaManager(val config: KafkaConfig, partitionsToMakeFollower } + /** + * From IBP 2.7 onwards, we send latest fetch epoch in the request and truncate if a + * diverging epoch is returned in the response, avoiding the need for a separate OffsetForLeaderEpoch + * request. We use log end offset as the next fetch offset and include the corresponding epoch. + */ + private def initialFetchOffsetAndEpoch(log: Log): (Long, Option[Int]) = { + val latestEpoch = log.latestEpoch + if (ApiVersion.isTruncationOnFetchSupported(config.interBrokerProtocolVersion) && latestEpoch.nonEmpty) + (log.logEndOffset, latestEpoch) + else + (log.highWatermark, None) + } + private def maybeShrinkIsr(): Unit = { trace("Evaluating ISR list of partitions to see which replicas can be removed from the ISR") diff --git a/core/src/test/scala/integration/kafka/server/DelayedFetchTest.scala b/core/src/test/scala/integration/kafka/server/DelayedFetchTest.scala index 5b9e056a18c63..f3365c3b5293a 100644 --- a/core/src/test/scala/integration/kafka/server/DelayedFetchTest.scala +++ b/core/src/test/scala/integration/kafka/server/DelayedFetchTest.scala @@ -45,7 +45,9 @@ class DelayedFetchTest extends EasyMockSupport { val fetchStatus = FetchPartitionStatus( startOffsetMetadata = LogOffsetMetadata(fetchOffset), - fetchInfo = new FetchRequest.PartitionData(fetchOffset, logStartOffset, maxBytes, currentLeaderEpoch)) + fetchInfo = new FetchRequest.PartitionData(fetchOffset, logStartOffset, maxBytes, currentLeaderEpoch), + hasDivergingEpoch = false + ) val fetchMetadata = buildFetchMetadata(replicaId, topicPartition, fetchStatus) var fetchResultOpt: Option[FetchPartitionData] = None @@ -93,7 +95,8 @@ class DelayedFetchTest extends EasyMockSupport { val fetchStatus = FetchPartitionStatus( startOffsetMetadata = LogOffsetMetadata(fetchOffset), - fetchInfo = new FetchRequest.PartitionData(fetchOffset, logStartOffset, maxBytes, currentLeaderEpoch)) + fetchInfo = new FetchRequest.PartitionData(fetchOffset, logStartOffset, maxBytes, currentLeaderEpoch), + hasDivergingEpoch = false) val fetchMetadata = buildFetchMetadata(replicaId, topicPartition, fetchStatus) var fetchResultOpt: Option[FetchPartitionData] = None diff --git a/core/src/test/scala/unit/kafka/server/AbstractFetcherManagerTest.scala b/core/src/test/scala/unit/kafka/server/AbstractFetcherManagerTest.scala index 6f07e4401856e..9ec56211819d8 100644 --- a/core/src/test/scala/unit/kafka/server/AbstractFetcherManagerTest.scala +++ b/core/src/test/scala/unit/kafka/server/AbstractFetcherManagerTest.scala @@ -54,13 +54,14 @@ class AbstractFetcherManagerTest { val initialFetchState = InitialFetchState( leader = new BrokerEndPoint(0, "localhost", 9092), currentLeaderEpoch = leaderEpoch, - initOffset = fetchOffset) + initOffset = fetchOffset, + lastFetchedEpoch = None) EasyMock.expect(fetcher.start()) EasyMock.expect(fetcher.addPartitions(Map(tp -> OffsetAndEpoch(fetchOffset, leaderEpoch)))) .andReturn(Set(tp)) EasyMock.expect(fetcher.fetchState(tp)) - .andReturn(Some(PartitionFetchState(fetchOffset, None, leaderEpoch, Truncating))) + .andReturn(Some(PartitionFetchState(fetchOffset, None, leaderEpoch, Truncating, lastFetchedEpoch = None))) EasyMock.expect(fetcher.removePartitions(Set(tp))) EasyMock.expect(fetcher.fetchState(tp)).andReturn(None) EasyMock.replay(fetcher) @@ -113,7 +114,8 @@ class AbstractFetcherManagerTest { val initialFetchState = InitialFetchState( leader = new BrokerEndPoint(0, "localhost", 9092), currentLeaderEpoch = leaderEpoch, - initOffset = fetchOffset) + initOffset = fetchOffset, + lastFetchedEpoch = None) EasyMock.expect(fetcher.start()) EasyMock.expect(fetcher.addPartitions(Map(tp -> OffsetAndEpoch(fetchOffset, leaderEpoch)))) diff --git a/core/src/test/scala/unit/kafka/server/AbstractFetcherThreadTest.scala b/core/src/test/scala/unit/kafka/server/AbstractFetcherThreadTest.scala index 4a5633d1009ab..8680e804fd59c 100644 --- a/core/src/test/scala/unit/kafka/server/AbstractFetcherThreadTest.scala +++ b/core/src/test/scala/unit/kafka/server/AbstractFetcherThreadTest.scala @@ -308,6 +308,8 @@ class AbstractFetcherThreadTest { throw new UnsupportedOperationException override protected def isOffsetForLeaderEpochSupported: Boolean = false + + override protected def isTruncationOnFetchSupported: Boolean = false } val replicaLog = Seq( @@ -981,6 +983,8 @@ class AbstractFetcherThreadTest { override protected def isOffsetForLeaderEpochSupported: Boolean = true + override protected def isTruncationOnFetchSupported: Boolean = false + override def fetchFromLeader(fetchRequest: FetchRequest.Builder): Map[TopicPartition, FetchData] = { fetchRequest.fetchData.asScala.map { case (partition, fetchData) => val leaderState = leaderPartitionState(partition) diff --git a/core/src/test/scala/unit/kafka/server/ReplicaAlterLogDirsThreadTest.scala b/core/src/test/scala/unit/kafka/server/ReplicaAlterLogDirsThreadTest.scala index 2d45b73815913..421f7e20a03fa 100644 --- a/core/src/test/scala/unit/kafka/server/ReplicaAlterLogDirsThreadTest.scala +++ b/core/src/test/scala/unit/kafka/server/ReplicaAlterLogDirsThreadTest.scala @@ -463,7 +463,7 @@ class ReplicaAlterLogDirsThreadTest { expect(partition.truncateTo(capture(truncateToCapture), EasyMock.eq(true))).anyTimes() expect(futureLog.logEndOffset).andReturn(futureReplicaLEO).anyTimes() expect(futureLog.latestEpoch).andReturn(Some(leaderEpoch)).once() - expect(futureLog.latestEpoch).andReturn(Some(leaderEpoch - 2)).once() + expect(futureLog.latestEpoch).andReturn(Some(leaderEpoch - 2)).times(3) // leader replica truncated and fetched new offsets with new leader epoch expect(partition.lastOffsetForLeaderEpoch(Optional.of(1), leaderEpoch, fetchOnlyFromLeader = false)) @@ -730,8 +730,8 @@ class ReplicaAlterLogDirsThreadTest { t1p1 -> offsetAndEpoch(0L, leaderEpoch))) val ResultWithPartitions(fetchRequestOpt, partitionsWithError) = thread.buildFetch(Map( - t1p0 -> PartitionFetchState(150, None, leaderEpoch, None, state = Fetching), - t1p1 -> PartitionFetchState(160, None, leaderEpoch, None, state = Fetching))) + t1p0 -> PartitionFetchState(150, None, leaderEpoch, None, state = Fetching, lastFetchedEpoch = None), + t1p1 -> PartitionFetchState(160, None, leaderEpoch, None, state = Fetching, lastFetchedEpoch = None))) assertTrue(fetchRequestOpt.isDefined) val fetchRequest = fetchRequestOpt.get.fetchRequest @@ -782,8 +782,8 @@ class ReplicaAlterLogDirsThreadTest { // one partition is ready and one is truncating val ResultWithPartitions(fetchRequestOpt, partitionsWithError) = thread.buildFetch(Map( - t1p0 -> PartitionFetchState(150, None, leaderEpoch, state = Fetching), - t1p1 -> PartitionFetchState(160, None, leaderEpoch, state = Truncating))) + t1p0 -> PartitionFetchState(150, None, leaderEpoch, state = Fetching, lastFetchedEpoch = None), + t1p1 -> PartitionFetchState(160, None, leaderEpoch, state = Truncating, lastFetchedEpoch = None))) assertTrue(fetchRequestOpt.isDefined) val fetchRequest = fetchRequestOpt.get @@ -796,8 +796,8 @@ class ReplicaAlterLogDirsThreadTest { // one partition is ready and one is delayed val ResultWithPartitions(fetchRequest2Opt, partitionsWithError2) = thread.buildFetch(Map( - t1p0 -> PartitionFetchState(140, None, leaderEpoch, state = Fetching), - t1p1 -> PartitionFetchState(160, None, leaderEpoch, delay = Some(new DelayedItem(5000)), state = Fetching))) + t1p0 -> PartitionFetchState(140, None, leaderEpoch, state = Fetching, lastFetchedEpoch = None), + t1p1 -> PartitionFetchState(160, None, leaderEpoch, delay = Some(new DelayedItem(5000)), state = Fetching, lastFetchedEpoch = None))) assertTrue(fetchRequest2Opt.isDefined) val fetchRequest2 = fetchRequest2Opt.get @@ -810,8 +810,8 @@ class ReplicaAlterLogDirsThreadTest { // both partitions are delayed val ResultWithPartitions(fetchRequest3Opt, partitionsWithError3) = thread.buildFetch(Map( - t1p0 -> PartitionFetchState(140, None, leaderEpoch, delay = Some(new DelayedItem(5000)), state = Fetching), - t1p1 -> PartitionFetchState(160, None, leaderEpoch, delay = Some(new DelayedItem(5000)), state = Fetching))) + t1p0 -> PartitionFetchState(140, None, leaderEpoch, delay = Some(new DelayedItem(5000)), state = Fetching, lastFetchedEpoch = None), + t1p1 -> PartitionFetchState(160, None, leaderEpoch, delay = Some(new DelayedItem(5000)), state = Fetching, lastFetchedEpoch = None))) assertTrue("Expected no fetch requests since all partitions are delayed", fetchRequest3Opt.isEmpty) assertFalse(partitionsWithError3.nonEmpty) } diff --git a/core/src/test/scala/unit/kafka/server/ReplicaFetcherThreadTest.scala b/core/src/test/scala/unit/kafka/server/ReplicaFetcherThreadTest.scala index 3fc5018f10d94..4e265c1bf8fbe 100644 --- a/core/src/test/scala/unit/kafka/server/ReplicaFetcherThreadTest.scala +++ b/core/src/test/scala/unit/kafka/server/ReplicaFetcherThreadTest.scala @@ -17,8 +17,10 @@ package kafka.server import java.nio.charset.StandardCharsets +import java.util import java.util.{Collections, Optional} +import kafka.api.{ApiVersion, KAFKA_2_6_IV0} import kafka.cluster.{BrokerEndPoint, Partition} import kafka.log.{Log, LogManager} import kafka.server.QuotaFactory.UnboundedQuota @@ -82,8 +84,19 @@ class ReplicaFetcherThreadTest { } @Test - def shouldFetchLeaderEpochRequestIfLastEpochDefinedForSomePartitions(): Unit = { - val config = KafkaConfig.fromProps(TestUtils.createBrokerConfig(1, "localhost:1234")) + def testFetchLeaderEpochRequestIfLastEpochDefinedForSomePartitions(): Unit = { + shouldFetchLeaderEpochRequestIfLastEpochDefinedForSomePartitions(ibp = ApiVersion.latestVersion) + } + + @Test + def testFetchLeaderEpochRequestIfLastEpochDefinedForSomePartitionsWithIbp26(): Unit = { + shouldFetchLeaderEpochRequestIfLastEpochDefinedForSomePartitions(KAFKA_2_6_IV0) + } + + private def shouldFetchLeaderEpochRequestIfLastEpochDefinedForSomePartitions(ibp: ApiVersion): Unit = { + val props = TestUtils.createBrokerConfig(1, "localhost:1234") + props.setProperty(KafkaConfig.InterBrokerProtocolVersionProp, ibp.version) + val config = KafkaConfig.fromProps(props) //Setup all dependencies val quota: ReplicationQuotaManager = createNiceMock(classOf[ReplicationQuotaManager]) @@ -102,6 +115,8 @@ class ReplicaFetcherThreadTest { expect(log.latestEpoch).andReturn(Some(leaderEpoch)).once() expect(log.latestEpoch).andReturn(Some(leaderEpoch)).once() expect(log.latestEpoch).andReturn(None).once() // t2p1 doesnt support epochs + if (ApiVersion.isTruncationOnFetchSupported(ibp)) + expect(log.latestEpoch).andReturn(Some(leaderEpoch)).times(2) // to set lastFetchedEpoch expect(log.endOffsetForEpoch(leaderEpoch)).andReturn( Some(OffsetAndEpoch(0, leaderEpoch))).anyTimes() expect(replicaManager.logManager).andReturn(logManager).anyTimes() @@ -123,7 +138,7 @@ class ReplicaFetcherThreadTest { val thread = new ReplicaFetcherThread("bob", 0, brokerEndPoint, config, failedPartitions, replicaManager, new Metrics(), new SystemTime(), quota, Some(mockNetwork)) - // topic 1 supports epoch, t2 doesn't + // topic 1 supports epoch, t2 doesn't. `lastFetchedEpoch` not set for any partition thread.addPartitions(Map( t1p0 -> offsetAndEpoch(0L), t1p1 -> offsetAndEpoch(0L), @@ -155,6 +170,122 @@ class ReplicaFetcherThreadTest { verify(logManager) } + @Test + def testFetchLeaderEpochRequestIfLastFetchedEpochDefinedForSomePartitions(): Unit = { + verifyFetchLeaderEpochRequestWithLastFetchedEpoch(ApiVersion.latestVersion, hasUnknownLastFetchedEpoch = true) + } + + @Test + def testFetchLeaderEpochRequestIfLastFetchedEpochDefinedForAllPartitions(): Unit = { + verifyFetchLeaderEpochRequestWithLastFetchedEpoch(ApiVersion.latestVersion, hasUnknownLastFetchedEpoch = false) + } + + @Test + def testFetchLeaderEpochRequestIfLastFetchedEpochDefinedForSomePartitionsIbp26(): Unit = { + verifyFetchLeaderEpochRequestWithLastFetchedEpoch(KAFKA_2_6_IV0, hasUnknownLastFetchedEpoch = true) + } + + @Test + def testFetchLeaderEpochRequestIfLastFetchedEpochDefinedForAllPartitionsIbp26(): Unit = { + verifyFetchLeaderEpochRequestWithLastFetchedEpoch(KAFKA_2_6_IV0, hasUnknownLastFetchedEpoch = false) + } + + private def verifyFetchLeaderEpochRequestWithLastFetchedEpoch(ibp: ApiVersion, hasUnknownLastFetchedEpoch: Boolean): Unit = { + val props = TestUtils.createBrokerConfig(1, "localhost:1234") + props.setProperty(KafkaConfig.InterBrokerProtocolVersionProp, ibp.version) + val config = KafkaConfig.fromProps(props) + + //Setup all dependencies + val quota: ReplicationQuotaManager = createNiceMock(classOf[ReplicationQuotaManager]) + val logManager: LogManager = createMock(classOf[LogManager]) + val replicaAlterLogDirsManager: ReplicaAlterLogDirsManager = createMock(classOf[ReplicaAlterLogDirsManager]) + val log: Log = createNiceMock(classOf[Log]) + val partition: Partition = createMock(classOf[Partition]) + val replicaManager: ReplicaManager = createMock(classOf[ReplicaManager]) + + val leaderEpoch = 5 + + //Stubs + expect(partition.localLogOrException).andReturn(log).anyTimes() + expect(log.logEndOffset).andReturn(0).anyTimes() + expect(log.highWatermark).andReturn(0).anyTimes() + expect(log.latestEpoch).andReturn(Some(leaderEpoch)).anyTimes() + expect(log.endOffsetForEpoch(leaderEpoch)).andReturn( + Some(OffsetAndEpoch(0, leaderEpoch))).anyTimes() + expect(replicaManager.logManager).andReturn(logManager).anyTimes() + expect(replicaManager.replicaAlterLogDirsManager).andReturn(replicaAlterLogDirsManager).anyTimes() + expect(replicaManager.brokerTopicStats).andReturn(mock(classOf[BrokerTopicStats])) + stub(partition, replicaManager, log) + + //Expectations + expect(partition.truncateTo(anyLong(), anyBoolean())).times(3) + + replay(replicaManager, logManager, quota, partition, log) + + //Define the offsets for the OffsetsForLeaderEpochResponse + val offsets: util.Map[TopicPartition, EpochEndOffset] = if (!ApiVersion.isTruncationOnFetchSupported(ibp)) + Map(t1p0 -> new EpochEndOffset(leaderEpoch, 1), + t1p1 -> new EpochEndOffset(leaderEpoch, 1), + t2p1 -> new EpochEndOffset(leaderEpoch, 1)).asJava + else if (hasUnknownLastFetchedEpoch) + Collections.singletonMap(t2p1, new EpochEndOffset(leaderEpoch, 1)) + else + Collections.emptyMap[TopicPartition, EpochEndOffset] + + //Create the fetcher thread + val mockNetwork = new ReplicaFetcherMockBlockingSend(offsets, brokerEndPoint, new SystemTime()) + + val thread = new ReplicaFetcherThread("bob", 0, brokerEndPoint, config, failedPartitions, replicaManager, new Metrics(), new SystemTime(), quota, Some(mockNetwork)) + + val lastFetchedEpoch = Some(4) + thread.addPartitions(Map( + t1p0 -> OffsetAndEpoch(0, leaderEpoch, lastFetchedEpoch), + t1p1 -> OffsetAndEpoch(0L, leaderEpoch, lastFetchedEpoch), + t2p1 -> OffsetAndEpoch(0L, leaderEpoch, if (hasUnknownLastFetchedEpoch) None else lastFetchedEpoch))) + + def verifyPartitionStates(fetching: Set[TopicPartition], + truncating: Set[TopicPartition]): Unit = { + for (tp <- List(t1p0, t1p1, t2p1)) { + assertTrue(thread.fetchState(tp).isDefined) + val fetchState = thread.fetchState(tp).get + + val shouldBeReadyForFetch = fetching.contains(tp) + assertEquals( + s"Partition $tp should${if (!shouldBeReadyForFetch) " NOT" else ""} be ready for fetching", + shouldBeReadyForFetch, fetchState.isReadyForFetch) + + val shouldBeTruncatingLog = truncating.contains(tp) + assertEquals( + s"Partition $tp should${if (!shouldBeTruncatingLog) " NOT" else ""} be truncating its log", + shouldBeTruncatingLog, fetchState.isTruncating) + + assertFalse(s"Partition $tp should not be delayed", fetchState.isDelayed) + } + } + + val truncating = if (!ApiVersion.isTruncationOnFetchSupported(ibp)) + Set(t1p0, t1p1, t2p1) + else if (hasUnknownLastFetchedEpoch) + Set(t2p1) + else + Set.empty[TopicPartition] + verifyPartitionStates(Set(t1p0, t1p1, t2p1) -- truncating, truncating) + //Loop 1 + thread.doWork() + val expectedEpochFetchCount = if (hasUnknownLastFetchedEpoch || !ApiVersion.isTruncationOnFetchSupported(ibp)) 1 else 0 + assertEquals(expectedEpochFetchCount, mockNetwork.epochFetchCount) + assertEquals(1, mockNetwork.fetchCount) + + verifyPartitionStates(Set(t1p0, t1p1, t2p1), Set.empty) + + //Loop 2 we should not fetch epochs + thread.doWork() + assertEquals(expectedEpochFetchCount, mockNetwork.epochFetchCount) + assertEquals(2, mockNetwork.fetchCount) + verifyPartitionStates(Set(t1p0, t1p1, t2p1), Set.empty) + verify(logManager) + } + /** * Assert that all partitions' states are as expected * @@ -541,7 +672,7 @@ class ReplicaFetcherThreadTest { expect(partition.truncateTo(capture(truncated), anyBoolean())).anyTimes() expect(partition.localLogOrException).andReturn(log).anyTimes() expect(log.highWatermark).andReturn(initialFetchOffset).anyTimes() - expect(log.latestEpoch).andReturn(Some(5)) + expect(log.latestEpoch).andReturn(Some(5)).times(2) expect(replicaManager.logManager).andReturn(logManager).anyTimes() expect(replicaManager.replicaAlterLogDirsManager).andReturn(replicaAlterLogDirsManager).anyTimes() expect(replicaManager.brokerTopicStats).andReturn(mock(classOf[BrokerTopicStats])) diff --git a/core/src/test/scala/unit/kafka/server/ReplicaManagerQuotasTest.scala b/core/src/test/scala/unit/kafka/server/ReplicaManagerQuotasTest.scala index 25ee84eb7b3f4..71bce3e9fe33b 100644 --- a/core/src/test/scala/unit/kafka/server/ReplicaManagerQuotasTest.scala +++ b/core/src/test/scala/unit/kafka/server/ReplicaManagerQuotasTest.scala @@ -177,7 +177,7 @@ class ReplicaManagerQuotasTest { val tp = new TopicPartition("t1", 0) val fetchPartitionStatus = FetchPartitionStatus(LogOffsetMetadata(messageOffset = 50L, segmentBaseOffset = 0L, - relativePositionInSegment = 250), new PartitionData(50, 0, 1, Optional.empty())) + relativePositionInSegment = 250), new PartitionData(50, 0, 1, Optional.empty()), hasDivergingEpoch = false) val fetchMetadata = FetchMetadata(fetchMinBytes = 1, fetchMaxBytes = 1000, hardMaxBytesLimit = true, diff --git a/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala b/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala index 761b58f47c8c3..7f2c8505eb560 100644 --- a/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala +++ b/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala @@ -781,12 +781,26 @@ class ReplicaManagerTest { } } - /** - * If a partition becomes a follower and the leader is unchanged it should check for truncation - * if the epoch has increased by more than one (which suggests it has missed an update) - */ @Test def testBecomeFollowerWhenLeaderIsUnchangedButMissedLeaderUpdate(): Unit = { + verifyBecomeFollowerWhenLeaderIsUnchangedButMissedLeaderUpdate(new Properties, expectTruncation = false) + } + + @Test + def testBecomeFollowerWhenLeaderIsUnchangedButMissedLeaderUpdateIbp26(): Unit = { + val extraProps = new Properties + extraProps.put(KafkaConfig.InterBrokerProtocolVersionProp, KAFKA_2_6_IV0.version) + verifyBecomeFollowerWhenLeaderIsUnchangedButMissedLeaderUpdate(extraProps, expectTruncation = true) + } + + /** + * If a partition becomes a follower and the leader is unchanged it should check for truncation + * if the epoch has increased by more than one (which suggests it has missed an update). For + * IBP version 2.7 onwards, we don't require this since we can truncate at any time based + * on diverging epochs returned in fetch responses. + */ + private def verifyBecomeFollowerWhenLeaderIsUnchangedButMissedLeaderUpdate(extraProps: Properties, + expectTruncation: Boolean): Unit = { val topicPartition = 0 val followerBrokerId = 0 val leaderBrokerId = 1 @@ -800,7 +814,7 @@ class ReplicaManagerTest { // Prepare the mocked components for the test val (replicaManager, mockLogMgr) = prepareReplicaManagerAndLogManager(new MockTimer(time), topicPartition, leaderEpoch + leaderEpochIncrement, followerBrokerId, leaderBrokerId, countDownLatch, - expectTruncation = true, localLogOffset = Some(10)) + expectTruncation = expectTruncation, localLogOffset = Some(10), extraProps = extraProps) // Initialize partition state to follower, with leader = 1, leaderEpoch = 1 val tp = new TopicPartition(topic, topicPartition) diff --git a/core/src/test/scala/unit/kafka/server/epoch/EpochDrivenReplicationProtocolAcceptanceWithIbp26Test.scala b/core/src/test/scala/unit/kafka/server/epoch/EpochDrivenReplicationProtocolAcceptanceWithIbp26Test.scala new file mode 100644 index 0000000000000..2ad4776bb2ca3 --- /dev/null +++ b/core/src/test/scala/unit/kafka/server/epoch/EpochDrivenReplicationProtocolAcceptanceWithIbp26Test.scala @@ -0,0 +1,29 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package kafka.server.epoch + +import kafka.api.KAFKA_2_6_IV0 + +/** + * With IBP 2.7 onwards, we truncate based on diverging epochs returned in fetch responses. + * EpochDrivenReplicationProtocolAcceptanceTest tests epochs with latest version. This test + * verifies that we handle older IBP versions with truncation on leader/follower change correctly. + */ +class EpochDrivenReplicationProtocolAcceptanceWithIbp26Test extends EpochDrivenReplicationProtocolAcceptanceTest { + override val apiVersion = KAFKA_2_6_IV0 +} diff --git a/jmh-benchmarks/src/main/java/org/apache/kafka/jmh/fetcher/ReplicaFetcherThreadBenchmark.java b/jmh-benchmarks/src/main/java/org/apache/kafka/jmh/fetcher/ReplicaFetcherThreadBenchmark.java index 6eeca1b00f910..0e6218a9bcf2b 100644 --- a/jmh-benchmarks/src/main/java/org/apache/kafka/jmh/fetcher/ReplicaFetcherThreadBenchmark.java +++ b/jmh-benchmarks/src/main/java/org/apache/kafka/jmh/fetcher/ReplicaFetcherThreadBenchmark.java @@ -162,7 +162,7 @@ public void setup() throws IOException { partition.makeFollower(partitionState, offsetCheckpoints); pool.put(tp, partition); - offsetAndEpochs.put(tp, new OffsetAndEpoch(0, 0)); + offsetAndEpochs.put(tp, new OffsetAndEpoch(0, 0, Option.empty())); BaseRecords fetched = new BaseRecords() { @Override public int sizeInBytes() { @@ -288,7 +288,7 @@ public void truncate(TopicPartition tp, OffsetTruncationState offsetTruncationSt @Override public Option endOffsetForEpoch(TopicPartition topicPartition, int epoch) { - return OptionConverters.toScala(Optional.of(new OffsetAndEpoch(0, 0))); + return OptionConverters.toScala(Optional.of(new OffsetAndEpoch(0, 0, Option.empty()))); } @Override From 88a64810464adb5387cc1ebb3e8cac664e81b295 Mon Sep 17 00:00:00 2001 From: Rajini Sivaram Date: Wed, 14 Oct 2020 12:22:57 +0100 Subject: [PATCH 02/11] Address review comments --- .../kafka/server/AbstractFetcherManager.scala | 8 +- .../kafka/server/AbstractFetcherThread.scala | 101 ++++++++++-------- .../scala/kafka/server/DelayedFetch.scala | 9 +- .../server/ReplicaAlterLogDirsManager.scala | 2 +- .../server/ReplicaAlterLogDirsThread.scala | 9 +- .../scala/kafka/server/ReplicaManager.scala | 2 +- .../kafka/server/DelayedFetchTest.scala | 7 +- .../DynamicBrokerReconfigurationTest.scala | 8 +- .../server/AbstractFetcherManagerTest.scala | 4 +- .../server/AbstractFetcherThreadTest.scala | 55 +++++----- .../ReplicaAlterLogDirsThreadTest.scala | 31 +++--- .../server/ReplicaFetcherThreadTest.scala | 35 +++--- .../server/ReplicaManagerQuotasTest.scala | 2 +- .../kafka/server/ReplicaManagerTest.scala | 4 +- .../ReplicaFetcherThreadBenchmark.java | 9 +- 15 files changed, 153 insertions(+), 133 deletions(-) diff --git a/core/src/main/scala/kafka/server/AbstractFetcherManager.scala b/core/src/main/scala/kafka/server/AbstractFetcherManager.scala index b956dfa97f2ba..483a58906d8df 100755 --- a/core/src/main/scala/kafka/server/AbstractFetcherManager.scala +++ b/core/src/main/scala/kafka/server/AbstractFetcherManager.scala @@ -142,17 +142,13 @@ abstract class AbstractFetcherManager[T <: AbstractFetcherThread](val name: Stri addAndStartFetcherThread(brokerAndFetcherId, brokerIdAndFetcherId) } - val initialOffsetAndEpochs = initialFetchOffsets.map { case (tp, brokerAndInitOffset) => - tp -> OffsetAndEpoch(brokerAndInitOffset.initOffset, brokerAndInitOffset.currentLeaderEpoch, brokerAndInitOffset.lastFetchedEpoch) - } - - addPartitionsToFetcherThread(fetcherThread, initialOffsetAndEpochs) + addPartitionsToFetcherThread(fetcherThread, initialFetchOffsets) } } } protected def addPartitionsToFetcherThread(fetcherThread: T, - initialOffsetAndEpochs: collection.Map[TopicPartition, OffsetAndEpoch]): Unit = { + initialOffsetAndEpochs: collection.Map[TopicPartition, InitialFetchState]): Unit = { fetcherThread.addPartitions(initialOffsetAndEpochs) info(s"Added fetcher to broker ${fetcherThread.sourceBroker.id} for partitions $initialOffsetAndEpochs") } diff --git a/core/src/main/scala/kafka/server/AbstractFetcherThread.scala b/core/src/main/scala/kafka/server/AbstractFetcherThread.scala index c41d5a78a3a8d..282cb8e3341c4 100755 --- a/core/src/main/scala/kafka/server/AbstractFetcherThread.scala +++ b/core/src/main/scala/kafka/server/AbstractFetcherThread.scala @@ -227,13 +227,7 @@ abstract class AbstractFetcherThread(name: String, } } - private def truncateOnFetchResponse(responseData: Map[TopicPartition, FetchData]): Unit = { - val epochEndOffsets = responseData - .filter { case (tp, fetchData) => fetchData.error == Errors.NONE && fetchData.divergingEpoch.isPresent } - .map { case (tp, fetchData) => - val divergingEpoch = fetchData.divergingEpoch.get - tp -> new EpochEndOffset(Errors.NONE, divergingEpoch.epoch, divergingEpoch.endOffset) - }.toMap + private def truncateOnFetchResponse(epochEndOffsets: Map[TopicPartition, EpochEndOffset]): Unit = { inLock(partitionMapLock) { val ResultWithPartitions(fetchOffsets, partitionsWithError) = maybeTruncateToEpochEndOffsets(epochEndOffsets, Map.empty) handlePartitionsWithErrors(partitionsWithError, "truncateOnFetchResponse") @@ -310,6 +304,7 @@ abstract class AbstractFetcherThread(name: String, private def processFetchRequest(sessionPartitions: util.Map[TopicPartition, FetchRequest.PartitionData], fetchRequest: FetchRequest.Builder): Unit = { val partitionsWithError = mutable.Set[TopicPartition]() + val divergingEndOffsets = mutable.Map.empty[TopicPartition, EpochEndOffset] var responseData: Map[TopicPartition, FetchData] = Map.empty try { @@ -364,6 +359,11 @@ abstract class AbstractFetcherThread(name: String, fetcherStats.byteRate.mark(validBytes) } } + if (isTruncationOnFetchSupported) { + partitionData.divergingEpoch.ifPresent { divergingEpoch => + divergingEndOffsets += topicPartition -> new EpochEndOffset(Errors.NONE, divergingEpoch.epoch, divergingEpoch.endOffset) + } + } } catch { case ime@( _: CorruptRecordException | _: InvalidRecordException) => // we log the error and continue. This ensures two things @@ -418,26 +418,31 @@ abstract class AbstractFetcherThread(name: String, } } - if (isTruncationOnFetchSupported) - truncateOnFetchResponse(responseData) + if (divergingEndOffsets.nonEmpty) + truncateOnFetchResponse(divergingEndOffsets) if (partitionsWithError.nonEmpty) { handlePartitionsWithErrors(partitionsWithError, "processFetchRequest") } } + /** + * This is used to mark partitions for truncation in ReplicaAlterLogDirsThread after leader + * offsets are known. From IBP 2.7 onwards, we track lastFetchedEpoch and perform truncation + * based on diverging offsets returned in fetch responses, so this step is not required. + */ def markPartitionsForTruncation(topicPartition: TopicPartition, truncationOffset: Long): Unit = { - partitionMapLock.lockInterruptibly() - try { - // It is safe to reset `lastFetchedEpoch` here since we don't expect diverging offsets - // immediately after truncation. Subsequent fetches will populate `lastFetchedEpoch`. - Option(partitionStates.stateValue(topicPartition)).foreach { state => - val newState = PartitionFetchState(math.min(truncationOffset, state.fetchOffset), - state.lag, state.currentLeaderEpoch, state.delay, state = Truncating, - lastFetchedEpoch = None) - partitionStates.updateAndMoveToEnd(topicPartition, newState) - partitionMapCond.signalAll() - } - } finally partitionMapLock.unlock() + if (!isTruncationOnFetchSupported) { + partitionMapLock.lockInterruptibly() + try { + Option(partitionStates.stateValue(topicPartition)).foreach { state => + val newState = PartitionFetchState(math.min(truncationOffset, state.fetchOffset), + state.lag, state.currentLeaderEpoch, state.delay, state = Truncating, + lastFetchedEpoch = None) + partitionStates.updateAndMoveToEnd(topicPartition, newState) + partitionMapCond.signalAll() + } + } finally partitionMapLock.unlock() + } } private def markPartitionFailed(topicPartition: TopicPartition): Unit = { @@ -449,29 +454,42 @@ abstract class AbstractFetcherThread(name: String, warn(s"Partition $topicPartition marked as failed") } - def addPartitions(initialFetchStates: Map[TopicPartition, OffsetAndEpoch]): Set[TopicPartition] = { + /** + * Returns initial partition fetch state based on current state and the provided `initialFetchState`. + * From IBP 2.7 onwards, we can rely on truncation based on diverging data returned in fetch responses. + * For older versions, we can skip the truncation step iff the leader epoch matches the existing epoch. + */ + private def partitionFetchState(tp: TopicPartition, initialFetchState: InitialFetchState, currentState: PartitionFetchState): PartitionFetchState = { + if (isTruncationOnFetchSupported && initialFetchState.initOffset >= 0 && initialFetchState.lastFetchedEpoch.nonEmpty) { + if (currentState == null) { + return PartitionFetchState(initialFetchState.initOffset, None, initialFetchState.currentLeaderEpoch, + state = Fetching, initialFetchState.lastFetchedEpoch) + } + // If we are in `Fetching` state can continue to fetch regardless of current leader epoch and truncate + // if necessary based on diverging epochs returned by the leader. If we are currently in Truncating state, + // fall through and handle based on current epoch. + if (currentState.state == Fetching) { + return currentState + } + } + if (currentState != null && currentState.currentLeaderEpoch == initialFetchState.currentLeaderEpoch) { + currentState + } else if (initialFetchState.initOffset < 0) { + fetchOffsetAndTruncate(tp, initialFetchState.currentLeaderEpoch) + } else { + PartitionFetchState(initialFetchState.initOffset, None, initialFetchState.currentLeaderEpoch, + state = Truncating, lastFetchedEpoch = None) + } + } + + def addPartitions(initialFetchStates: Map[TopicPartition, InitialFetchState]): Set[TopicPartition] = { partitionMapLock.lockInterruptibly() try { failedPartitions.removeAll(initialFetchStates.keySet) initialFetchStates.forKeyValue { (tp, initialFetchState) => - // For IBP 2.7 onwards, we can rely on truncation based on diverging data returned in fetch responses. - // For older versions, we can skip the truncation step iff the leader epoch matches the existing epoch val currentState = partitionStates.stateValue(tp) - val updatedState = if (initialFetchState.offset >= 0 && isTruncationOnFetchSupported && initialFetchState.lastFetchedEpoch.nonEmpty) { - if (currentState != null) - currentState - else - PartitionFetchState(initialFetchState.offset, None, initialFetchState.leaderEpoch, - state = Fetching, initialFetchState.lastFetchedEpoch) - } else if (currentState != null && (currentState.currentLeaderEpoch == initialFetchState.leaderEpoch)) { - currentState - } else if (initialFetchState.offset < 0) { - fetchOffsetAndTruncate(tp, initialFetchState.leaderEpoch) - } else { - PartitionFetchState(initialFetchState.offset, None, initialFetchState.leaderEpoch, - state = Truncating, lastFetchedEpoch = None) - } + val updatedState = partitionFetchState(tp, initialFetchState, currentState) partitionStates.updateAndMoveToEnd(tp, updatedState) } @@ -547,7 +565,7 @@ abstract class AbstractFetcherThread(name: String, // get (leader epoch, end offset) pair that corresponds to the largest leader epoch // less than or equal to the requested epoch. endOffsetForEpoch(tp, leaderEpochOffset.leaderEpoch) match { - case Some(OffsetAndEpoch(followerEndOffset, followerEpoch, _)) => + case Some(OffsetAndEpoch(followerEndOffset, followerEpoch)) => if (followerEpoch != leaderEpochOffset.leaderEpoch) { // the follower does not know about the epoch that leader replied with // we truncate to the end offset of the largest epoch that is smaller than the @@ -852,9 +870,8 @@ case class OffsetTruncationState(offset: Long, truncationCompleted: Boolean) { override def toString: String = "offset:%d-truncationCompleted:%b".format(offset, truncationCompleted) } -case class OffsetAndEpoch(offset: Long, leaderEpoch: Int, lastFetchedEpoch: Option[Int] = None) { +case class OffsetAndEpoch(offset: Long, leaderEpoch: Int) { override def toString: String = { - val lastFetched = lastFetchedEpoch.map(epoch => s", lastFetchedEpoch=$epoch").getOrElse("") - s"(offset=$offset, leaderEpoch=$leaderEpoch$lastFetched)" + s"(offset=$offset, leaderEpoch=$leaderEpoch)" } } diff --git a/core/src/main/scala/kafka/server/DelayedFetch.scala b/core/src/main/scala/kafka/server/DelayedFetch.scala index 12065ec15d2ae..fb07077d1725b 100644 --- a/core/src/main/scala/kafka/server/DelayedFetch.scala +++ b/core/src/main/scala/kafka/server/DelayedFetch.scala @@ -28,12 +28,11 @@ import org.apache.kafka.common.requests.FetchRequest.PartitionData import scala.collection._ -case class FetchPartitionStatus(startOffsetMetadata: LogOffsetMetadata, fetchInfo: PartitionData, hasDivergingEpoch: Boolean) { +case class FetchPartitionStatus(startOffsetMetadata: LogOffsetMetadata, fetchInfo: PartitionData) { override def toString: String = { "[startOffsetMetadata: " + startOffsetMetadata + ", fetchInfo: " + fetchInfo + - ", hasDivergingEpoch: " + hasDivergingEpoch + "]" } } @@ -99,12 +98,6 @@ class DelayedFetch(delayMs: Long, case FetchTxnCommitted => offsetSnapshot.lastStableOffset } - // Case H: Return diverging epoch in response to trigger truncation - if (fetchStatus.hasDivergingEpoch) { - debug(s"Satisfying fetch $fetchMetadata since it has diverging epoch requiring truncation for partition $topicPartition.") - return forceComplete() - } - // Go directly to the check for Case G if the message offsets are the same. If the log segment // has just rolled, then the high watermark offset will remain the same but be on the old segment, // which would incorrectly be seen as an instance of Case F. diff --git a/core/src/main/scala/kafka/server/ReplicaAlterLogDirsManager.scala b/core/src/main/scala/kafka/server/ReplicaAlterLogDirsManager.scala index 85eaeaa89caf3..b45a76620c74c 100644 --- a/core/src/main/scala/kafka/server/ReplicaAlterLogDirsManager.scala +++ b/core/src/main/scala/kafka/server/ReplicaAlterLogDirsManager.scala @@ -36,7 +36,7 @@ class ReplicaAlterLogDirsManager(brokerConfig: KafkaConfig, } override protected def addPartitionsToFetcherThread(fetcherThread: ReplicaAlterLogDirsThread, - initialOffsetAndEpochs: collection.Map[TopicPartition, OffsetAndEpoch]): Unit = { + initialOffsetAndEpochs: collection.Map[TopicPartition, InitialFetchState]): Unit = { val addedPartitions = fetcherThread.addPartitions(initialOffsetAndEpochs) val (addedInitialOffsets, notAddedInitialOffsets) = initialOffsetAndEpochs.partition { case (tp, _) => addedPartitions.contains(tp) diff --git a/core/src/main/scala/kafka/server/ReplicaAlterLogDirsThread.scala b/core/src/main/scala/kafka/server/ReplicaAlterLogDirsThread.scala index e6580fa450546..89c159b5ffac6 100644 --- a/core/src/main/scala/kafka/server/ReplicaAlterLogDirsThread.scala +++ b/core/src/main/scala/kafka/server/ReplicaAlterLogDirsThread.scala @@ -36,6 +36,7 @@ import org.apache.kafka.common.requests.{EpochEndOffset, FetchRequest, FetchResp import scala.jdk.CollectionConverters._ import scala.collection.{Map, Seq, Set, mutable} +import scala.compat.java8.OptionConverters._ class ReplicaAlterLogDirsThread(name: String, sourceBroker: BrokerEndPoint, @@ -131,7 +132,7 @@ class ReplicaAlterLogDirsThread(name: String, logAppendInfo } - override def addPartitions(initialFetchStates: Map[TopicPartition, OffsetAndEpoch]): Set[TopicPartition] = { + override def addPartitions(initialFetchStates: Map[TopicPartition, InitialFetchState]): Set[TopicPartition] = { partitionMapLock.lockInterruptibly() try { // It is possible that the log dir fetcher completed just before this call, so we @@ -250,8 +251,12 @@ class ReplicaAlterLogDirsThread(name: String, try { val logStartOffset = replicaMgr.futureLocalLogOrException(tp).logStartOffset + val lastFetchedEpoch = if (isTruncationOnFetchSupported) + fetchState.lastFetchedEpoch.map(_.asInstanceOf[Integer]).asJava + else + Optional.empty[Integer] requestMap.put(tp, new FetchRequest.PartitionData(fetchState.fetchOffset, logStartOffset, - fetchSize, Optional.of(fetchState.currentLeaderEpoch))) + fetchSize, Optional.of(fetchState.currentLeaderEpoch), lastFetchedEpoch)) } catch { case e: KafkaStorageException => debug(s"Failed to build fetch for $tp", e) diff --git a/core/src/main/scala/kafka/server/ReplicaManager.scala b/core/src/main/scala/kafka/server/ReplicaManager.scala index 1aaf78c2b8e26..9845d856cb293 100644 --- a/core/src/main/scala/kafka/server/ReplicaManager.scala +++ b/core/src/main/scala/kafka/server/ReplicaManager.scala @@ -1089,7 +1089,7 @@ class ReplicaManager(val config: KafkaConfig, fetchInfos.foreach { case (topicPartition, partitionData) => logReadResultMap.get(topicPartition).foreach(logReadResult => { val logOffsetMetadata = logReadResult.info.fetchOffsetMetadata - fetchPartitionStatus += (topicPartition -> FetchPartitionStatus(logOffsetMetadata, partitionData, logReadResult.divergingEpoch.nonEmpty)) + fetchPartitionStatus += (topicPartition -> FetchPartitionStatus(logOffsetMetadata, partitionData)) }) } val fetchMetadata: SFetchMetadata = SFetchMetadata(fetchMinBytes, fetchMaxBytes, hardMaxBytesLimit, diff --git a/core/src/test/scala/integration/kafka/server/DelayedFetchTest.scala b/core/src/test/scala/integration/kafka/server/DelayedFetchTest.scala index f3365c3b5293a..5b9e056a18c63 100644 --- a/core/src/test/scala/integration/kafka/server/DelayedFetchTest.scala +++ b/core/src/test/scala/integration/kafka/server/DelayedFetchTest.scala @@ -45,9 +45,7 @@ class DelayedFetchTest extends EasyMockSupport { val fetchStatus = FetchPartitionStatus( startOffsetMetadata = LogOffsetMetadata(fetchOffset), - fetchInfo = new FetchRequest.PartitionData(fetchOffset, logStartOffset, maxBytes, currentLeaderEpoch), - hasDivergingEpoch = false - ) + fetchInfo = new FetchRequest.PartitionData(fetchOffset, logStartOffset, maxBytes, currentLeaderEpoch)) val fetchMetadata = buildFetchMetadata(replicaId, topicPartition, fetchStatus) var fetchResultOpt: Option[FetchPartitionData] = None @@ -95,8 +93,7 @@ class DelayedFetchTest extends EasyMockSupport { val fetchStatus = FetchPartitionStatus( startOffsetMetadata = LogOffsetMetadata(fetchOffset), - fetchInfo = new FetchRequest.PartitionData(fetchOffset, logStartOffset, maxBytes, currentLeaderEpoch), - hasDivergingEpoch = false) + fetchInfo = new FetchRequest.PartitionData(fetchOffset, logStartOffset, maxBytes, currentLeaderEpoch)) val fetchMetadata = buildFetchMetadata(replicaId, topicPartition, fetchStatus) var fetchResultOpt: Option[FetchPartitionData] = None diff --git a/core/src/test/scala/integration/kafka/server/DynamicBrokerReconfigurationTest.scala b/core/src/test/scala/integration/kafka/server/DynamicBrokerReconfigurationTest.scala index dc55c13e6ef10..6abcde85d9b96 100644 --- a/core/src/test/scala/integration/kafka/server/DynamicBrokerReconfigurationTest.scala +++ b/core/src/test/scala/integration/kafka/server/DynamicBrokerReconfigurationTest.scala @@ -849,7 +849,13 @@ class DynamicBrokerReconfigurationTest extends ZooKeeperTestHarness with SaslSet val fetcherThreads = replicaFetcherManager.fetcherThreadMap.filter(_._2.fetchState(tp).isDefined) assertEquals(1, fetcherThreads.size) assertEquals(replicaFetcherManager.getFetcherId(tp), fetcherThreads.head._1.fetcherId) - assertEquals(Some(truncationOffset), fetcherThreads.head._2.fetchState(tp).map(_.fetchOffset)) + val thread = fetcherThreads.head._2 + if (!thread.isTruncationOnFetchSupported) { + assertEquals(Some(truncationOffset), thread.fetchState(tp).map(_.fetchOffset)) + assertEquals(Some(Truncating), thread.fetchState(tp).map(_.state)) + } else { + assertEquals(Some(Fetching), thread.fetchState(tp).map(_.state)) + } } } } diff --git a/core/src/test/scala/unit/kafka/server/AbstractFetcherManagerTest.scala b/core/src/test/scala/unit/kafka/server/AbstractFetcherManagerTest.scala index 9ec56211819d8..989a11191e033 100644 --- a/core/src/test/scala/unit/kafka/server/AbstractFetcherManagerTest.scala +++ b/core/src/test/scala/unit/kafka/server/AbstractFetcherManagerTest.scala @@ -58,7 +58,7 @@ class AbstractFetcherManagerTest { lastFetchedEpoch = None) EasyMock.expect(fetcher.start()) - EasyMock.expect(fetcher.addPartitions(Map(tp -> OffsetAndEpoch(fetchOffset, leaderEpoch)))) + EasyMock.expect(fetcher.addPartitions(Map(tp -> initialFetchState))) .andReturn(Set(tp)) EasyMock.expect(fetcher.fetchState(tp)) .andReturn(Some(PartitionFetchState(fetchOffset, None, leaderEpoch, Truncating, lastFetchedEpoch = None))) @@ -118,7 +118,7 @@ class AbstractFetcherManagerTest { lastFetchedEpoch = None) EasyMock.expect(fetcher.start()) - EasyMock.expect(fetcher.addPartitions(Map(tp -> OffsetAndEpoch(fetchOffset, leaderEpoch)))) + EasyMock.expect(fetcher.addPartitions(Map(tp -> initialFetchState))) .andReturn(Set(tp)) EasyMock.expect(fetcher.isThreadFailed).andReturn(true) EasyMock.replay(fetcher) diff --git a/core/src/test/scala/unit/kafka/server/AbstractFetcherThreadTest.scala b/core/src/test/scala/unit/kafka/server/AbstractFetcherThreadTest.scala index 8680e804fd59c..09846f3614c89 100644 --- a/core/src/test/scala/unit/kafka/server/AbstractFetcherThreadTest.scala +++ b/core/src/test/scala/unit/kafka/server/AbstractFetcherThreadTest.scala @@ -63,8 +63,9 @@ class AbstractFetcherThreadTest { .batches.asScala.head } - private def offsetAndEpoch(fetchOffset: Long, leaderEpoch: Int): OffsetAndEpoch = { - OffsetAndEpoch(offset = fetchOffset, leaderEpoch = leaderEpoch) + private def initialFetchState(fetchOffset: Long, leaderEpoch: Int): InitialFetchState = { + InitialFetchState(leader = new BrokerEndPoint(0, "localhost", 9092), + initOffset = fetchOffset, currentLeaderEpoch = leaderEpoch, lastFetchedEpoch = None) } @Test @@ -74,7 +75,7 @@ class AbstractFetcherThreadTest { // add one partition to create the consumer lag metric fetcher.setReplicaState(partition, MockFetcherThread.PartitionState(leaderEpoch = 0)) - fetcher.addPartitions(Map(partition -> offsetAndEpoch(0L, leaderEpoch = 0))) + fetcher.addPartitions(Map(partition -> initialFetchState(0L, leaderEpoch = 0))) fetcher.setLeaderState(partition, MockFetcherThread.PartitionState(leaderEpoch = 0)) fetcher.start() @@ -101,7 +102,7 @@ class AbstractFetcherThreadTest { // add one partition to create the consumer lag metric fetcher.setReplicaState(partition, MockFetcherThread.PartitionState(leaderEpoch = 0)) - fetcher.addPartitions(Map(partition -> offsetAndEpoch(0L, leaderEpoch = 0))) + fetcher.addPartitions(Map(partition -> initialFetchState(0L, leaderEpoch = 0))) fetcher.setLeaderState(partition, MockFetcherThread.PartitionState(leaderEpoch = 0)) fetcher.doWork() @@ -122,7 +123,7 @@ class AbstractFetcherThreadTest { val fetcher = new MockFetcherThread fetcher.setReplicaState(partition, MockFetcherThread.PartitionState(leaderEpoch = 0)) - fetcher.addPartitions(Map(partition -> offsetAndEpoch(0L, leaderEpoch = 0))) + fetcher.addPartitions(Map(partition -> initialFetchState(0L, leaderEpoch = 0))) val batch = mkBatch(baseOffset = 0L, leaderEpoch = 0, new SimpleRecord("a".getBytes), new SimpleRecord("b".getBytes)) @@ -142,7 +143,7 @@ class AbstractFetcherThreadTest { val fetcher = new MockFetcherThread fetcher.setReplicaState(partition, MockFetcherThread.PartitionState(leaderEpoch = 0)) - fetcher.addPartitions(Map(partition -> offsetAndEpoch(0L, leaderEpoch = 0))) + fetcher.addPartitions(Map(partition -> initialFetchState(0L, leaderEpoch = 0))) val batch = mkBatch(baseOffset = 0L, leaderEpoch = 1, new SimpleRecord("a".getBytes), new SimpleRecord("b".getBytes)) @@ -168,7 +169,7 @@ class AbstractFetcherThreadTest { val replicaState = MockFetcherThread.PartitionState(leaderEpoch = 0) fetcher.setReplicaState(partition, replicaState) - fetcher.addPartitions(Map(partition -> offsetAndEpoch(0L, leaderEpoch = 0))) + fetcher.addPartitions(Map(partition -> initialFetchState(0L, leaderEpoch = 0))) val batch = mkBatch(baseOffset = 0L, leaderEpoch = 0, new SimpleRecord("a".getBytes), @@ -199,7 +200,7 @@ class AbstractFetcherThreadTest { // The replica's leader epoch is ahead of the leader val replicaState = MockFetcherThread.PartitionState(leaderEpoch = 1) fetcher.setReplicaState(partition, replicaState) - fetcher.addPartitions(Map(partition -> offsetAndEpoch(0L, leaderEpoch = 1))) + fetcher.addPartitions(Map(partition -> initialFetchState(0L, leaderEpoch = 1))) val batch = mkBatch(baseOffset = 0L, leaderEpoch = 0, new SimpleRecord("a".getBytes)) val leaderState = MockFetcherThread.PartitionState(Seq(batch), leaderEpoch = 0, highWatermark = 2L) @@ -232,7 +233,7 @@ class AbstractFetcherThreadTest { val replicaState = MockFetcherThread.PartitionState(leaderEpoch = 1) fetcher.setReplicaState(partition, replicaState) - fetcher.addPartitions(Map(partition -> offsetAndEpoch(0L, leaderEpoch = 1))) + fetcher.addPartitions(Map(partition -> initialFetchState(0L, leaderEpoch = 1))) val leaderState = MockFetcherThread.PartitionState(Seq( mkBatch(baseOffset = 0L, leaderEpoch = 0, new SimpleRecord("a".getBytes)), @@ -273,7 +274,7 @@ class AbstractFetcherThreadTest { val replicaState = MockFetcherThread.PartitionState(replicaLog, leaderEpoch = 5, highWatermark = 0L) fetcher.setReplicaState(partition, replicaState) - fetcher.addPartitions(Map(partition -> offsetAndEpoch(3L, leaderEpoch = 5))) + fetcher.addPartitions(Map(partition -> initialFetchState(3L, leaderEpoch = 5))) val leaderLog = Seq( mkBatch(baseOffset = 0, leaderEpoch = 1, new SimpleRecord("a".getBytes)), @@ -319,7 +320,7 @@ class AbstractFetcherThreadTest { val replicaState = MockFetcherThread.PartitionState(replicaLog, leaderEpoch = 5, highWatermark) fetcher.setReplicaState(partition, replicaState) - fetcher.addPartitions(Map(partition -> offsetAndEpoch(highWatermark, leaderEpoch = 5))) + fetcher.addPartitions(Map(partition -> initialFetchState(highWatermark, leaderEpoch = 5))) fetcher.doWork() @@ -352,7 +353,7 @@ class AbstractFetcherThreadTest { val replicaState = MockFetcherThread.PartitionState(replicaLog, leaderEpoch = 5, highWatermark) fetcher.setReplicaState(partition, replicaState) - fetcher.addPartitions(Map(partition -> offsetAndEpoch(highWatermark, leaderEpoch = 5))) + fetcher.addPartitions(Map(partition -> initialFetchState(highWatermark, leaderEpoch = 5))) fetcher.doWork() @@ -381,7 +382,7 @@ class AbstractFetcherThreadTest { val replicaState = MockFetcherThread.PartitionState(replicaLog, leaderEpoch = 5, highWatermark) fetcher.setReplicaState(partition, replicaState) - fetcher.addPartitions(Map(partition -> offsetAndEpoch(highWatermark, leaderEpoch = 5))) + fetcher.addPartitions(Map(partition -> initialFetchState(highWatermark, leaderEpoch = 5))) fetcher.doWork() @@ -403,7 +404,7 @@ class AbstractFetcherThreadTest { val replicaState = MockFetcherThread.PartitionState(leaderEpoch = 5) fetcher.setReplicaState(partition, replicaState) - fetcher.addPartitions(Map(partition -> offsetAndEpoch(0L, leaderEpoch = 5))) + fetcher.addPartitions(Map(partition -> initialFetchState(0L, leaderEpoch = 5))) val leaderLog = Seq( mkBatch(baseOffset = 0, leaderEpoch = 1, new SimpleRecord("a".getBytes)), @@ -421,7 +422,7 @@ class AbstractFetcherThreadTest { assertEquals(1, truncations) // Add partitions again with the same epoch - fetcher.addPartitions(Map(partition -> offsetAndEpoch(3L, leaderEpoch = 5))) + fetcher.addPartitions(Map(partition -> initialFetchState(3L, leaderEpoch = 5))) // Verify we did not truncate fetcher.doWork() @@ -443,7 +444,7 @@ class AbstractFetcherThreadTest { val replicaState = MockFetcherThread.PartitionState(replicaLog, leaderEpoch = 4, highWatermark = 0L) fetcher.setReplicaState(partition, replicaState) - fetcher.addPartitions(Map(partition -> offsetAndEpoch(3L, leaderEpoch = 4))) + fetcher.addPartitions(Map(partition -> initialFetchState(3L, leaderEpoch = 4))) val leaderLog = Seq( mkBatch(baseOffset = 0, leaderEpoch = 0, new SimpleRecord("a".getBytes)), @@ -485,7 +486,7 @@ class AbstractFetcherThreadTest { val replicaLog = Seq() val replicaState = MockFetcherThread.PartitionState(replicaLog, leaderEpoch = 4, highWatermark = 0L) fetcher.setReplicaState(partition, replicaState) - fetcher.addPartitions(Map(partition -> offsetAndEpoch(0L, leaderEpoch = 4))) + fetcher.addPartitions(Map(partition -> initialFetchState(0L, leaderEpoch = 4))) val leaderLog = Seq( mkBatch(baseOffset = 1, leaderEpoch = 2, new SimpleRecord("b".getBytes)), @@ -512,7 +513,7 @@ class AbstractFetcherThreadTest { val replicaState = MockFetcherThread.PartitionState(replicaLog, leaderEpoch = 0, highWatermark = 0L) fetcher.setReplicaState(partition, replicaState) - fetcher.addPartitions(Map(partition -> offsetAndEpoch(3L, leaderEpoch = 0))) + fetcher.addPartitions(Map(partition -> initialFetchState(3L, leaderEpoch = 0))) val leaderLog = Seq( mkBatch(baseOffset = 2, leaderEpoch = 4, new SimpleRecord("c".getBytes))) @@ -554,7 +555,7 @@ class AbstractFetcherThreadTest { val replicaState = MockFetcherThread.PartitionState(replicaLog, leaderEpoch = 0, highWatermark = 0L) fetcher.setReplicaState(partition, replicaState) - fetcher.addPartitions(Map(partition -> offsetAndEpoch(3L, leaderEpoch = 0))) + fetcher.addPartitions(Map(partition -> initialFetchState(3L, leaderEpoch = 0))) val leaderLog = Seq( mkBatch(baseOffset = 2, leaderEpoch = 4, new SimpleRecord("c".getBytes))) @@ -596,7 +597,7 @@ class AbstractFetcherThreadTest { } fetcher.setReplicaState(partition, MockFetcherThread.PartitionState(leaderEpoch = 0)) - fetcher.addPartitions(Map(partition -> offsetAndEpoch(0L, leaderEpoch = 0))) + fetcher.addPartitions(Map(partition -> initialFetchState(0L, leaderEpoch = 0))) val batch = mkBatch(baseOffset = 0L, leaderEpoch = 0, new SimpleRecord("a".getBytes), new SimpleRecord("b".getBytes)) @@ -641,7 +642,7 @@ class AbstractFetcherThreadTest { // leader epoch changes while fetching epochs from leader removePartitions(Set(partition)) setReplicaState(partition, MockFetcherThread.PartitionState(leaderEpoch = nextLeaderEpochOnFollower)) - addPartitions(Map(partition -> offsetAndEpoch(0L, leaderEpoch = nextLeaderEpochOnFollower))) + addPartitions(Map(partition -> initialFetchState(0L, leaderEpoch = nextLeaderEpochOnFollower))) fetchEpochsFromLeaderOnce = true } fetchedEpochs @@ -649,7 +650,7 @@ class AbstractFetcherThreadTest { } fetcher.setReplicaState(partition, MockFetcherThread.PartitionState(leaderEpoch = initialLeaderEpochOnFollower)) - fetcher.addPartitions(Map(partition -> offsetAndEpoch(0L, leaderEpoch = initialLeaderEpochOnFollower))) + fetcher.addPartitions(Map(partition -> initialFetchState(0L, leaderEpoch = initialLeaderEpochOnFollower))) val leaderLog = Seq( mkBatch(baseOffset = 0, leaderEpoch = initialLeaderEpochOnFollower, new SimpleRecord("c".getBytes))) @@ -693,7 +694,7 @@ class AbstractFetcherThreadTest { } fetcher.setReplicaState(partition, MockFetcherThread.PartitionState(leaderEpoch = initialLeaderEpochOnFollower)) - fetcher.addPartitions(Map(partition -> offsetAndEpoch(0L, leaderEpoch = initialLeaderEpochOnFollower))) + fetcher.addPartitions(Map(partition -> initialFetchState(0L, leaderEpoch = initialLeaderEpochOnFollower))) val leaderLog = Seq( mkBatch(baseOffset = 0, leaderEpoch = initialLeaderEpochOnFollower, new SimpleRecord("c".getBytes))) @@ -727,7 +728,7 @@ class AbstractFetcherThreadTest { } fetcher.setReplicaState(partition, MockFetcherThread.PartitionState(leaderEpoch = 0)) - fetcher.addPartitions(Map(partition -> offsetAndEpoch(0L, leaderEpoch = 0))) + fetcher.addPartitions(Map(partition -> initialFetchState(0L, leaderEpoch = 0))) fetcher.setLeaderState(partition, MockFetcherThread.PartitionState(leaderEpoch = 0)) // first round of truncation should throw an exception @@ -767,11 +768,11 @@ class AbstractFetcherThreadTest { private def verifyFetcherThreadHandlingPartitionFailure(fetcher: MockFetcherThread): Unit = { fetcher.setReplicaState(partition1, MockFetcherThread.PartitionState(leaderEpoch = 0)) - fetcher.addPartitions(Map(partition1 -> offsetAndEpoch(0L, leaderEpoch = 0))) + fetcher.addPartitions(Map(partition1 -> initialFetchState(0L, leaderEpoch = 0))) fetcher.setLeaderState(partition1, MockFetcherThread.PartitionState(leaderEpoch = 0)) fetcher.setReplicaState(partition2, MockFetcherThread.PartitionState(leaderEpoch = 0)) - fetcher.addPartitions(Map(partition2 -> offsetAndEpoch(0L, leaderEpoch = 0))) + fetcher.addPartitions(Map(partition2 -> initialFetchState(0L, leaderEpoch = 0))) fetcher.setLeaderState(partition2, MockFetcherThread.PartitionState(leaderEpoch = 0)) // processing data fails for partition1 @@ -789,7 +790,7 @@ class AbstractFetcherThreadTest { // simulate a leader change fetcher.removePartitions(Set(partition1)) failedPartitions.removeAll(Set(partition1)) - fetcher.addPartitions(Map(partition1 -> offsetAndEpoch(0L, leaderEpoch = 1))) + fetcher.addPartitions(Map(partition1 -> initialFetchState(0L, leaderEpoch = 1))) // partition1 added back assertEquals(Some(Truncating), fetcher.fetchState(partition1).map(_.state)) diff --git a/core/src/test/scala/unit/kafka/server/ReplicaAlterLogDirsThreadTest.scala b/core/src/test/scala/unit/kafka/server/ReplicaAlterLogDirsThreadTest.scala index 421f7e20a03fa..8a41f675db56b 100644 --- a/core/src/test/scala/unit/kafka/server/ReplicaAlterLogDirsThreadTest.scala +++ b/core/src/test/scala/unit/kafka/server/ReplicaAlterLogDirsThreadTest.scala @@ -46,8 +46,9 @@ class ReplicaAlterLogDirsThreadTest { private val t1p1 = new TopicPartition("topic1", 1) private val failedPartitions = new FailedPartitions - private def offsetAndEpoch(fetchOffset: Long, leaderEpoch: Int = 1): OffsetAndEpoch = { - OffsetAndEpoch(offset = fetchOffset, leaderEpoch = leaderEpoch) + private def initialFetchState(fetchOffset: Long, leaderEpoch: Int = 1): InitialFetchState = { + InitialFetchState(leader = new BrokerEndPoint(0, "localhost", 9092), + initOffset = fetchOffset, currentLeaderEpoch = leaderEpoch, lastFetchedEpoch = None) } @Test @@ -70,7 +71,7 @@ class ReplicaAlterLogDirsThreadTest { quota = quotaManager, brokerTopicStats = new BrokerTopicStats) - val addedPartitions = thread.addPartitions(Map(t1p0 -> offsetAndEpoch(0L))) + val addedPartitions = thread.addPartitions(Map(t1p0 -> initialFetchState(0L))) assertEquals(Set.empty, addedPartitions) assertEquals(0, thread.partitionCount) assertEquals(None, thread.fetchState(t1p0)) @@ -131,7 +132,7 @@ class ReplicaAlterLogDirsThreadTest { brokerTopicStats = new BrokerTopicStats) // Initially we add the partition with an older epoch which results in an error - thread.addPartitions(Map(t1p0 -> offsetAndEpoch(fetchOffset = 0L, leaderEpoch - 1))) + thread.addPartitions(Map(t1p0 -> initialFetchState(fetchOffset = 0L, leaderEpoch - 1))) assertTrue(thread.fetchState(t1p0).isDefined) assertEquals(1, thread.partitionCount) @@ -142,7 +143,7 @@ class ReplicaAlterLogDirsThreadTest { assertEquals(0, thread.partitionCount) // Next we update the epoch and assert that we can continue - thread.addPartitions(Map(t1p0 -> offsetAndEpoch(fetchOffset = 0L, leaderEpoch))) + thread.addPartitions(Map(t1p0 -> initialFetchState(fetchOffset = 0L, leaderEpoch))) assertEquals(Some(leaderEpoch), thread.fetchState(t1p0).map(_.currentLeaderEpoch)) assertEquals(1, thread.partitionCount) @@ -221,7 +222,7 @@ class ReplicaAlterLogDirsThreadTest { quota = quotaManager, brokerTopicStats = new BrokerTopicStats) - thread.addPartitions(Map(t1p0 -> offsetAndEpoch(fetchOffset = 0L, leaderEpoch))) + thread.addPartitions(Map(t1p0 -> initialFetchState(fetchOffset = 0L, leaderEpoch))) assertTrue(thread.fetchState(t1p0).isDefined) assertEquals(1, thread.partitionCount) @@ -421,7 +422,7 @@ class ReplicaAlterLogDirsThreadTest { replicaMgr = replicaManager, quota = quotaManager, brokerTopicStats = null) - thread.addPartitions(Map(t1p0 -> offsetAndEpoch(0L), t1p1 -> offsetAndEpoch(0L))) + thread.addPartitions(Map(t1p0 -> initialFetchState(0L), t1p1 -> initialFetchState(0L))) //Run it thread.doWork() @@ -494,7 +495,7 @@ class ReplicaAlterLogDirsThreadTest { replicaMgr = replicaManager, quota = quotaManager, brokerTopicStats = null) - thread.addPartitions(Map(t1p0 -> offsetAndEpoch(0L))) + thread.addPartitions(Map(t1p0 -> initialFetchState(0L))) // First run will result in another offset for leader epoch request thread.doWork() @@ -549,7 +550,7 @@ class ReplicaAlterLogDirsThreadTest { replicaMgr = replicaManager, quota = quotaManager, brokerTopicStats = null) - thread.addPartitions(Map(t1p0 -> offsetAndEpoch(initialFetchOffset))) + thread.addPartitions(Map(t1p0 -> initialFetchState(initialFetchOffset))) //Run it thread.doWork() @@ -624,7 +625,7 @@ class ReplicaAlterLogDirsThreadTest { replicaMgr = replicaManager, quota = quotaManager, brokerTopicStats = null) - thread.addPartitions(Map(t1p0 -> offsetAndEpoch(0L))) + thread.addPartitions(Map(t1p0 -> initialFetchState(0L))) // Run thread 3 times (exactly number of times we mock exception for getReplicaOrException) (0 to 2).foreach { _ => @@ -685,7 +686,7 @@ class ReplicaAlterLogDirsThreadTest { replicaMgr = replicaManager, quota = quotaManager, brokerTopicStats = null) - thread.addPartitions(Map(t1p0 -> offsetAndEpoch(0L))) + thread.addPartitions(Map(t1p0 -> initialFetchState(0L))) // loop few times (0 to 3).foreach { _ => @@ -726,8 +727,8 @@ class ReplicaAlterLogDirsThreadTest { quota = quotaManager, brokerTopicStats = null) thread.addPartitions(Map( - t1p0 -> offsetAndEpoch(0L, leaderEpoch), - t1p1 -> offsetAndEpoch(0L, leaderEpoch))) + t1p0 -> initialFetchState(0L, leaderEpoch), + t1p1 -> initialFetchState(0L, leaderEpoch))) val ResultWithPartitions(fetchRequestOpt, partitionsWithError) = thread.buildFetch(Map( t1p0 -> PartitionFetchState(150, None, leaderEpoch, None, state = Fetching, lastFetchedEpoch = None), @@ -777,8 +778,8 @@ class ReplicaAlterLogDirsThreadTest { quota = quotaManager, brokerTopicStats = null) thread.addPartitions(Map( - t1p0 -> offsetAndEpoch(0L, leaderEpoch), - t1p1 -> offsetAndEpoch(0L, leaderEpoch))) + t1p0 -> initialFetchState(0L, leaderEpoch), + t1p1 -> initialFetchState(0L, leaderEpoch))) // one partition is ready and one is truncating val ResultWithPartitions(fetchRequestOpt, partitionsWithError) = thread.buildFetch(Map( diff --git a/core/src/test/scala/unit/kafka/server/ReplicaFetcherThreadTest.scala b/core/src/test/scala/unit/kafka/server/ReplicaFetcherThreadTest.scala index 4e265c1bf8fbe..0761e807cbf28 100644 --- a/core/src/test/scala/unit/kafka/server/ReplicaFetcherThreadTest.scala +++ b/core/src/test/scala/unit/kafka/server/ReplicaFetcherThreadTest.scala @@ -51,8 +51,9 @@ class ReplicaFetcherThreadTest { private val brokerEndPoint = new BrokerEndPoint(0, "localhost", 1000) private val failedPartitions = new FailedPartitions - private def offsetAndEpoch(fetchOffset: Long, leaderEpoch: Int = 1): OffsetAndEpoch = { - OffsetAndEpoch(offset = fetchOffset, leaderEpoch = leaderEpoch) + private def initialFetchState(fetchOffset: Long, leaderEpoch: Int = 1, lastFetchedEpoch: Option[Int] = None): InitialFetchState = { + InitialFetchState(leader = new BrokerEndPoint(0, "localhost", 9092), + initOffset = fetchOffset, currentLeaderEpoch = leaderEpoch, lastFetchedEpoch = lastFetchedEpoch) } @After @@ -140,9 +141,9 @@ class ReplicaFetcherThreadTest { // topic 1 supports epoch, t2 doesn't. `lastFetchedEpoch` not set for any partition thread.addPartitions(Map( - t1p0 -> offsetAndEpoch(0L), - t1p1 -> offsetAndEpoch(0L), - t2p1 -> offsetAndEpoch(0L))) + t1p0 -> initialFetchState(0L), + t1p1 -> initialFetchState(0L), + t2p1 -> initialFetchState(0L))) assertPartitionStates(thread, shouldBeReadyForFetch = false, shouldBeTruncatingLog = true, shouldBeDelayed = false) //Loop 1 @@ -239,9 +240,9 @@ class ReplicaFetcherThreadTest { val lastFetchedEpoch = Some(4) thread.addPartitions(Map( - t1p0 -> OffsetAndEpoch(0, leaderEpoch, lastFetchedEpoch), - t1p1 -> OffsetAndEpoch(0L, leaderEpoch, lastFetchedEpoch), - t2p1 -> OffsetAndEpoch(0L, leaderEpoch, if (hasUnknownLastFetchedEpoch) None else lastFetchedEpoch))) + t1p0 -> initialFetchState(0, leaderEpoch, lastFetchedEpoch), + t1p1 -> initialFetchState(0L, leaderEpoch, lastFetchedEpoch), + t2p1 -> initialFetchState(0L, leaderEpoch, if (hasUnknownLastFetchedEpoch) None else lastFetchedEpoch))) def verifyPartitionStates(fetching: Set[TopicPartition], truncating: Set[TopicPartition]): Unit = { @@ -384,7 +385,7 @@ class ReplicaFetcherThreadTest { val mockNetwork = new ReplicaFetcherMockBlockingSend(offsets, brokerEndPoint, new SystemTime()) val thread = new ReplicaFetcherThread("bob", 0, brokerEndPoint, config, failedPartitions, replicaManager, new Metrics, new SystemTime, UnboundedQuota, Some(mockNetwork)) - thread.addPartitions(Map(t1p0 -> offsetAndEpoch(0L), t1p1 -> offsetAndEpoch(0L))) + thread.addPartitions(Map(t1p0 -> initialFetchState(0L), t1p1 -> initialFetchState(0L))) //Loop 1 thread.doWork() @@ -446,7 +447,7 @@ class ReplicaFetcherThreadTest { val mockNetwork = new ReplicaFetcherMockBlockingSend(offsetsReply, brokerEndPoint, new SystemTime()) val thread = new ReplicaFetcherThread("bob", 0, brokerEndPoint, configs.head, failedPartitions, replicaManager, new Metrics(), new SystemTime(), quota, Some(mockNetwork)) - thread.addPartitions(Map(t1p0 -> offsetAndEpoch(0L), t2p1 -> offsetAndEpoch(0L))) + thread.addPartitions(Map(t1p0 -> initialFetchState(0L), t2p1 -> initialFetchState(0L))) //Run it thread.doWork() @@ -499,7 +500,7 @@ class ReplicaFetcherThreadTest { val mockNetwork = new ReplicaFetcherMockBlockingSend(offsetsReply, brokerEndPoint, new SystemTime()) val thread = new ReplicaFetcherThread("bob", 0, brokerEndPoint, configs.head, failedPartitions, replicaManager, new Metrics(), new SystemTime(), quota, Some(mockNetwork)) - thread.addPartitions(Map(t1p0 -> offsetAndEpoch(0L), t2p1 -> offsetAndEpoch(0L))) + thread.addPartitions(Map(t1p0 -> initialFetchState(0L), t2p1 -> initialFetchState(0L))) //Run it thread.doWork() @@ -554,7 +555,7 @@ class ReplicaFetcherThreadTest { // Create the fetcher thread val mockNetwork = new ReplicaFetcherMockBlockingSend(offsets, brokerEndPoint, new SystemTime()) val thread = new ReplicaFetcherThread("bob", 0, brokerEndPoint, config, failedPartitions, replicaManager, new Metrics(), new SystemTime(), quota, Some(mockNetwork)) - thread.addPartitions(Map(t1p0 -> offsetAndEpoch(0L), t1p1 -> offsetAndEpoch(0L))) + thread.addPartitions(Map(t1p0 -> initialFetchState(0L), t1p1 -> initialFetchState(0L))) // Loop 1 -- both topic partitions will need to fetch another leader epoch thread.doWork() @@ -629,7 +630,7 @@ class ReplicaFetcherThreadTest { // Create the fetcher thread val mockNetwork = new ReplicaFetcherMockBlockingSend(offsets, brokerEndPoint, new SystemTime()) val thread = new ReplicaFetcherThread("bob", 0, brokerEndPoint, config, failedPartitions, replicaManager, new Metrics(), new SystemTime(), quota, Some(mockNetwork)) - thread.addPartitions(Map(t1p0 -> offsetAndEpoch(0L), t1p1 -> offsetAndEpoch(0L))) + thread.addPartitions(Map(t1p0 -> initialFetchState(0L), t1p1 -> initialFetchState(0L))) // Loop 1 -- both topic partitions will truncate to leader offset even though they don't know // about leader epoch @@ -685,7 +686,7 @@ class ReplicaFetcherThreadTest { //Create the thread val mockNetwork = new ReplicaFetcherMockBlockingSend(offsetsReply, brokerEndPoint, new SystemTime()) val thread = new ReplicaFetcherThread("bob", 0, brokerEndPoint, configs.head, failedPartitions, replicaManager, new Metrics(), new SystemTime(), quota, Some(mockNetwork)) - thread.addPartitions(Map(t1p0 -> offsetAndEpoch(initialFetchOffset))) + thread.addPartitions(Map(t1p0 -> initialFetchState(initialFetchOffset))) //Run it thread.doWork() @@ -738,7 +739,7 @@ class ReplicaFetcherThreadTest { //Create the thread val mockNetwork = new ReplicaFetcherMockBlockingSend(offsetsReply, brokerEndPoint, new SystemTime()) val thread = new ReplicaFetcherThread("bob", 0, brokerEndPoint, configs.head, failedPartitions, replicaManager, new Metrics(), new SystemTime(), quota, Some(mockNetwork)) - thread.addPartitions(Map(t1p0 -> offsetAndEpoch(0L), t1p1 -> offsetAndEpoch(0L))) + thread.addPartitions(Map(t1p0 -> initialFetchState(0L), t1p1 -> initialFetchState(0L))) //Run thread 3 times (0 to 3).foreach { _ => @@ -794,7 +795,7 @@ class ReplicaFetcherThreadTest { val thread = new ReplicaFetcherThread("bob", 0, brokerEndPoint, config, failedPartitions, replicaManager, new Metrics(), new SystemTime(), quota, Some(mockNetwork)) //When - thread.addPartitions(Map(t1p0 -> offsetAndEpoch(0L), t1p1 -> offsetAndEpoch(0L))) + thread.addPartitions(Map(t1p0 -> initialFetchState(0L), t1p1 -> initialFetchState(0L))) //Then all partitions should start in an TruncatingLog state assertEquals(Option(Truncating), thread.fetchState(t1p0).map(_.state)) @@ -847,7 +848,7 @@ class ReplicaFetcherThreadTest { new SystemTime(), quota, Some(mockNetwork)) //When - thread.addPartitions(Map(t1p0 -> offsetAndEpoch(0L), t1p1 -> offsetAndEpoch(0L))) + thread.addPartitions(Map(t1p0 -> initialFetchState(0L), t1p1 -> initialFetchState(0L))) //When the epoch request is outstanding, remove one of the partitions to simulate a leader change. We do this via a callback passed to the mock thread val partitionThatBecameLeader = t1p0 diff --git a/core/src/test/scala/unit/kafka/server/ReplicaManagerQuotasTest.scala b/core/src/test/scala/unit/kafka/server/ReplicaManagerQuotasTest.scala index 71bce3e9fe33b..25ee84eb7b3f4 100644 --- a/core/src/test/scala/unit/kafka/server/ReplicaManagerQuotasTest.scala +++ b/core/src/test/scala/unit/kafka/server/ReplicaManagerQuotasTest.scala @@ -177,7 +177,7 @@ class ReplicaManagerQuotasTest { val tp = new TopicPartition("t1", 0) val fetchPartitionStatus = FetchPartitionStatus(LogOffsetMetadata(messageOffset = 50L, segmentBaseOffset = 0L, - relativePositionInSegment = 250), new PartitionData(50, 0, 1, Optional.empty()), hasDivergingEpoch = false) + relativePositionInSegment = 250), new PartitionData(50, 0, 1, Optional.empty())) val fetchMetadata = FetchMetadata(fetchMinBytes = 1, fetchMaxBytes = 1000, hardMaxBytesLimit = true, diff --git a/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala b/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala index 7f2c8505eb560..cd0ca0c7c22b4 100644 --- a/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala +++ b/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala @@ -1542,7 +1542,9 @@ class ReplicaManagerTest { override def doWork() = { // In case the thread starts before the partition is added by AbstractFetcherManager, // add it here (it's a no-op if already added) - val initialOffset = OffsetAndEpoch(offset = 0L, leaderEpoch = leaderEpochInLeaderAndIsr) + val initialOffset = InitialFetchState( + leader = new BrokerEndPoint(0, "localhost", 9092), + initOffset = 0L, currentLeaderEpoch = leaderEpochInLeaderAndIsr, lastFetchedEpoch = None) addPartitions(Map(new TopicPartition(topic, topicPartition) -> initialOffset)) super.doWork() diff --git a/jmh-benchmarks/src/main/java/org/apache/kafka/jmh/fetcher/ReplicaFetcherThreadBenchmark.java b/jmh-benchmarks/src/main/java/org/apache/kafka/jmh/fetcher/ReplicaFetcherThreadBenchmark.java index 0e6218a9bcf2b..e14b2dded09dc 100644 --- a/jmh-benchmarks/src/main/java/org/apache/kafka/jmh/fetcher/ReplicaFetcherThreadBenchmark.java +++ b/jmh-benchmarks/src/main/java/org/apache/kafka/jmh/fetcher/ReplicaFetcherThreadBenchmark.java @@ -31,6 +31,7 @@ import kafka.server.BrokerState; import kafka.server.BrokerTopicStats; import kafka.server.FailedPartitions; +import kafka.server.InitialFetchState; import kafka.server.KafkaConfig; import kafka.server.LogDirFailureChannel; import kafka.server.MetadataCache; @@ -137,7 +138,7 @@ public void setup() throws IOException { Time.SYSTEM); LinkedHashMap> initialFetched = new LinkedHashMap<>(); - scala.collection.mutable.Map offsetAndEpochs = new scala.collection.mutable.HashMap<>(); + scala.collection.mutable.Map initialFetchStates = new scala.collection.mutable.HashMap<>(); for (int i = 0; i < partitionCount; i++) { TopicPartition tp = new TopicPartition("topic", i); @@ -162,7 +163,7 @@ public void setup() throws IOException { partition.makeFollower(partitionState, offsetCheckpoints); pool.put(tp, partition); - offsetAndEpochs.put(tp, new OffsetAndEpoch(0, 0, Option.empty())); + initialFetchStates.put(tp, new InitialFetchState(new BrokerEndPoint(3, "host", 3000), 0, 0, Option.empty())); BaseRecords fetched = new BaseRecords() { @Override public int sizeInBytes() { @@ -181,7 +182,7 @@ public RecordsSend toSend(String destination) { ReplicaManager replicaManager = Mockito.mock(ReplicaManager.class); Mockito.when(replicaManager.brokerTopicStats()).thenReturn(brokerTopicStats); fetcher = new ReplicaFetcherBenchThread(config, replicaManager, pool); - fetcher.addPartitions(offsetAndEpochs); + fetcher.addPartitions(initialFetchStates); // force a pass to move partitions to fetching state. We do this in the setup phase // so that we do not measure this time as part of the steady state work fetcher.doWork(); @@ -288,7 +289,7 @@ public void truncate(TopicPartition tp, OffsetTruncationState offsetTruncationSt @Override public Option endOffsetForEpoch(TopicPartition topicPartition, int epoch) { - return OptionConverters.toScala(Optional.of(new OffsetAndEpoch(0, 0, Option.empty()))); + return OptionConverters.toScala(Optional.of(new OffsetAndEpoch(0, 0))); } @Override From bc4a933f6b7d4ddf557b6bf1b9660c8e527c6f0b Mon Sep 17 00:00:00 2001 From: Rajini Sivaram Date: Fri, 16 Oct 2020 22:31:10 +0100 Subject: [PATCH 03/11] Fix initial offsets for ReplicaAlterLogDirs --- core/src/main/scala/kafka/server/ReplicaManager.scala | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/core/src/main/scala/kafka/server/ReplicaManager.scala b/core/src/main/scala/kafka/server/ReplicaManager.scala index 9845d856cb293..fdfd36dc1dc21 100644 --- a/core/src/main/scala/kafka/server/ReplicaManager.scala +++ b/core/src/main/scala/kafka/server/ReplicaManager.scala @@ -769,9 +769,8 @@ class ReplicaManager(val config: KafkaConfig, val futureLog = futureLocalLogOrException(topicPartition) logManager.abortAndPauseCleaning(topicPartition) - val (fetchOffset, lastFetchedEpoch) = initialFetchOffsetAndEpoch(futureLog) val initialFetchState = InitialFetchState(BrokerEndPoint(config.brokerId, "localhost", -1), - partition.getLeaderEpoch, fetchOffset, lastFetchedEpoch) + partition.getLeaderEpoch, futureLog.highWatermark, lastFetchedEpoch = None) replicaAlterLogDirsManager.addFetcherForPartitions(Map(topicPartition -> initialFetchState)) } @@ -1484,9 +1483,8 @@ class ReplicaManager(val config: KafkaConfig, // replica from source dir to destination dir logManager.abortAndPauseCleaning(topicPartition) - val (fetchOffset, lastFetchedEpoch) = initialFetchOffsetAndEpoch(log) futureReplicasAndInitialOffset.put(topicPartition, InitialFetchState(leader, - partition.getLeaderEpoch, fetchOffset, lastFetchedEpoch)) + partition.getLeaderEpoch, log.highWatermark, lastFetchedEpoch = None)) } } } From 800f11726d6f2056a10ff6ddf3e396fd9395c21d Mon Sep 17 00:00:00 2001 From: Rajini Sivaram Date: Mon, 19 Oct 2020 17:24:02 +0100 Subject: [PATCH 04/11] Fix resizeThreadPool --- .../main/scala/kafka/server/AbstractFetcherThread.scala | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/core/src/main/scala/kafka/server/AbstractFetcherThread.scala b/core/src/main/scala/kafka/server/AbstractFetcherThread.scala index 282cb8e3341c4..9d4fd3fda84a3 100755 --- a/core/src/main/scala/kafka/server/AbstractFetcherThread.scala +++ b/core/src/main/scala/kafka/server/AbstractFetcherThread.scala @@ -723,12 +723,18 @@ abstract class AbstractFetcherThread(name: String, Option(partitionStates.stateValue(topicPartition)) } + /** + * Returns current fetch state for each partition assigned to this thread. This is used to reassign + * partitions when thread pool is resized. We return `lastFetchedEpoch=None` to ensure we go through + * the truncation path to simplify the code path where this thread makes progress after this method + * before the thread is shutdown. + */ private[server] def partitionsAndOffsets: Map[TopicPartition, InitialFetchState] = inLock(partitionMapLock) { partitionStates.partitionStateMap.asScala.map { case (topicPartition, currentFetchState) => val initialFetchState = InitialFetchState(sourceBroker, currentLeaderEpoch = currentFetchState.currentLeaderEpoch, initOffset = currentFetchState.fetchOffset, - currentFetchState.lastFetchedEpoch) + lastFetchedEpoch = None) topicPartition -> initialFetchState } } From 1fa0340795256d5862f79a958fa98a837447fc47 Mon Sep 17 00:00:00 2001 From: Rajini Sivaram Date: Thu, 22 Oct 2020 22:03:36 +0100 Subject: [PATCH 05/11] Address review comments --- core/src/main/scala/kafka/log/Log.scala | 15 +++- .../kafka/server/AbstractFetcherThread.scala | 44 +++++----- .../scala/kafka/server/ReplicaManager.scala | 4 +- .../server/AbstractFetcherThreadTest.scala | 88 +++++++++++++++++-- .../AbstractFetcherThreadWithIbp26Test.scala | 23 +++++ .../server/ReplicaFetcherThreadTest.scala | 2 +- 6 files changed, 140 insertions(+), 36 deletions(-) create mode 100644 core/src/test/scala/unit/kafka/server/AbstractFetcherThreadWithIbp26Test.scala diff --git a/core/src/main/scala/kafka/log/Log.scala b/core/src/main/scala/kafka/log/Log.scala index 15dc9cedd1079..2abfcf6cc7c9b 100644 --- a/core/src/main/scala/kafka/log/Log.scala +++ b/core/src/main/scala/kafka/log/Log.scala @@ -50,11 +50,11 @@ import scala.collection.mutable.{ArrayBuffer, ListBuffer} import scala.collection.{Seq, Set, mutable} object LogAppendInfo { - val UnknownLogAppendInfo = LogAppendInfo(None, -1, RecordBatch.NO_TIMESTAMP, -1L, RecordBatch.NO_TIMESTAMP, -1L, + val UnknownLogAppendInfo = LogAppendInfo(None, -1, None, RecordBatch.NO_TIMESTAMP, -1L, RecordBatch.NO_TIMESTAMP, -1L, RecordConversionStats.EMPTY, NoCompressionCodec, NoCompressionCodec, -1, -1, offsetsMonotonic = false, -1L) def unknownLogAppendInfoWithLogStartOffset(logStartOffset: Long): LogAppendInfo = - LogAppendInfo(None, -1, RecordBatch.NO_TIMESTAMP, -1L, RecordBatch.NO_TIMESTAMP, logStartOffset, + LogAppendInfo(None, -1, None, RecordBatch.NO_TIMESTAMP, -1L, RecordBatch.NO_TIMESTAMP, logStartOffset, RecordConversionStats.EMPTY, NoCompressionCodec, NoCompressionCodec, -1, -1, offsetsMonotonic = false, -1L) @@ -64,7 +64,7 @@ object LogAppendInfo { * in unknownLogAppendInfoWithLogStartOffset, but with additiona fields recordErrors and errorMessage */ def unknownLogAppendInfoWithAdditionalInfo(logStartOffset: Long, recordErrors: Seq[RecordError], errorMessage: String): LogAppendInfo = - LogAppendInfo(None, -1, RecordBatch.NO_TIMESTAMP, -1L, RecordBatch.NO_TIMESTAMP, logStartOffset, + LogAppendInfo(None, -1, None, RecordBatch.NO_TIMESTAMP, -1L, RecordBatch.NO_TIMESTAMP, logStartOffset, RecordConversionStats.EMPTY, NoCompressionCodec, NoCompressionCodec, -1, -1, offsetsMonotonic = false, -1L, recordErrors, errorMessage) } @@ -82,6 +82,7 @@ object LeaderHwChange { * @param firstOffset The first offset in the message set unless the message format is less than V2 and we are appending * to the follower. * @param lastOffset The last offset in the message set + * @param lastLeaderEpoch The partition leader epoch corresponding to the last offset, if available. * @param maxTimestamp The maximum timestamp of the message set. * @param offsetOfMaxTimestamp The offset of the message with the maximum timestamp. * @param logAppendTime The log append time (if used) of the message set, otherwise Message.NoTimestamp @@ -99,6 +100,7 @@ object LeaderHwChange { */ case class LogAppendInfo(var firstOffset: Option[Long], var lastOffset: Long, + var lastLeaderEpoch: Option[Int], var maxTimestamp: Long, var offsetOfMaxTimestamp: Long, var logAppendTime: Long, @@ -1383,6 +1385,7 @@ class Log(@volatile private var _dir: File, var validBytesCount = 0 var firstOffset: Option[Long] = None var lastOffset = -1L + var lastLeaderEpoch: Option[Int] = None var sourceCodec: CompressionCodec = NoCompressionCodec var monotonic = true var maxTimestamp = RecordBatch.NO_TIMESTAMP @@ -1415,6 +1418,10 @@ class Log(@volatile private var _dir: File, // update the last offset seen lastOffset = batch.lastOffset + lastLeaderEpoch = if (batch.partitionLeaderEpoch != RecordBatch.NO_PARTITION_LEADER_EPOCH) + Some(batch.partitionLeaderEpoch) + else + None // Check if the message sizes are valid. val batchSize = batch.sizeInBytes @@ -1446,7 +1453,7 @@ class Log(@volatile private var _dir: File, // Apply broker-side compression if any val targetCodec = BrokerCompressionCodec.getTargetCompressionCodec(config.compressionType, sourceCodec) - LogAppendInfo(firstOffset, lastOffset, maxTimestamp, offsetOfMaxTimestamp, RecordBatch.NO_TIMESTAMP, logStartOffset, + LogAppendInfo(firstOffset, lastOffset, lastLeaderEpoch, maxTimestamp, offsetOfMaxTimestamp, RecordBatch.NO_TIMESTAMP, logStartOffset, RecordConversionStats.EMPTY, sourceCodec, targetCodec, shallowMessageCount, validBytesCount, monotonic, lastOffsetOfFirstBatch) } diff --git a/core/src/main/scala/kafka/server/AbstractFetcherThread.scala b/core/src/main/scala/kafka/server/AbstractFetcherThread.scala index 9d4fd3fda84a3..d7de9216cef87 100755 --- a/core/src/main/scala/kafka/server/AbstractFetcherThread.scala +++ b/core/src/main/scala/kafka/server/AbstractFetcherThread.scala @@ -223,7 +223,7 @@ abstract class AbstractFetcherThread(name: String, val ResultWithPartitions(fetchOffsets, partitionsWithError) = maybeTruncateToEpochEndOffsets(epochEndOffsets, latestEpochsForPartitions) handlePartitionsWithErrors(partitionsWithError, "truncateToEpochEndOffsets") - updateFetchOffsetAndMaybeMarkTruncationComplete(fetchOffsets) + updateFetchOffsetAndMaybeMarkTruncationComplete(fetchOffsets, isTruncationOnFetchSupported) } } @@ -231,7 +231,7 @@ abstract class AbstractFetcherThread(name: String, inLock(partitionMapLock) { val ResultWithPartitions(fetchOffsets, partitionsWithError) = maybeTruncateToEpochEndOffsets(epochEndOffsets, Map.empty) handlePartitionsWithErrors(partitionsWithError, "truncateOnFetchResponse") - updateFetchOffsetAndMaybeMarkTruncationComplete(fetchOffsets) + updateFetchOffsetAndMaybeMarkTruncationComplete(fetchOffsets, maySkipTruncation = false) } } @@ -251,7 +251,7 @@ abstract class AbstractFetcherThread(name: String, } } - updateFetchOffsetAndMaybeMarkTruncationComplete(fetchOffsets) + updateFetchOffsetAndMaybeMarkTruncationComplete(fetchOffsets, isTruncationOnFetchSupported) } private def maybeTruncateToEpochEndOffsets(fetchedEpochs: Map[TopicPartition, EpochEndOffset], @@ -354,7 +354,7 @@ abstract class AbstractFetcherThread(name: String, // Update partitionStates only if there is no exception during processPartitionData val newFetchState = PartitionFetchState(nextOffset, Some(lag), currentFetchState.currentLeaderEpoch, state = Fetching, - Some(currentFetchState.currentLeaderEpoch)) + logAppendInfo.lastLeaderEpoch) partitionStates.updateAndMoveToEnd(topicPartition, newFetchState) fetcherStats.byteRate.mark(validBytes) } @@ -460,20 +460,12 @@ abstract class AbstractFetcherThread(name: String, * For older versions, we can skip the truncation step iff the leader epoch matches the existing epoch. */ private def partitionFetchState(tp: TopicPartition, initialFetchState: InitialFetchState, currentState: PartitionFetchState): PartitionFetchState = { - if (isTruncationOnFetchSupported && initialFetchState.initOffset >= 0 && initialFetchState.lastFetchedEpoch.nonEmpty) { - if (currentState == null) { - return PartitionFetchState(initialFetchState.initOffset, None, initialFetchState.currentLeaderEpoch, - state = Fetching, initialFetchState.lastFetchedEpoch) - } - // If we are in `Fetching` state can continue to fetch regardless of current leader epoch and truncate - // if necessary based on diverging epochs returned by the leader. If we are currently in Truncating state, - // fall through and handle based on current epoch. - if (currentState.state == Fetching) { - return currentState - } - } if (currentState != null && currentState.currentLeaderEpoch == initialFetchState.currentLeaderEpoch) { currentState + } else if (isTruncationOnFetchSupported && initialFetchState.initOffset >= 0 && initialFetchState.lastFetchedEpoch.nonEmpty && + (currentState == null || currentState.state == Fetching)) { + PartitionFetchState(initialFetchState.initOffset, None, initialFetchState.currentLeaderEpoch, + state = Fetching, initialFetchState.lastFetchedEpoch) } else if (initialFetchState.initOffset < 0) { fetchOffsetAndTruncate(tp, initialFetchState.currentLeaderEpoch) } else { @@ -503,16 +495,23 @@ abstract class AbstractFetcherThread(name: String, * truncation completed if their offsetTruncationState indicates truncation completed * * @param fetchOffsets the partitions to update fetch offset and maybe mark truncation complete + * @param maySkipTruncation true if we can stay in Fetching mode and perform truncation later based on + * diverging epochs from fetch responses. */ - private def updateFetchOffsetAndMaybeMarkTruncationComplete(fetchOffsets: Map[TopicPartition, OffsetTruncationState]): Unit = { + private def updateFetchOffsetAndMaybeMarkTruncationComplete(fetchOffsets: Map[TopicPartition, OffsetTruncationState], + maySkipTruncation: Boolean): Unit = { val newStates: Map[TopicPartition, PartitionFetchState] = partitionStates.partitionStateMap.asScala .map { case (topicPartition, currentFetchState) => val maybeTruncationComplete = fetchOffsets.get(topicPartition) match { case Some(offsetTruncationState) => - val state = if (offsetTruncationState.truncationCompleted) Fetching else Truncating - // Resetting `lastFetchedEpoch` since we are truncating and don't expect diverging epoch in the next fetch + val (state, lastFetchedEpoch) = if (offsetTruncationState.truncationCompleted) + (Fetching, latestEpoch(topicPartition)) + else if (maySkipTruncation && currentFetchState.lastFetchedEpoch.nonEmpty) + (Fetching, currentFetchState.lastFetchedEpoch) + else + (Truncating, latestEpoch(topicPartition)) PartitionFetchState(offsetTruncationState.offset, currentFetchState.lag, - currentFetchState.currentLeaderEpoch, currentFetchState.delay, state, lastFetchedEpoch = None) + currentFetchState.currentLeaderEpoch, currentFetchState.delay, state, lastFetchedEpoch) case None => currentFetchState } (topicPartition, maybeTruncationComplete) @@ -647,7 +646,7 @@ abstract class AbstractFetcherThread(name: String, fetcherLagStats.getAndMaybePut(topicPartition).lag = 0 PartitionFetchState(leaderEndOffset, Some(0), currentLeaderEpoch, - state = Fetching, lastFetchedEpoch = Some(currentLeaderEpoch)) + state = Fetching, lastFetchedEpoch = latestEpoch(topicPartition)) } else { /** * If the leader's log end offset is greater than the follower's log end offset, there are two possibilities: @@ -680,9 +679,8 @@ abstract class AbstractFetcherThread(name: String, val initialLag = leaderEndOffset - offsetToFetch fetcherLagStats.getAndMaybePut(topicPartition).lag = initialLag - // We don't expect diverging epochs from the next fetch request, so resetting `lastFetchedEpoch` PartitionFetchState(offsetToFetch, Some(initialLag), currentLeaderEpoch, - state = Fetching, lastFetchedEpoch = None) + state = Fetching, lastFetchedEpoch = latestEpoch(topicPartition)) } } diff --git a/core/src/main/scala/kafka/server/ReplicaManager.scala b/core/src/main/scala/kafka/server/ReplicaManager.scala index fdfd36dc1dc21..77bf028a67fb6 100644 --- a/core/src/main/scala/kafka/server/ReplicaManager.scala +++ b/core/src/main/scala/kafka/server/ReplicaManager.scala @@ -770,7 +770,7 @@ class ReplicaManager(val config: KafkaConfig, logManager.abortAndPauseCleaning(topicPartition) val initialFetchState = InitialFetchState(BrokerEndPoint(config.brokerId, "localhost", -1), - partition.getLeaderEpoch, futureLog.highWatermark, lastFetchedEpoch = None) + partition.getLeaderEpoch, futureLog.highWatermark, lastFetchedEpoch = futureLog.latestEpoch) replicaAlterLogDirsManager.addFetcherForPartitions(Map(topicPartition -> initialFetchState)) } @@ -1484,7 +1484,7 @@ class ReplicaManager(val config: KafkaConfig, logManager.abortAndPauseCleaning(topicPartition) futureReplicasAndInitialOffset.put(topicPartition, InitialFetchState(leader, - partition.getLeaderEpoch, log.highWatermark, lastFetchedEpoch = None)) + partition.getLeaderEpoch, log.highWatermark, lastFetchedEpoch = log.latestEpoch)) } } } diff --git a/core/src/test/scala/unit/kafka/server/AbstractFetcherThreadTest.scala b/core/src/test/scala/unit/kafka/server/AbstractFetcherThreadTest.scala index 09846f3614c89..53a479ecd023d 100644 --- a/core/src/test/scala/unit/kafka/server/AbstractFetcherThreadTest.scala +++ b/core/src/test/scala/unit/kafka/server/AbstractFetcherThreadTest.scala @@ -31,6 +31,7 @@ import kafka.utils.TestUtils import org.apache.kafka.common.KafkaException import org.apache.kafka.common.TopicPartition import org.apache.kafka.common.errors.{FencedLeaderEpochException, UnknownLeaderEpochException} +import org.apache.kafka.common.message.FetchResponseData import org.apache.kafka.common.protocol.{ApiKeys, Errors} import org.apache.kafka.common.record._ import org.apache.kafka.common.requests.{EpochEndOffset, FetchRequest} @@ -44,9 +45,11 @@ import scala.util.Random import org.scalatest.Assertions.assertThrows import scala.collection.mutable.ArrayBuffer +import scala.compat.java8.OptionConverters._ class AbstractFetcherThreadTest { + val truncateOnFetch = true private val partition1 = new TopicPartition("topic1", 0) private val partition2 = new TopicPartition("topic2", 0) private val failedPartitions = new FailedPartitions @@ -63,9 +66,9 @@ class AbstractFetcherThreadTest { .batches.asScala.head } - private def initialFetchState(fetchOffset: Long, leaderEpoch: Int): InitialFetchState = { + private def initialFetchState(fetchOffset: Long, leaderEpoch: Int, lastFetchedEpoch: Option[Int] = None): InitialFetchState = { InitialFetchState(leader = new BrokerEndPoint(0, "localhost", 9092), - initOffset = fetchOffset, currentLeaderEpoch = leaderEpoch, lastFetchedEpoch = None) + initOffset = fetchOffset, currentLeaderEpoch = leaderEpoch, lastFetchedEpoch = lastFetchedEpoch) } @Test @@ -798,6 +801,40 @@ class AbstractFetcherThreadTest { } + @Test + def testDivergingEpochs(): Unit = { + val partition = new TopicPartition("topic", 0) + val fetcher = new MockFetcherThread + + val replicaLog = Seq( + mkBatch(baseOffset = 0, leaderEpoch = 0, new SimpleRecord("a".getBytes)), + mkBatch(baseOffset = 1, leaderEpoch = 2, new SimpleRecord("b".getBytes)), + mkBatch(baseOffset = 2, leaderEpoch = 4, new SimpleRecord("c".getBytes))) + + val replicaState = MockFetcherThread.PartitionState(replicaLog, leaderEpoch = 5, highWatermark = 0L) + fetcher.setReplicaState(partition, replicaState) + fetcher.addPartitions(Map(partition -> initialFetchState(3L, leaderEpoch = 5, lastFetchedEpoch = Some(4)))) + assertEquals(3L, replicaState.logEndOffset) + fetcher.verifyLastFetchedEpoch(partition, expectedEpoch = Some(4)) + + val leaderLog = Seq( + mkBatch(baseOffset = 0, leaderEpoch = 0, new SimpleRecord("a".getBytes)), + mkBatch(baseOffset = 1, leaderEpoch = 2, new SimpleRecord("b".getBytes)), + mkBatch(baseOffset = 2, leaderEpoch = 5, new SimpleRecord("d".getBytes))) + + val leaderState = MockFetcherThread.PartitionState(leaderLog, leaderEpoch = 5, highWatermark = 2L) + fetcher.setLeaderState(partition, leaderState) + + fetcher.doWork() + fetcher.verifyLastFetchedEpoch(partition, Some(2)) + + TestUtils.waitUntilTrue(() => { + fetcher.doWork() + fetcher.replicaPartitionState(partition).log == fetcher.leaderPartitionState(partition).log + }, "Failed to reconcile leader and follower logs") + fetcher.verifyLastFetchedEpoch(partition, Some(5)) + } + object MockFetcherThread { class PartitionState(var log: mutable.Buffer[RecordBatch], var leaderEpoch: Int, @@ -863,6 +900,7 @@ class AbstractFetcherThreadTest { var maxTimestamp = RecordBatch.NO_TIMESTAMP var offsetOfMaxTimestamp = -1L var lastOffset = state.logEndOffset + var lastEpoch: Option[Int] = None for (batch <- batches) { batch.ensureValid() @@ -873,6 +911,7 @@ class AbstractFetcherThreadTest { state.log.append(batch) state.logEndOffset = batch.nextOffset lastOffset = batch.lastOffset + lastEpoch = Some(batch.partitionLeaderEpoch) } state.logStartOffset = partitionData.logStartOffset @@ -880,6 +919,7 @@ class AbstractFetcherThreadTest { Some(LogAppendInfo(firstOffset = Some(fetchOffset), lastOffset = lastOffset, + lastLeaderEpoch = lastEpoch, maxTimestamp = maxTimestamp, offsetOfMaxTimestamp = offsetOfMaxTimestamp, logAppendTime = Time.SYSTEM.milliseconds(), @@ -915,8 +955,12 @@ class AbstractFetcherThreadTest { partitionMap.foreach { case (partition, state) => if (state.isReadyForFetch) { val replicaState = replicaPartitionState(partition) + val lastFetchedEpoch = if (isTruncationOnFetchSupported) + state.lastFetchedEpoch.map(_.asInstanceOf[Integer]).asJava + else + Optional.empty[Integer] fetchData.put(partition, new FetchRequest.PartitionData(state.fetchOffset, replicaState.logStartOffset, - 1024 * 1024, Optional.of[Integer](state.currentLeaderEpoch))) + 1024 * 1024, Optional.of[Integer](state.currentLeaderEpoch), lastFetchedEpoch)) } } val fetchRequest = FetchRequest.Builder.forReplica(ApiKeys.FETCH.latestVersion, replicaId, 0, 1, fetchData.asJava) @@ -956,6 +1000,31 @@ class AbstractFetcherThreadTest { } } + def verifyLastFetchedEpoch(partition: TopicPartition, expectedEpoch: Option[Int]): Unit = { + if (isTruncationOnFetchSupported) { + assertEquals(Some(Fetching), fetchState(partition).map(_.state)) + assertEquals(expectedEpoch, fetchState(partition).flatMap(_.lastFetchedEpoch)) + } + } + + private def divergingEpochAndOffset(partition: TopicPartition, + lastFetchedEpoch: Optional[Integer], + fetchOffset: Long, + partitionState: PartitionState): Option[FetchResponseData.EpochEndOffset] = { + lastFetchedEpoch.asScala.flatMap { fetchEpoch => + val epochEndOffset = fetchEpochEndOffsets(Map(partition -> new EpochData(Optional.empty[Integer], fetchEpoch)))(partition) + + if (partitionState.log.isEmpty || epochEndOffset.hasUndefinedEpochOrOffset) + None + else if (epochEndOffset.leaderEpoch < fetchEpoch || epochEndOffset.endOffset < fetchOffset) { + Some(new FetchResponseData.EpochEndOffset() + .setEpoch(epochEndOffset.leaderEpoch) + .setEndOffset(epochEndOffset.endOffset)) + } else + None + } + } + private def lookupEndOffsetForEpoch(epochData: EpochData, partitionState: PartitionState): EpochEndOffset = { checkExpectedLeaderEpoch(epochData.currentLeaderEpoch, partitionState).foreach { error => @@ -965,7 +1034,11 @@ class AbstractFetcherThreadTest { var epochLowerBound = EpochEndOffset.UNDEFINED_EPOCH for (batch <- partitionState.log) { if (batch.partitionLeaderEpoch > epochData.leaderEpoch) { - return new EpochEndOffset(Errors.NONE, epochLowerBound, batch.baseOffset) + // If we don't have the requested epoch, return the next higher entry + if (epochLowerBound == EpochEndOffset.UNDEFINED_EPOCH) + return new EpochEndOffset(Errors.NONE, batch.partitionLeaderEpoch, batch.baseOffset) + else + return new EpochEndOffset(Errors.NONE, epochLowerBound, batch.baseOffset) } epochLowerBound = batch.partitionLeaderEpoch } @@ -984,17 +1057,20 @@ class AbstractFetcherThreadTest { override protected def isOffsetForLeaderEpochSupported: Boolean = true - override protected def isTruncationOnFetchSupported: Boolean = false + override protected def isTruncationOnFetchSupported: Boolean = truncateOnFetch override def fetchFromLeader(fetchRequest: FetchRequest.Builder): Map[TopicPartition, FetchData] = { fetchRequest.fetchData.asScala.map { case (partition, fetchData) => val leaderState = leaderPartitionState(partition) val epochCheckError = checkExpectedLeaderEpoch(fetchData.currentLeaderEpoch, leaderState) + val divergingEpoch = divergingEpochAndOffset(partition, fetchData.lastFetchedEpoch, fetchData.fetchOffset, leaderState) val (error, records) = if (epochCheckError.isDefined) { (epochCheckError.get, MemoryRecords.EMPTY) } else if (fetchData.fetchOffset > leaderState.logEndOffset || fetchData.fetchOffset < leaderState.logStartOffset) { (Errors.OFFSET_OUT_OF_RANGE, MemoryRecords.EMPTY) + } else if (divergingEpoch.nonEmpty) { + (Errors.NONE, MemoryRecords.EMPTY) } else { // for simplicity, we fetch only one batch at a time val records = leaderState.log.find(_.baseOffset >= fetchData.fetchOffset) match { @@ -1012,7 +1088,7 @@ class AbstractFetcherThreadTest { } (partition, new FetchData(error, leaderState.highWatermark, leaderState.highWatermark, leaderState.logStartOffset, - List.empty.asJava, records)) + Optional.empty[Integer], List.empty.asJava, divergingEpoch.asJava, records)) }.toMap } diff --git a/core/src/test/scala/unit/kafka/server/AbstractFetcherThreadWithIbp26Test.scala b/core/src/test/scala/unit/kafka/server/AbstractFetcherThreadWithIbp26Test.scala new file mode 100644 index 0000000000000..a85246567c50a --- /dev/null +++ b/core/src/test/scala/unit/kafka/server/AbstractFetcherThreadWithIbp26Test.scala @@ -0,0 +1,23 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package kafka.server + +class AbstractFetcherThreadWithIbp26Test extends AbstractFetcherThreadTest { + + override val truncateOnFetch = false +} diff --git a/core/src/test/scala/unit/kafka/server/ReplicaFetcherThreadTest.scala b/core/src/test/scala/unit/kafka/server/ReplicaFetcherThreadTest.scala index 0761e807cbf28..929ffe69732d7 100644 --- a/core/src/test/scala/unit/kafka/server/ReplicaFetcherThreadTest.scala +++ b/core/src/test/scala/unit/kafka/server/ReplicaFetcherThreadTest.scala @@ -117,7 +117,7 @@ class ReplicaFetcherThreadTest { expect(log.latestEpoch).andReturn(Some(leaderEpoch)).once() expect(log.latestEpoch).andReturn(None).once() // t2p1 doesnt support epochs if (ApiVersion.isTruncationOnFetchSupported(ibp)) - expect(log.latestEpoch).andReturn(Some(leaderEpoch)).times(2) // to set lastFetchedEpoch + expect(log.latestEpoch).andReturn(Some(leaderEpoch)).times(3) // to set lastFetchedEpoch expect(log.endOffsetForEpoch(leaderEpoch)).andReturn( Some(OffsetAndEpoch(0, leaderEpoch))).anyTimes() expect(replicaManager.logManager).andReturn(logManager).anyTimes() From 16dc12548c3261d03b6b2e02ff7010dcac331c07 Mon Sep 17 00:00:00 2001 From: Rajini Sivaram Date: Fri, 23 Oct 2020 22:44:46 +0100 Subject: [PATCH 06/11] Revert log.dir fetcher changes --- .../kafka/server/AbstractFetcherThread.scala | 25 ++++++++----------- .../server/ReplicaAlterLogDirsThread.scala | 4 +-- .../scala/kafka/server/ReplicaManager.scala | 4 +-- .../DynamicBrokerReconfigurationTest.scala | 8 ++---- 4 files changed, 17 insertions(+), 24 deletions(-) diff --git a/core/src/main/scala/kafka/server/AbstractFetcherThread.scala b/core/src/main/scala/kafka/server/AbstractFetcherThread.scala index d7de9216cef87..01b49d947f9d4 100755 --- a/core/src/main/scala/kafka/server/AbstractFetcherThread.scala +++ b/core/src/main/scala/kafka/server/AbstractFetcherThread.scala @@ -427,22 +427,19 @@ abstract class AbstractFetcherThread(name: String, /** * This is used to mark partitions for truncation in ReplicaAlterLogDirsThread after leader - * offsets are known. From IBP 2.7 onwards, we track lastFetchedEpoch and perform truncation - * based on diverging offsets returned in fetch responses, so this step is not required. + * offsets are known. */ def markPartitionsForTruncation(topicPartition: TopicPartition, truncationOffset: Long): Unit = { - if (!isTruncationOnFetchSupported) { - partitionMapLock.lockInterruptibly() - try { - Option(partitionStates.stateValue(topicPartition)).foreach { state => - val newState = PartitionFetchState(math.min(truncationOffset, state.fetchOffset), - state.lag, state.currentLeaderEpoch, state.delay, state = Truncating, - lastFetchedEpoch = None) - partitionStates.updateAndMoveToEnd(topicPartition, newState) - partitionMapCond.signalAll() - } - } finally partitionMapLock.unlock() - } + partitionMapLock.lockInterruptibly() + try { + Option(partitionStates.stateValue(topicPartition)).foreach { state => + val newState = PartitionFetchState(math.min(truncationOffset, state.fetchOffset), + state.lag, state.currentLeaderEpoch, state.delay, state = Truncating, + lastFetchedEpoch = None) + partitionStates.updateAndMoveToEnd(topicPartition, newState) + partitionMapCond.signalAll() + } + } finally partitionMapLock.unlock() } private def markPartitionFailed(topicPartition: TopicPartition): Unit = { diff --git a/core/src/main/scala/kafka/server/ReplicaAlterLogDirsThread.scala b/core/src/main/scala/kafka/server/ReplicaAlterLogDirsThread.scala index 89c159b5ffac6..e266c5b09cde2 100644 --- a/core/src/main/scala/kafka/server/ReplicaAlterLogDirsThread.scala +++ b/core/src/main/scala/kafka/server/ReplicaAlterLogDirsThread.scala @@ -20,7 +20,7 @@ package kafka.server import java.util import java.util.Optional -import kafka.api.{ApiVersion, Request} +import kafka.api.Request import kafka.cluster.BrokerEndPoint import kafka.log.{LeaderOffsetIncremented, LogAppendInfo} import kafka.server.AbstractFetcherThread.ReplicaFetch @@ -184,7 +184,7 @@ class ReplicaAlterLogDirsThread(name: String, override protected def isOffsetForLeaderEpochSupported: Boolean = true - override protected def isTruncationOnFetchSupported: Boolean = ApiVersion.isTruncationOnFetchSupported(brokerConfig.interBrokerProtocolVersion) + override protected def isTruncationOnFetchSupported: Boolean = false /** * Truncate the log for each partition based on current replica's returned epoch and offset. diff --git a/core/src/main/scala/kafka/server/ReplicaManager.scala b/core/src/main/scala/kafka/server/ReplicaManager.scala index 77bf028a67fb6..fdfd36dc1dc21 100644 --- a/core/src/main/scala/kafka/server/ReplicaManager.scala +++ b/core/src/main/scala/kafka/server/ReplicaManager.scala @@ -770,7 +770,7 @@ class ReplicaManager(val config: KafkaConfig, logManager.abortAndPauseCleaning(topicPartition) val initialFetchState = InitialFetchState(BrokerEndPoint(config.brokerId, "localhost", -1), - partition.getLeaderEpoch, futureLog.highWatermark, lastFetchedEpoch = futureLog.latestEpoch) + partition.getLeaderEpoch, futureLog.highWatermark, lastFetchedEpoch = None) replicaAlterLogDirsManager.addFetcherForPartitions(Map(topicPartition -> initialFetchState)) } @@ -1484,7 +1484,7 @@ class ReplicaManager(val config: KafkaConfig, logManager.abortAndPauseCleaning(topicPartition) futureReplicasAndInitialOffset.put(topicPartition, InitialFetchState(leader, - partition.getLeaderEpoch, log.highWatermark, lastFetchedEpoch = log.latestEpoch)) + partition.getLeaderEpoch, log.highWatermark, lastFetchedEpoch = None)) } } } diff --git a/core/src/test/scala/integration/kafka/server/DynamicBrokerReconfigurationTest.scala b/core/src/test/scala/integration/kafka/server/DynamicBrokerReconfigurationTest.scala index 6abcde85d9b96..5c38fe27d3984 100644 --- a/core/src/test/scala/integration/kafka/server/DynamicBrokerReconfigurationTest.scala +++ b/core/src/test/scala/integration/kafka/server/DynamicBrokerReconfigurationTest.scala @@ -850,12 +850,8 @@ class DynamicBrokerReconfigurationTest extends ZooKeeperTestHarness with SaslSet assertEquals(1, fetcherThreads.size) assertEquals(replicaFetcherManager.getFetcherId(tp), fetcherThreads.head._1.fetcherId) val thread = fetcherThreads.head._2 - if (!thread.isTruncationOnFetchSupported) { - assertEquals(Some(truncationOffset), thread.fetchState(tp).map(_.fetchOffset)) - assertEquals(Some(Truncating), thread.fetchState(tp).map(_.state)) - } else { - assertEquals(Some(Fetching), thread.fetchState(tp).map(_.state)) - } + assertEquals(Some(truncationOffset), thread.fetchState(tp).map(_.fetchOffset)) + assertEquals(Some(Truncating), thread.fetchState(tp).map(_.state)) } } } From 42c536ebd17fb4feb8d3bd5aadc3acec0ffdc8b9 Mon Sep 17 00:00:00 2001 From: Rajini Sivaram Date: Thu, 5 Nov 2020 15:18:49 +0000 Subject: [PATCH 07/11] Address review comments --- core/src/main/scala/kafka/log/Log.scala | 13 +++++++------ .../scala/kafka/server/AbstractFetcherThread.scala | 13 ++++++------- .../main/scala/kafka/server/ReplicaManager.scala | 2 +- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/core/src/main/scala/kafka/log/Log.scala b/core/src/main/scala/kafka/log/Log.scala index 2abfcf6cc7c9b..b325adb778f6c 100644 --- a/core/src/main/scala/kafka/log/Log.scala +++ b/core/src/main/scala/kafka/log/Log.scala @@ -1385,7 +1385,7 @@ class Log(@volatile private var _dir: File, var validBytesCount = 0 var firstOffset: Option[Long] = None var lastOffset = -1L - var lastLeaderEpoch: Option[Int] = None + var lastLeaderEpoch = RecordBatch.NO_PARTITION_LEADER_EPOCH var sourceCodec: CompressionCodec = NoCompressionCodec var monotonic = true var maxTimestamp = RecordBatch.NO_TIMESTAMP @@ -1418,10 +1418,7 @@ class Log(@volatile private var _dir: File, // update the last offset seen lastOffset = batch.lastOffset - lastLeaderEpoch = if (batch.partitionLeaderEpoch != RecordBatch.NO_PARTITION_LEADER_EPOCH) - Some(batch.partitionLeaderEpoch) - else - None + lastLeaderEpoch = batch.partitionLeaderEpoch // Check if the message sizes are valid. val batchSize = batch.sizeInBytes @@ -1453,7 +1450,11 @@ class Log(@volatile private var _dir: File, // Apply broker-side compression if any val targetCodec = BrokerCompressionCodec.getTargetCompressionCodec(config.compressionType, sourceCodec) - LogAppendInfo(firstOffset, lastOffset, lastLeaderEpoch, maxTimestamp, offsetOfMaxTimestamp, RecordBatch.NO_TIMESTAMP, logStartOffset, + val lastLeaderEpochOpt: Option[Int] = if (lastLeaderEpoch != RecordBatch.NO_PARTITION_LEADER_EPOCH) + Some(lastLeaderEpoch) + else + None + LogAppendInfo(firstOffset, lastOffset, lastLeaderEpochOpt, maxTimestamp, offsetOfMaxTimestamp, RecordBatch.NO_TIMESTAMP, logStartOffset, RecordConversionStats.EMPTY, sourceCodec, targetCodec, shallowMessageCount, validBytesCount, monotonic, lastOffsetOfFirstBatch) } diff --git a/core/src/main/scala/kafka/server/AbstractFetcherThread.scala b/core/src/main/scala/kafka/server/AbstractFetcherThread.scala index 01b49d947f9d4..cc11848f4db75 100755 --- a/core/src/main/scala/kafka/server/AbstractFetcherThread.scala +++ b/core/src/main/scala/kafka/server/AbstractFetcherThread.scala @@ -459,8 +459,8 @@ abstract class AbstractFetcherThread(name: String, private def partitionFetchState(tp: TopicPartition, initialFetchState: InitialFetchState, currentState: PartitionFetchState): PartitionFetchState = { if (currentState != null && currentState.currentLeaderEpoch == initialFetchState.currentLeaderEpoch) { currentState - } else if (isTruncationOnFetchSupported && initialFetchState.initOffset >= 0 && initialFetchState.lastFetchedEpoch.nonEmpty && - (currentState == null || currentState.state == Fetching)) { + } else if (isTruncationOnFetchSupported && initialFetchState.initOffset >= 0 && + initialFetchState.lastFetchedEpoch.nonEmpty && currentState == null) { PartitionFetchState(initialFetchState.initOffset, None, initialFetchState.currentLeaderEpoch, state = Fetching, initialFetchState.lastFetchedEpoch) } else if (initialFetchState.initOffset < 0) { @@ -501,12 +501,11 @@ abstract class AbstractFetcherThread(name: String, .map { case (topicPartition, currentFetchState) => val maybeTruncationComplete = fetchOffsets.get(topicPartition) match { case Some(offsetTruncationState) => - val (state, lastFetchedEpoch) = if (offsetTruncationState.truncationCompleted) - (Fetching, latestEpoch(topicPartition)) - else if (maySkipTruncation && currentFetchState.lastFetchedEpoch.nonEmpty) - (Fetching, currentFetchState.lastFetchedEpoch) + val state = if ((maySkipTruncation && currentFetchState.lastFetchedEpoch.nonEmpty) || offsetTruncationState.truncationCompleted) + Fetching else - (Truncating, latestEpoch(topicPartition)) + Truncating + val lastFetchedEpoch = latestEpoch(topicPartition) PartitionFetchState(offsetTruncationState.offset, currentFetchState.lag, currentFetchState.currentLeaderEpoch, currentFetchState.delay, state, lastFetchedEpoch) case None => currentFetchState diff --git a/core/src/main/scala/kafka/server/ReplicaManager.scala b/core/src/main/scala/kafka/server/ReplicaManager.scala index fdfd36dc1dc21..3e62a92e609ed 100644 --- a/core/src/main/scala/kafka/server/ReplicaManager.scala +++ b/core/src/main/scala/kafka/server/ReplicaManager.scala @@ -1670,7 +1670,7 @@ class ReplicaManager(val config: KafkaConfig, val log = partition.localLogOrException val (fetchOffset, lastFetchedEpoch) = initialFetchOffsetAndEpoch(log) partition.topicPartition -> InitialFetchState(leader, partition.getLeaderEpoch, fetchOffset, lastFetchedEpoch) - }.toMap + }.toMap replicaFetcherManager.addFetcherForPartitions(partitionsToMakeFollowerWithLeaderAndOffset) } From 3a13fa8bad0d1a2697475513df5c6370d4572a8d Mon Sep 17 00:00:00 2001 From: Rajini Sivaram Date: Thu, 5 Nov 2020 19:48:45 +0000 Subject: [PATCH 08/11] Address review comment --- .../kafka/server/AbstractFetcherThread.scala | 15 +-- .../server/ReplicaFetcherThreadTest.scala | 109 +++++++++++++++++- .../util/ReplicaFetcherMockBlockingSend.scala | 18 ++- 3 files changed, 126 insertions(+), 16 deletions(-) diff --git a/core/src/main/scala/kafka/server/AbstractFetcherThread.scala b/core/src/main/scala/kafka/server/AbstractFetcherThread.scala index cc11848f4db75..4629c02368f78 100755 --- a/core/src/main/scala/kafka/server/AbstractFetcherThread.scala +++ b/core/src/main/scala/kafka/server/AbstractFetcherThread.scala @@ -223,7 +223,7 @@ abstract class AbstractFetcherThread(name: String, val ResultWithPartitions(fetchOffsets, partitionsWithError) = maybeTruncateToEpochEndOffsets(epochEndOffsets, latestEpochsForPartitions) handlePartitionsWithErrors(partitionsWithError, "truncateToEpochEndOffsets") - updateFetchOffsetAndMaybeMarkTruncationComplete(fetchOffsets, isTruncationOnFetchSupported) + updateFetchOffsetAndMaybeMarkTruncationComplete(fetchOffsets) } } @@ -231,7 +231,7 @@ abstract class AbstractFetcherThread(name: String, inLock(partitionMapLock) { val ResultWithPartitions(fetchOffsets, partitionsWithError) = maybeTruncateToEpochEndOffsets(epochEndOffsets, Map.empty) handlePartitionsWithErrors(partitionsWithError, "truncateOnFetchResponse") - updateFetchOffsetAndMaybeMarkTruncationComplete(fetchOffsets, maySkipTruncation = false) + updateFetchOffsetAndMaybeMarkTruncationComplete(fetchOffsets) } } @@ -251,7 +251,7 @@ abstract class AbstractFetcherThread(name: String, } } - updateFetchOffsetAndMaybeMarkTruncationComplete(fetchOffsets, isTruncationOnFetchSupported) + updateFetchOffsetAndMaybeMarkTruncationComplete(fetchOffsets) } private def maybeTruncateToEpochEndOffsets(fetchedEpochs: Map[TopicPartition, EpochEndOffset], @@ -492,20 +492,17 @@ abstract class AbstractFetcherThread(name: String, * truncation completed if their offsetTruncationState indicates truncation completed * * @param fetchOffsets the partitions to update fetch offset and maybe mark truncation complete - * @param maySkipTruncation true if we can stay in Fetching mode and perform truncation later based on - * diverging epochs from fetch responses. */ - private def updateFetchOffsetAndMaybeMarkTruncationComplete(fetchOffsets: Map[TopicPartition, OffsetTruncationState], - maySkipTruncation: Boolean): Unit = { + private def updateFetchOffsetAndMaybeMarkTruncationComplete(fetchOffsets: Map[TopicPartition, OffsetTruncationState]): Unit = { val newStates: Map[TopicPartition, PartitionFetchState] = partitionStates.partitionStateMap.asScala .map { case (topicPartition, currentFetchState) => val maybeTruncationComplete = fetchOffsets.get(topicPartition) match { case Some(offsetTruncationState) => - val state = if ((maySkipTruncation && currentFetchState.lastFetchedEpoch.nonEmpty) || offsetTruncationState.truncationCompleted) + val lastFetchedEpoch = latestEpoch(topicPartition) + val state = if (isTruncationOnFetchSupported || offsetTruncationState.truncationCompleted) Fetching else Truncating - val lastFetchedEpoch = latestEpoch(topicPartition) PartitionFetchState(offsetTruncationState.offset, currentFetchState.lag, currentFetchState.currentLeaderEpoch, currentFetchState.delay, state, lastFetchedEpoch) case None => currentFetchState diff --git a/core/src/test/scala/unit/kafka/server/ReplicaFetcherThreadTest.scala b/core/src/test/scala/unit/kafka/server/ReplicaFetcherThreadTest.scala index 929ffe69732d7..b98e309d7f4f9 100644 --- a/core/src/test/scala/unit/kafka/server/ReplicaFetcherThreadTest.scala +++ b/core/src/test/scala/unit/kafka/server/ReplicaFetcherThreadTest.scala @@ -22,11 +22,12 @@ import java.util.{Collections, Optional} import kafka.api.{ApiVersion, KAFKA_2_6_IV0} import kafka.cluster.{BrokerEndPoint, Partition} -import kafka.log.{Log, LogManager} +import kafka.log.{Log, LogAppendInfo, LogManager} import kafka.server.QuotaFactory.UnboundedQuota import kafka.server.epoch.util.ReplicaFetcherMockBlockingSend import kafka.utils.TestUtils import org.apache.kafka.common.TopicPartition +import org.apache.kafka.common.message.FetchResponseData import org.apache.kafka.common.metrics.Metrics import org.apache.kafka.common.protocol.Errors._ import org.apache.kafka.common.protocol.{ApiKeys, Errors} @@ -515,11 +516,12 @@ class ReplicaFetcherThreadTest { @Test def shouldFetchLeaderEpochSecondTimeIfLeaderRepliesWithEpochNotKnownToFollower(): Unit = { - // Create a capture to track what partitions/offsets are truncated val truncateToCapture: Capture[Long] = newCapture(CaptureType.ALL) - val config = KafkaConfig.fromProps(TestUtils.createBrokerConfig(1, "localhost:1234")) + val props = TestUtils.createBrokerConfig(1, "localhost:1234") + props.setProperty(KafkaConfig.InterBrokerProtocolVersionProp, KAFKA_2_6_IV0.version) + val config = KafkaConfig.fromProps(props) // Setup all dependencies val quota: ReplicationQuotaManager = createNiceMock(classOf[ReplicationQuotaManager]) @@ -585,6 +587,107 @@ class ReplicaFetcherThreadTest { truncateToCapture.getValues.asScala.contains(101)) } + @Test + def shouldTruncateIfLeaderRepliesWithDivergingEpochNotKnownToFollower(): Unit = { + + // Create a capture to track what partitions/offsets are truncated + val truncateToCapture: Capture[Long] = newCapture(CaptureType.ALL) + + val config = KafkaConfig.fromProps(TestUtils.createBrokerConfig(1, "localhost:1234")) + + // Setup all dependencies + val quota: ReplicationQuotaManager = createNiceMock(classOf[ReplicationQuotaManager]) + val logManager: LogManager = createMock(classOf[LogManager]) + val replicaAlterLogDirsManager: ReplicaAlterLogDirsManager = createMock(classOf[ReplicaAlterLogDirsManager]) + val log: Log = createNiceMock(classOf[Log]) + val partition: Partition = createNiceMock(classOf[Partition]) + val replicaManager: ReplicaManager = createMock(classOf[ReplicaManager]) + + val initialLEO = 200 + var latestLogEpoch: Option[Int] = Some(5) + + // Stubs + expect(partition.truncateTo(capture(truncateToCapture), anyBoolean())).anyTimes() + expect(partition.localLogOrException).andReturn(log).anyTimes() + expect(log.highWatermark).andReturn(115).anyTimes() + expect(log.latestEpoch).andAnswer(() => latestLogEpoch).anyTimes() + expect(log.endOffsetForEpoch(4)).andReturn(Some(OffsetAndEpoch(149, 4))).anyTimes() + expect(log.endOffsetForEpoch(3)).andReturn(Some(OffsetAndEpoch(129, 2))).anyTimes() + expect(log.endOffsetForEpoch(2)).andReturn(Some(OffsetAndEpoch(119, 1))).anyTimes() + expect(log.logEndOffset).andReturn(initialLEO).anyTimes() + expect(replicaManager.localLogOrException(anyObject(classOf[TopicPartition]))).andReturn(log).anyTimes() + expect(replicaManager.logManager).andReturn(logManager).anyTimes() + expect(replicaManager.replicaAlterLogDirsManager).andReturn(replicaAlterLogDirsManager).anyTimes() + expect(replicaManager.brokerTopicStats).andReturn(mock(classOf[BrokerTopicStats])) + stub(partition, replicaManager, log) + + replay(replicaManager, logManager, quota, partition, log) + + // Create the fetcher thread + val mockNetwork = new ReplicaFetcherMockBlockingSend(Collections.emptyMap(), brokerEndPoint, new SystemTime()) + val thread = new ReplicaFetcherThread("bob", 0, brokerEndPoint, config, failedPartitions, replicaManager, new Metrics(), new SystemTime(), quota, Some(mockNetwork)) { + override def processPartitionData(topicPartition: TopicPartition, fetchOffset: Long, partitionData: FetchData): Option[LogAppendInfo] = None + } + thread.addPartitions(Map(t1p0 -> initialFetchState(initialLEO, lastFetchedEpoch = Some(5)), t1p1 -> initialFetchState(initialLEO, lastFetchedEpoch = Some(5)))) + val partitions = Set(t1p0, t1p1) + + // Loop 1 -- both topic partitions skip epoch fetch and send fetch request since + // lastFetchedEpoch is set in initial fetch state. + thread.doWork() + assertEquals(0, mockNetwork.epochFetchCount) + assertEquals(1, mockNetwork.fetchCount) + partitions.foreach { tp => assertEquals(Fetching, thread.fetchState(tp).get.state) } + + def partitionData(divergingEpoch: FetchResponseData.EpochEndOffset): FetchResponse.PartitionData[Records] = { + new FetchResponse.PartitionData[Records]( + Errors.NONE, 0, 0, 0, Optional.empty(), Collections.emptyList(), + Optional.of(divergingEpoch), MemoryRecords.EMPTY) + } + + // Loop 2 should truncate based on diverging epoch and continue to send fetch requests. + mockNetwork.setFetchPartitionDataForNextResponse(Map( + t1p0 -> partitionData(new FetchResponseData.EpochEndOffset().setEpoch(4).setEndOffset(140)), + t1p1 -> partitionData(new FetchResponseData.EpochEndOffset().setEpoch(4).setEndOffset(141)) + )) + latestLogEpoch = Some(4) + thread.doWork() + assertEquals(0, mockNetwork.epochFetchCount) + assertEquals(2, mockNetwork.fetchCount) + assertTrue("Expected " + t1p0 + " to truncate to offset 140 (truncation offsets: " + truncateToCapture.getValues + ")", + truncateToCapture.getValues.asScala.contains(140)) + assertTrue("Expected " + t1p1 + " to truncate to offset 141 (truncation offsets: " + truncateToCapture.getValues + ")", + truncateToCapture.getValues.asScala.contains(141)) + partitions.foreach { tp => assertEquals(Fetching, thread.fetchState(tp).get.state) } + + // Loop 3 should truncate because of diverging epoch. Offset truncation is not complete + // because divergent epoch is not known to follower. We truncate and stay in Fetching state. + mockNetwork.setFetchPartitionDataForNextResponse(Map( + t1p0 -> partitionData(new FetchResponseData.EpochEndOffset().setEpoch(3).setEndOffset(130)), + t1p1 -> partitionData(new FetchResponseData.EpochEndOffset().setEpoch(3).setEndOffset(131)) + )) + thread.doWork() + assertEquals(0, mockNetwork.epochFetchCount) + assertEquals(3, mockNetwork.fetchCount) + assertTrue("Expected to truncate to offset 129 (truncation offsets: " + truncateToCapture.getValues + ")", + truncateToCapture.getValues.asScala.contains(129)) + partitions.foreach { tp => assertEquals(Fetching, thread.fetchState(tp).get.state) } + + // Loop 4 should truncate because of diverging epoch. Offset truncation is not complete + // because divergent epoch is not known to follower. Last fetched epoch cannot be determined + // from the log. We truncate and stay in Fetching state. + mockNetwork.setFetchPartitionDataForNextResponse(Map( + t1p0 -> partitionData(new FetchResponseData.EpochEndOffset().setEpoch(2).setEndOffset(120)), + t1p1 -> partitionData(new FetchResponseData.EpochEndOffset().setEpoch(2).setEndOffset(121)) + )) + latestLogEpoch = None + thread.doWork() + assertEquals(0, mockNetwork.epochFetchCount) + assertEquals(4, mockNetwork.fetchCount) + assertTrue("Expected to truncate to offset 119 (truncation offsets: " + truncateToCapture.getValues + ")", + truncateToCapture.getValues.asScala.contains(119)) + partitions.foreach { tp => assertEquals(Fetching, thread.fetchState(tp).get.state) } + } + @Test def shouldUseLeaderEndOffsetIfInterBrokerVersionBelow20(): Unit = { diff --git a/core/src/test/scala/unit/kafka/server/epoch/util/ReplicaFetcherMockBlockingSend.scala b/core/src/test/scala/unit/kafka/server/epoch/util/ReplicaFetcherMockBlockingSend.scala index f8c528b29108d..a58743025f0be 100644 --- a/core/src/test/scala/unit/kafka/server/epoch/util/ReplicaFetcherMockBlockingSend.scala +++ b/core/src/test/scala/unit/kafka/server/epoch/util/ReplicaFetcherMockBlockingSend.scala @@ -31,6 +31,8 @@ import org.apache.kafka.common.requests.{AbstractRequest, EpochEndOffset, FetchR import org.apache.kafka.common.utils.{SystemTime, Time} import org.apache.kafka.common.{Node, TopicPartition} +import scala.collection.Map + /** * Stub network client used for testing the ReplicaFetcher, wraps the MockClient used for consumer testing * @@ -53,17 +55,22 @@ class ReplicaFetcherMockBlockingSend(offsets: java.util.Map[TopicPartition, Epoc var epochFetchCount = 0 var lastUsedOffsetForLeaderEpochVersion = -1 var callback: Option[() => Unit] = None - var currentOffsets: java.util.Map[TopicPartition, EpochEndOffset] = offsets + var currentOffsets: util.Map[TopicPartition, EpochEndOffset] = offsets + var fetchPartitionData: Map[TopicPartition, FetchResponse.PartitionData[Records]] = Map.empty private val sourceNode = new Node(sourceBroker.id, sourceBroker.host, sourceBroker.port) def setEpochRequestCallback(postEpochFunction: () => Unit): Unit = { callback = Some(postEpochFunction) } - def setOffsetsForNextResponse(newOffsets: java.util.Map[TopicPartition, EpochEndOffset]): Unit = { + def setOffsetsForNextResponse(newOffsets: util.Map[TopicPartition, EpochEndOffset]): Unit = { currentOffsets = newOffsets } + def setFetchPartitionDataForNextResponse(partitionData: Map[TopicPartition, FetchResponse.PartitionData[Records]]): Unit = { + fetchPartitionData = partitionData + } + override def sendRequest(requestBuilder: Builder[_ <: AbstractRequest]): ClientResponse = { if (!NetworkClientUtils.awaitReady(client, sourceNode, time, 500)) throw new SocketTimeoutException(s"Failed to connect within 500 ms") @@ -82,8 +89,11 @@ class ReplicaFetcherMockBlockingSend(offsets: java.util.Map[TopicPartition, Epoc case ApiKeys.FETCH => fetchCount += 1 - new FetchResponse(Errors.NONE, new java.util.LinkedHashMap[TopicPartition, FetchResponse.PartitionData[Records]], 0, - JFetchMetadata.INVALID_SESSION_ID) + val partitionData = new util.LinkedHashMap[TopicPartition, FetchResponse.PartitionData[Records]] + fetchPartitionData.foreach { case (tp, data) => partitionData.put(tp, data) } + fetchPartitionData = Map.empty + new FetchResponse(Errors.NONE, partitionData, 0, + if (partitionData.isEmpty) JFetchMetadata.INVALID_SESSION_ID else 1) case _ => throw new UnsupportedOperationException From bf976df9fb3201c9f996ec71f8b325fc4a13c05c Mon Sep 17 00:00:00 2001 From: Rajini Sivaram Date: Wed, 2 Dec 2020 11:50:48 +0000 Subject: [PATCH 09/11] Address review comments --- .../kafka/server/AbstractFetcherManager.scala | 6 +- .../kafka/server/AbstractFetcherThread.scala | 30 +-- .../server/ReplicaAlterLogDirsThread.scala | 4 +- .../kafka/server/ReplicaFetcherThread.scala | 8 +- .../scala/kafka/server/ReplicaManager.scala | 21 +- .../server/AbstractFetcherManagerTest.scala | 6 +- .../server/AbstractFetcherThreadTest.scala | 25 ++- .../ReplicaAlterLogDirsThreadTest.scala | 2 +- .../server/ReplicaFetcherThreadTest.scala | 191 ++++-------------- .../kafka/server/ReplicaManagerTest.scala | 2 +- .../ReplicaFetcherThreadBenchmark.java | 2 +- 11 files changed, 93 insertions(+), 204 deletions(-) diff --git a/core/src/main/scala/kafka/server/AbstractFetcherManager.scala b/core/src/main/scala/kafka/server/AbstractFetcherManager.scala index 483a58906d8df..b48253127616a 100755 --- a/core/src/main/scala/kafka/server/AbstractFetcherManager.scala +++ b/core/src/main/scala/kafka/server/AbstractFetcherManager.scala @@ -64,8 +64,8 @@ abstract class AbstractFetcherManager[T <: AbstractFetcherThread](val name: Stri def resizeThreadPool(newSize: Int): Unit = { def migratePartitions(newSize: Int): Unit = { fetcherThreadMap.forKeyValue { (id, thread) => - val removedPartitions = thread.partitionsAndOffsets - removeFetcherForPartitions(removedPartitions.keySet) + val removedPartitions = thread.removeAllPartitions() + removeFetcherForPartitions(removedPartitions.keySet) // clear state for removed partitions if (id.fetcherId >= newSize) thread.shutdown() addFetcherForPartitions(removedPartitions) @@ -223,6 +223,6 @@ class FailedPartitions { case class BrokerAndFetcherId(broker: BrokerEndPoint, fetcherId: Int) -case class InitialFetchState(leader: BrokerEndPoint, currentLeaderEpoch: Int, initOffset: Long, lastFetchedEpoch: Option[Int]) +case class InitialFetchState(leader: BrokerEndPoint, currentLeaderEpoch: Int, initOffset: Long) case class BrokerIdAndFetcherId(brokerId: Int, fetcherId: Int) diff --git a/core/src/main/scala/kafka/server/AbstractFetcherThread.scala b/core/src/main/scala/kafka/server/AbstractFetcherThread.scala index 4629c02368f78..cf20b7b42dc08 100755 --- a/core/src/main/scala/kafka/server/AbstractFetcherThread.scala +++ b/core/src/main/scala/kafka/server/AbstractFetcherThread.scala @@ -99,9 +99,9 @@ abstract class AbstractFetcherThread(name: String, protected def fetchLatestOffsetFromLeader(topicPartition: TopicPartition, currentLeaderEpoch: Int): Long - protected def isOffsetForLeaderEpochSupported: Boolean + protected val isOffsetForLeaderEpochSupported: Boolean - protected def isTruncationOnFetchSupported: Boolean + protected val isTruncationOnFetchSupported: Boolean override def shutdown(): Unit = { initiateShutdown() @@ -227,7 +227,7 @@ abstract class AbstractFetcherThread(name: String, } } - private def truncateOnFetchResponse(epochEndOffsets: Map[TopicPartition, EpochEndOffset]): Unit = { + protected def truncateOnFetchResponse(epochEndOffsets: Map[TopicPartition, EpochEndOffset]): Unit = { inLock(partitionMapLock) { val ResultWithPartitions(fetchOffsets, partitionsWithError) = maybeTruncateToEpochEndOffsets(epochEndOffsets, Map.empty) handlePartitionsWithErrors(partitionsWithError, "truncateOnFetchResponse") @@ -459,12 +459,13 @@ abstract class AbstractFetcherThread(name: String, private def partitionFetchState(tp: TopicPartition, initialFetchState: InitialFetchState, currentState: PartitionFetchState): PartitionFetchState = { if (currentState != null && currentState.currentLeaderEpoch == initialFetchState.currentLeaderEpoch) { currentState - } else if (isTruncationOnFetchSupported && initialFetchState.initOffset >= 0 && - initialFetchState.lastFetchedEpoch.nonEmpty && currentState == null) { - PartitionFetchState(initialFetchState.initOffset, None, initialFetchState.currentLeaderEpoch, - state = Fetching, initialFetchState.lastFetchedEpoch) } else if (initialFetchState.initOffset < 0) { fetchOffsetAndTruncate(tp, initialFetchState.currentLeaderEpoch) + } else if (isTruncationOnFetchSupported) { + val lastFetchedEpoch = latestEpoch(tp) + val state = if (lastFetchedEpoch.exists(_ != EpochEndOffset.UNDEFINED_EPOCH)) Fetching else Truncating + PartitionFetchState(initialFetchState.initOffset, None, initialFetchState.currentLeaderEpoch, + state, lastFetchedEpoch) } else { PartitionFetchState(initialFetchState.initOffset, None, initialFetchState.currentLeaderEpoch, state = Truncating, lastFetchedEpoch = None) @@ -715,19 +716,18 @@ abstract class AbstractFetcherThread(name: String, } /** - * Returns current fetch state for each partition assigned to this thread. This is used to reassign - * partitions when thread pool is resized. We return `lastFetchedEpoch=None` to ensure we go through - * the truncation path to simplify the code path where this thread makes progress after this method - * before the thread is shutdown. + * Removes all partitions assigned to this thread and returns current fetch state + * for each partition. This is used to reassign partitions when thread pool is resized. */ - private[server] def partitionsAndOffsets: Map[TopicPartition, InitialFetchState] = inLock(partitionMapLock) { - partitionStates.partitionStateMap.asScala.map { case (topicPartition, currentFetchState) => + private[server] def removeAllPartitions(): Map[TopicPartition, InitialFetchState] = inLock(partitionMapLock) { + val fetchStates = partitionStates.partitionStateMap.asScala.map { case (topicPartition, currentFetchState) => val initialFetchState = InitialFetchState(sourceBroker, currentLeaderEpoch = currentFetchState.currentLeaderEpoch, - initOffset = currentFetchState.fetchOffset, - lastFetchedEpoch = None) + initOffset = currentFetchState.fetchOffset) topicPartition -> initialFetchState } + partitionStates.clear() + fetchStates } protected def toMemoryRecords(records: Records): MemoryRecords = { diff --git a/core/src/main/scala/kafka/server/ReplicaAlterLogDirsThread.scala b/core/src/main/scala/kafka/server/ReplicaAlterLogDirsThread.scala index e266c5b09cde2..0812f878c936e 100644 --- a/core/src/main/scala/kafka/server/ReplicaAlterLogDirsThread.scala +++ b/core/src/main/scala/kafka/server/ReplicaAlterLogDirsThread.scala @@ -182,9 +182,9 @@ class ReplicaAlterLogDirsThread(name: String, } } - override protected def isOffsetForLeaderEpochSupported: Boolean = true + override protected val isOffsetForLeaderEpochSupported: Boolean = true - override protected def isTruncationOnFetchSupported: Boolean = false + override protected val isTruncationOnFetchSupported: Boolean = false /** * Truncate the log for each partition based on current replica's returned epoch and offset. diff --git a/core/src/main/scala/kafka/server/ReplicaFetcherThread.scala b/core/src/main/scala/kafka/server/ReplicaFetcherThread.scala index d60b47ba9281f..2db3d6aa61b7a 100644 --- a/core/src/main/scala/kafka/server/ReplicaFetcherThread.scala +++ b/core/src/main/scala/kafka/server/ReplicaFetcherThread.scala @@ -104,8 +104,8 @@ class ReplicaFetcherThread(name: String, private val minBytes = brokerConfig.replicaFetchMinBytes private val maxBytes = brokerConfig.replicaFetchResponseMaxBytes private val fetchSize = brokerConfig.replicaFetchMaxBytes - private val brokerSupportsLeaderEpochRequest = brokerConfig.interBrokerProtocolVersion >= KAFKA_0_11_0_IV2 - private val brokerSupportsTruncationOnFetch = ApiVersion.isTruncationOnFetchSupported(brokerConfig.interBrokerProtocolVersion) + override protected val isOffsetForLeaderEpochSupported: Boolean = brokerConfig.interBrokerProtocolVersion >= KAFKA_0_11_0_IV2 + override protected val isTruncationOnFetchSupported = ApiVersion.isTruncationOnFetchSupported(brokerConfig.interBrokerProtocolVersion) val fetchSessionHandler = new FetchSessionHandler(logContext, sourceBroker.id) override protected def latestEpoch(topicPartition: TopicPartition): Option[Int] = { @@ -355,10 +355,6 @@ class ReplicaFetcherThread(name: String, } } - override def isOffsetForLeaderEpochSupported: Boolean = brokerSupportsLeaderEpochRequest - - override def isTruncationOnFetchSupported: Boolean = brokerSupportsTruncationOnFetch - /** * To avoid ISR thrashing, we only throttle a replica on the follower if it's in the throttled replica list, * the quota is exceeded and the replica is not in sync. diff --git a/core/src/main/scala/kafka/server/ReplicaManager.scala b/core/src/main/scala/kafka/server/ReplicaManager.scala index 3e62a92e609ed..23da8bb37a330 100644 --- a/core/src/main/scala/kafka/server/ReplicaManager.scala +++ b/core/src/main/scala/kafka/server/ReplicaManager.scala @@ -770,7 +770,7 @@ class ReplicaManager(val config: KafkaConfig, logManager.abortAndPauseCleaning(topicPartition) val initialFetchState = InitialFetchState(BrokerEndPoint(config.brokerId, "localhost", -1), - partition.getLeaderEpoch, futureLog.highWatermark, lastFetchedEpoch = None) + partition.getLeaderEpoch, futureLog.highWatermark) replicaAlterLogDirsManager.addFetcherForPartitions(Map(topicPartition -> initialFetchState)) } @@ -1484,7 +1484,7 @@ class ReplicaManager(val config: KafkaConfig, logManager.abortAndPauseCleaning(topicPartition) futureReplicasAndInitialOffset.put(topicPartition, InitialFetchState(leader, - partition.getLeaderEpoch, log.highWatermark, lastFetchedEpoch = None)) + partition.getLeaderEpoch, log.highWatermark)) } } } @@ -1668,8 +1668,8 @@ class ReplicaManager(val config: KafkaConfig, val leader = metadataCache.getAliveBrokers.find(_.id == partition.leaderReplicaIdOpt.get).get .brokerEndPoint(config.interBrokerListenerName) val log = partition.localLogOrException - val (fetchOffset, lastFetchedEpoch) = initialFetchOffsetAndEpoch(log) - partition.topicPartition -> InitialFetchState(leader, partition.getLeaderEpoch, fetchOffset, lastFetchedEpoch) + val fetchOffset = initialFetchOffset(log) + partition.topicPartition -> InitialFetchState(leader, partition.getLeaderEpoch, fetchOffset) }.toMap replicaFetcherManager.addFetcherForPartitions(partitionsToMakeFollowerWithLeaderAndOffset) @@ -1694,15 +1694,14 @@ class ReplicaManager(val config: KafkaConfig, /** * From IBP 2.7 onwards, we send latest fetch epoch in the request and truncate if a - * diverging epoch is returned in the response, avoiding the need for a separate OffsetForLeaderEpoch - * request. We use log end offset as the next fetch offset and include the corresponding epoch. + * diverging epoch is returned in the response, avoiding the need for a separate + * OffsetForLeaderEpoch request. */ - private def initialFetchOffsetAndEpoch(log: Log): (Long, Option[Int]) = { - val latestEpoch = log.latestEpoch - if (ApiVersion.isTruncationOnFetchSupported(config.interBrokerProtocolVersion) && latestEpoch.nonEmpty) - (log.logEndOffset, latestEpoch) + private def initialFetchOffset(log: Log): Long = { + if (ApiVersion.isTruncationOnFetchSupported(config.interBrokerProtocolVersion) && log.latestEpoch.nonEmpty) + log.logEndOffset else - (log.highWatermark, None) + log.highWatermark } private def maybeShrinkIsr(): Unit = { diff --git a/core/src/test/scala/unit/kafka/server/AbstractFetcherManagerTest.scala b/core/src/test/scala/unit/kafka/server/AbstractFetcherManagerTest.scala index 989a11191e033..497aa1ab334f5 100644 --- a/core/src/test/scala/unit/kafka/server/AbstractFetcherManagerTest.scala +++ b/core/src/test/scala/unit/kafka/server/AbstractFetcherManagerTest.scala @@ -54,8 +54,7 @@ class AbstractFetcherManagerTest { val initialFetchState = InitialFetchState( leader = new BrokerEndPoint(0, "localhost", 9092), currentLeaderEpoch = leaderEpoch, - initOffset = fetchOffset, - lastFetchedEpoch = None) + initOffset = fetchOffset) EasyMock.expect(fetcher.start()) EasyMock.expect(fetcher.addPartitions(Map(tp -> initialFetchState))) @@ -114,8 +113,7 @@ class AbstractFetcherManagerTest { val initialFetchState = InitialFetchState( leader = new BrokerEndPoint(0, "localhost", 9092), currentLeaderEpoch = leaderEpoch, - initOffset = fetchOffset, - lastFetchedEpoch = None) + initOffset = fetchOffset) EasyMock.expect(fetcher.start()) EasyMock.expect(fetcher.addPartitions(Map(tp -> initialFetchState))) diff --git a/core/src/test/scala/unit/kafka/server/AbstractFetcherThreadTest.scala b/core/src/test/scala/unit/kafka/server/AbstractFetcherThreadTest.scala index 53a479ecd023d..2a8b6ba9d3e50 100644 --- a/core/src/test/scala/unit/kafka/server/AbstractFetcherThreadTest.scala +++ b/core/src/test/scala/unit/kafka/server/AbstractFetcherThreadTest.scala @@ -66,9 +66,9 @@ class AbstractFetcherThreadTest { .batches.asScala.head } - private def initialFetchState(fetchOffset: Long, leaderEpoch: Int, lastFetchedEpoch: Option[Int] = None): InitialFetchState = { + private def initialFetchState(fetchOffset: Long, leaderEpoch: Int): InitialFetchState = { InitialFetchState(leader = new BrokerEndPoint(0, "localhost", 9092), - initOffset = fetchOffset, currentLeaderEpoch = leaderEpoch, lastFetchedEpoch = lastFetchedEpoch) + initOffset = fetchOffset, currentLeaderEpoch = leaderEpoch) } @Test @@ -311,9 +311,9 @@ class AbstractFetcherThreadTest { override def fetchEpochEndOffsets(partitions: Map[TopicPartition, EpochData]): Map[TopicPartition, EpochEndOffset] = throw new UnsupportedOperationException - override protected def isOffsetForLeaderEpochSupported: Boolean = false + override protected val isOffsetForLeaderEpochSupported: Boolean = false - override protected def isTruncationOnFetchSupported: Boolean = false + override protected val isTruncationOnFetchSupported: Boolean = false } val replicaLog = Seq( @@ -526,6 +526,11 @@ class AbstractFetcherThreadTest { // initial truncation and verify that the log start offset is updated fetcher.doWork() + if (truncateOnFetch) { + // Second iteration required here since first iteration is required to + // perform initial truncaton based on diverging epoch. + fetcher.doWork() + } assertEquals(Option(Fetching), fetcher.fetchState(partition).map(_.state)) assertEquals(2, replicaState.logStartOffset) assertEquals(List(), replicaState.log.toList) @@ -813,7 +818,7 @@ class AbstractFetcherThreadTest { val replicaState = MockFetcherThread.PartitionState(replicaLog, leaderEpoch = 5, highWatermark = 0L) fetcher.setReplicaState(partition, replicaState) - fetcher.addPartitions(Map(partition -> initialFetchState(3L, leaderEpoch = 5, lastFetchedEpoch = Some(4)))) + fetcher.addPartitions(Map(partition -> initialFetchState(3L, leaderEpoch = 5))) assertEquals(3L, replicaState.logEndOffset) fetcher.verifyLastFetchedEpoch(partition, expectedEpoch = Some(4)) @@ -890,6 +895,12 @@ class AbstractFetcherThreadTest { partitionData: FetchData): Option[LogAppendInfo] = { val state = replicaPartitionState(topicPartition) + if (isTruncationOnFetchSupported && partitionData.divergingEpoch.isPresent) { + val divergingEpoch = partitionData.divergingEpoch.get + truncateOnFetchResponse(Map(topicPartition -> new EpochEndOffset(Errors.NONE, divergingEpoch.epoch, divergingEpoch.endOffset))) + return None + } + // Throw exception if the fetchOffset does not match the fetcherThread partition state if (fetchOffset != state.logEndOffset) throw new RuntimeException(s"Offset mismatch for partition $topicPartition: " + @@ -1055,9 +1066,9 @@ class AbstractFetcherThreadTest { endOffsets } - override protected def isOffsetForLeaderEpochSupported: Boolean = true + override protected val isOffsetForLeaderEpochSupported: Boolean = true - override protected def isTruncationOnFetchSupported: Boolean = truncateOnFetch + override protected val isTruncationOnFetchSupported: Boolean = truncateOnFetch override def fetchFromLeader(fetchRequest: FetchRequest.Builder): Map[TopicPartition, FetchData] = { fetchRequest.fetchData.asScala.map { case (partition, fetchData) => diff --git a/core/src/test/scala/unit/kafka/server/ReplicaAlterLogDirsThreadTest.scala b/core/src/test/scala/unit/kafka/server/ReplicaAlterLogDirsThreadTest.scala index 8a41f675db56b..2547c0c44b020 100644 --- a/core/src/test/scala/unit/kafka/server/ReplicaAlterLogDirsThreadTest.scala +++ b/core/src/test/scala/unit/kafka/server/ReplicaAlterLogDirsThreadTest.scala @@ -48,7 +48,7 @@ class ReplicaAlterLogDirsThreadTest { private def initialFetchState(fetchOffset: Long, leaderEpoch: Int = 1): InitialFetchState = { InitialFetchState(leader = new BrokerEndPoint(0, "localhost", 9092), - initOffset = fetchOffset, currentLeaderEpoch = leaderEpoch, lastFetchedEpoch = None) + initOffset = fetchOffset, currentLeaderEpoch = leaderEpoch) } @Test diff --git a/core/src/test/scala/unit/kafka/server/ReplicaFetcherThreadTest.scala b/core/src/test/scala/unit/kafka/server/ReplicaFetcherThreadTest.scala index b98e309d7f4f9..2e4d85cf58d8e 100644 --- a/core/src/test/scala/unit/kafka/server/ReplicaFetcherThreadTest.scala +++ b/core/src/test/scala/unit/kafka/server/ReplicaFetcherThreadTest.scala @@ -17,7 +17,6 @@ package kafka.server import java.nio.charset.StandardCharsets -import java.util import java.util.{Collections, Optional} import kafka.api.{ApiVersion, KAFKA_2_6_IV0} @@ -52,9 +51,9 @@ class ReplicaFetcherThreadTest { private val brokerEndPoint = new BrokerEndPoint(0, "localhost", 1000) private val failedPartitions = new FailedPartitions - private def initialFetchState(fetchOffset: Long, leaderEpoch: Int = 1, lastFetchedEpoch: Option[Int] = None): InitialFetchState = { + private def initialFetchState(fetchOffset: Long, leaderEpoch: Int = 1): InitialFetchState = { InitialFetchState(leader = new BrokerEndPoint(0, "localhost", 9092), - initOffset = fetchOffset, currentLeaderEpoch = leaderEpoch, lastFetchedEpoch = lastFetchedEpoch) + initOffset = fetchOffset, currentLeaderEpoch = leaderEpoch) } @After @@ -87,18 +86,7 @@ class ReplicaFetcherThreadTest { @Test def testFetchLeaderEpochRequestIfLastEpochDefinedForSomePartitions(): Unit = { - shouldFetchLeaderEpochRequestIfLastEpochDefinedForSomePartitions(ibp = ApiVersion.latestVersion) - } - - @Test - def testFetchLeaderEpochRequestIfLastEpochDefinedForSomePartitionsWithIbp26(): Unit = { - shouldFetchLeaderEpochRequestIfLastEpochDefinedForSomePartitions(KAFKA_2_6_IV0) - } - - private def shouldFetchLeaderEpochRequestIfLastEpochDefinedForSomePartitions(ibp: ApiVersion): Unit = { - val props = TestUtils.createBrokerConfig(1, "localhost:1234") - props.setProperty(KafkaConfig.InterBrokerProtocolVersionProp, ibp.version) - val config = KafkaConfig.fromProps(props) + val config = kafkaConfigNoTruncateOnFetch //Setup all dependencies val quota: ReplicationQuotaManager = createNiceMock(classOf[ReplicationQuotaManager]) @@ -117,8 +105,6 @@ class ReplicaFetcherThreadTest { expect(log.latestEpoch).andReturn(Some(leaderEpoch)).once() expect(log.latestEpoch).andReturn(Some(leaderEpoch)).once() expect(log.latestEpoch).andReturn(None).once() // t2p1 doesnt support epochs - if (ApiVersion.isTruncationOnFetchSupported(ibp)) - expect(log.latestEpoch).andReturn(Some(leaderEpoch)).times(3) // to set lastFetchedEpoch expect(log.endOffsetForEpoch(leaderEpoch)).andReturn( Some(OffsetAndEpoch(0, leaderEpoch))).anyTimes() expect(replicaManager.logManager).andReturn(logManager).anyTimes() @@ -140,7 +126,7 @@ class ReplicaFetcherThreadTest { val thread = new ReplicaFetcherThread("bob", 0, brokerEndPoint, config, failedPartitions, replicaManager, new Metrics(), new SystemTime(), quota, Some(mockNetwork)) - // topic 1 supports epoch, t2 doesn't. `lastFetchedEpoch` not set for any partition + // topic 1 supports epoch, t2 doesn't. thread.addPartitions(Map( t1p0 -> initialFetchState(0L), t1p1 -> initialFetchState(0L), @@ -172,122 +158,6 @@ class ReplicaFetcherThreadTest { verify(logManager) } - @Test - def testFetchLeaderEpochRequestIfLastFetchedEpochDefinedForSomePartitions(): Unit = { - verifyFetchLeaderEpochRequestWithLastFetchedEpoch(ApiVersion.latestVersion, hasUnknownLastFetchedEpoch = true) - } - - @Test - def testFetchLeaderEpochRequestIfLastFetchedEpochDefinedForAllPartitions(): Unit = { - verifyFetchLeaderEpochRequestWithLastFetchedEpoch(ApiVersion.latestVersion, hasUnknownLastFetchedEpoch = false) - } - - @Test - def testFetchLeaderEpochRequestIfLastFetchedEpochDefinedForSomePartitionsIbp26(): Unit = { - verifyFetchLeaderEpochRequestWithLastFetchedEpoch(KAFKA_2_6_IV0, hasUnknownLastFetchedEpoch = true) - } - - @Test - def testFetchLeaderEpochRequestIfLastFetchedEpochDefinedForAllPartitionsIbp26(): Unit = { - verifyFetchLeaderEpochRequestWithLastFetchedEpoch(KAFKA_2_6_IV0, hasUnknownLastFetchedEpoch = false) - } - - private def verifyFetchLeaderEpochRequestWithLastFetchedEpoch(ibp: ApiVersion, hasUnknownLastFetchedEpoch: Boolean): Unit = { - val props = TestUtils.createBrokerConfig(1, "localhost:1234") - props.setProperty(KafkaConfig.InterBrokerProtocolVersionProp, ibp.version) - val config = KafkaConfig.fromProps(props) - - //Setup all dependencies - val quota: ReplicationQuotaManager = createNiceMock(classOf[ReplicationQuotaManager]) - val logManager: LogManager = createMock(classOf[LogManager]) - val replicaAlterLogDirsManager: ReplicaAlterLogDirsManager = createMock(classOf[ReplicaAlterLogDirsManager]) - val log: Log = createNiceMock(classOf[Log]) - val partition: Partition = createMock(classOf[Partition]) - val replicaManager: ReplicaManager = createMock(classOf[ReplicaManager]) - - val leaderEpoch = 5 - - //Stubs - expect(partition.localLogOrException).andReturn(log).anyTimes() - expect(log.logEndOffset).andReturn(0).anyTimes() - expect(log.highWatermark).andReturn(0).anyTimes() - expect(log.latestEpoch).andReturn(Some(leaderEpoch)).anyTimes() - expect(log.endOffsetForEpoch(leaderEpoch)).andReturn( - Some(OffsetAndEpoch(0, leaderEpoch))).anyTimes() - expect(replicaManager.logManager).andReturn(logManager).anyTimes() - expect(replicaManager.replicaAlterLogDirsManager).andReturn(replicaAlterLogDirsManager).anyTimes() - expect(replicaManager.brokerTopicStats).andReturn(mock(classOf[BrokerTopicStats])) - stub(partition, replicaManager, log) - - //Expectations - expect(partition.truncateTo(anyLong(), anyBoolean())).times(3) - - replay(replicaManager, logManager, quota, partition, log) - - //Define the offsets for the OffsetsForLeaderEpochResponse - val offsets: util.Map[TopicPartition, EpochEndOffset] = if (!ApiVersion.isTruncationOnFetchSupported(ibp)) - Map(t1p0 -> new EpochEndOffset(leaderEpoch, 1), - t1p1 -> new EpochEndOffset(leaderEpoch, 1), - t2p1 -> new EpochEndOffset(leaderEpoch, 1)).asJava - else if (hasUnknownLastFetchedEpoch) - Collections.singletonMap(t2p1, new EpochEndOffset(leaderEpoch, 1)) - else - Collections.emptyMap[TopicPartition, EpochEndOffset] - - //Create the fetcher thread - val mockNetwork = new ReplicaFetcherMockBlockingSend(offsets, brokerEndPoint, new SystemTime()) - - val thread = new ReplicaFetcherThread("bob", 0, brokerEndPoint, config, failedPartitions, replicaManager, new Metrics(), new SystemTime(), quota, Some(mockNetwork)) - - val lastFetchedEpoch = Some(4) - thread.addPartitions(Map( - t1p0 -> initialFetchState(0, leaderEpoch, lastFetchedEpoch), - t1p1 -> initialFetchState(0L, leaderEpoch, lastFetchedEpoch), - t2p1 -> initialFetchState(0L, leaderEpoch, if (hasUnknownLastFetchedEpoch) None else lastFetchedEpoch))) - - def verifyPartitionStates(fetching: Set[TopicPartition], - truncating: Set[TopicPartition]): Unit = { - for (tp <- List(t1p0, t1p1, t2p1)) { - assertTrue(thread.fetchState(tp).isDefined) - val fetchState = thread.fetchState(tp).get - - val shouldBeReadyForFetch = fetching.contains(tp) - assertEquals( - s"Partition $tp should${if (!shouldBeReadyForFetch) " NOT" else ""} be ready for fetching", - shouldBeReadyForFetch, fetchState.isReadyForFetch) - - val shouldBeTruncatingLog = truncating.contains(tp) - assertEquals( - s"Partition $tp should${if (!shouldBeTruncatingLog) " NOT" else ""} be truncating its log", - shouldBeTruncatingLog, fetchState.isTruncating) - - assertFalse(s"Partition $tp should not be delayed", fetchState.isDelayed) - } - } - - val truncating = if (!ApiVersion.isTruncationOnFetchSupported(ibp)) - Set(t1p0, t1p1, t2p1) - else if (hasUnknownLastFetchedEpoch) - Set(t2p1) - else - Set.empty[TopicPartition] - verifyPartitionStates(Set(t1p0, t1p1, t2p1) -- truncating, truncating) - //Loop 1 - thread.doWork() - val expectedEpochFetchCount = if (hasUnknownLastFetchedEpoch || !ApiVersion.isTruncationOnFetchSupported(ibp)) 1 else 0 - assertEquals(expectedEpochFetchCount, mockNetwork.epochFetchCount) - assertEquals(1, mockNetwork.fetchCount) - - verifyPartitionStates(Set(t1p0, t1p1, t2p1), Set.empty) - - //Loop 2 we should not fetch epochs - thread.doWork() - assertEquals(expectedEpochFetchCount, mockNetwork.epochFetchCount) - assertEquals(2, mockNetwork.fetchCount) - verifyPartitionStates(Set(t1p0, t1p1, t2p1), Set.empty) - verify(logManager) - } - /** * Assert that all partitions' states are as expected * @@ -351,8 +221,19 @@ class ReplicaFetcherThreadTest { } @Test - def shouldFetchLeaderEpochOnFirstFetchOnlyIfLeaderEpochKnownToBoth(): Unit = { - val config = KafkaConfig.fromProps(TestUtils.createBrokerConfig(1, "localhost:1234")) + def shouldFetchLeaderEpochOnFirstFetchOnlyIfLeaderEpochKnownToBothIbp26(): Unit = { + verifyFetchLeaderEpochOnFirstFetch(KAFKA_2_6_IV0) + } + + @Test + def shouldNotFetchLeaderEpochOnFirstFetchWithTruncateOnFetch(): Unit = { + verifyFetchLeaderEpochOnFirstFetch(ApiVersion.latestVersion, epochFetchCount = 0) + } + + private def verifyFetchLeaderEpochOnFirstFetch(ibp: ApiVersion, epochFetchCount: Int = 1): Unit = { + val props = TestUtils.createBrokerConfig(1, "localhost:1234") + props.setProperty(KafkaConfig.InterBrokerProtocolVersionProp, ibp.version) + val config = KafkaConfig.fromProps(props) //Setup all dependencies val logManager: LogManager = createMock(classOf[LogManager]) @@ -390,17 +271,17 @@ class ReplicaFetcherThreadTest { //Loop 1 thread.doWork() - assertEquals(1, mockNetwork.epochFetchCount) + assertEquals(epochFetchCount, mockNetwork.epochFetchCount) assertEquals(1, mockNetwork.fetchCount) //Loop 2 we should not fetch epochs thread.doWork() - assertEquals(1, mockNetwork.epochFetchCount) + assertEquals(epochFetchCount, mockNetwork.epochFetchCount) assertEquals(2, mockNetwork.fetchCount) //Loop 3 we should not fetch epochs thread.doWork() - assertEquals(1, mockNetwork.epochFetchCount) + assertEquals(epochFetchCount, mockNetwork.epochFetchCount) assertEquals(3, mockNetwork.fetchCount) //Assert that truncate to is called exactly once (despite two loops) @@ -414,7 +295,7 @@ class ReplicaFetcherThreadTest { val truncateToCapture: Capture[Long] = newCapture(CaptureType.ALL) // Setup all the dependencies - val configs = TestUtils.createBrokerConfigs(1, "localhost:1234").map(KafkaConfig.fromProps) + val config = kafkaConfigNoTruncateOnFetch val quota: ReplicationQuotaManager = createNiceMock(classOf[ReplicationQuotaManager]) val logManager: LogManager = createMock(classOf[LogManager]) val replicaAlterLogDirsManager: ReplicaAlterLogDirsManager = createMock(classOf[ReplicaAlterLogDirsManager]) @@ -446,7 +327,7 @@ class ReplicaFetcherThreadTest { //Create the thread val mockNetwork = new ReplicaFetcherMockBlockingSend(offsetsReply, brokerEndPoint, new SystemTime()) - val thread = new ReplicaFetcherThread("bob", 0, brokerEndPoint, configs.head, failedPartitions, replicaManager, + val thread = new ReplicaFetcherThread("bob", 0, brokerEndPoint, config, failedPartitions, replicaManager, new Metrics(), new SystemTime(), quota, Some(mockNetwork)) thread.addPartitions(Map(t1p0 -> initialFetchState(0L), t2p1 -> initialFetchState(0L))) @@ -466,7 +347,7 @@ class ReplicaFetcherThreadTest { val truncateToCapture: Capture[Long] = newCapture(CaptureType.ALL) // Setup all the dependencies - val configs = TestUtils.createBrokerConfigs(1, "localhost:1234").map(KafkaConfig.fromProps) + val config = kafkaConfigNoTruncateOnFetch val quota: ReplicationQuotaManager = createNiceMock(classOf[ReplicationQuotaManager]) val logManager: LogManager = createMock(classOf[LogManager]) val replicaAlterLogDirsManager: ReplicaAlterLogDirsManager = createMock(classOf[ReplicaAlterLogDirsManager]) @@ -499,7 +380,7 @@ class ReplicaFetcherThreadTest { //Create the thread val mockNetwork = new ReplicaFetcherMockBlockingSend(offsetsReply, brokerEndPoint, new SystemTime()) - val thread = new ReplicaFetcherThread("bob", 0, brokerEndPoint, configs.head, failedPartitions, + val thread = new ReplicaFetcherThread("bob", 0, brokerEndPoint, config, failedPartitions, replicaManager, new Metrics(), new SystemTime(), quota, Some(mockNetwork)) thread.addPartitions(Map(t1p0 -> initialFetchState(0L), t2p1 -> initialFetchState(0L))) @@ -519,9 +400,7 @@ class ReplicaFetcherThreadTest { // Create a capture to track what partitions/offsets are truncated val truncateToCapture: Capture[Long] = newCapture(CaptureType.ALL) - val props = TestUtils.createBrokerConfig(1, "localhost:1234") - props.setProperty(KafkaConfig.InterBrokerProtocolVersionProp, KAFKA_2_6_IV0.version) - val config = KafkaConfig.fromProps(props) + val config = kafkaConfigNoTruncateOnFetch // Setup all dependencies val quota: ReplicationQuotaManager = createNiceMock(classOf[ReplicationQuotaManager]) @@ -628,7 +507,7 @@ class ReplicaFetcherThreadTest { val thread = new ReplicaFetcherThread("bob", 0, brokerEndPoint, config, failedPartitions, replicaManager, new Metrics(), new SystemTime(), quota, Some(mockNetwork)) { override def processPartitionData(topicPartition: TopicPartition, fetchOffset: Long, partitionData: FetchData): Option[LogAppendInfo] = None } - thread.addPartitions(Map(t1p0 -> initialFetchState(initialLEO, lastFetchedEpoch = Some(5)), t1p1 -> initialFetchState(initialLEO, lastFetchedEpoch = Some(5)))) + thread.addPartitions(Map(t1p0 -> initialFetchState(initialLEO), t1p1 -> initialFetchState(initialLEO))) val partitions = Set(t1p0, t1p1) // Loop 1 -- both topic partitions skip epoch fetch and send fetch request since @@ -762,7 +641,7 @@ class ReplicaFetcherThreadTest { val truncated: Capture[Long] = newCapture(CaptureType.ALL) // Setup all the dependencies - val configs = TestUtils.createBrokerConfigs(1, "localhost:1234").map(KafkaConfig.fromProps) + val config = kafkaConfigNoTruncateOnFetch val quota: ReplicationQuotaManager = createNiceMock(classOf[ReplicationQuotaManager]) val logManager: LogManager = createMock(classOf[LogManager]) val replicaAlterLogDirsManager: ReplicaAlterLogDirsManager = createMock(classOf[ReplicaAlterLogDirsManager]) @@ -788,7 +667,7 @@ class ReplicaFetcherThreadTest { //Create the thread val mockNetwork = new ReplicaFetcherMockBlockingSend(offsetsReply, brokerEndPoint, new SystemTime()) - val thread = new ReplicaFetcherThread("bob", 0, brokerEndPoint, configs.head, failedPartitions, replicaManager, new Metrics(), new SystemTime(), quota, Some(mockNetwork)) + val thread = new ReplicaFetcherThread("bob", 0, brokerEndPoint, config, failedPartitions, replicaManager, new Metrics(), new SystemTime(), quota, Some(mockNetwork)) thread.addPartitions(Map(t1p0 -> initialFetchState(initialFetchOffset))) //Run it @@ -805,7 +684,7 @@ class ReplicaFetcherThreadTest { val truncated: Capture[Long] = newCapture(CaptureType.ALL) // Setup all the dependencies - val configs = TestUtils.createBrokerConfigs(1, "localhost:1234").map(KafkaConfig.fromProps) + val config = kafkaConfigNoTruncateOnFetch val quota: ReplicationQuotaManager = createNiceMock(classOf[ReplicationQuotaManager]) val logManager: LogManager = createMock(classOf[LogManager]) val replicaAlterLogDirsManager: ReplicaAlterLogDirsManager = createMock(classOf[ReplicaAlterLogDirsManager]) @@ -841,7 +720,7 @@ class ReplicaFetcherThreadTest { //Create the thread val mockNetwork = new ReplicaFetcherMockBlockingSend(offsetsReply, brokerEndPoint, new SystemTime()) - val thread = new ReplicaFetcherThread("bob", 0, brokerEndPoint, configs.head, failedPartitions, replicaManager, new Metrics(), new SystemTime(), quota, Some(mockNetwork)) + val thread = new ReplicaFetcherThread("bob", 0, brokerEndPoint, config, failedPartitions, replicaManager, new Metrics(), new SystemTime(), quota, Some(mockNetwork)) thread.addPartitions(Map(t1p0 -> initialFetchState(0L), t1p1 -> initialFetchState(0L))) //Run thread 3 times @@ -863,7 +742,7 @@ class ReplicaFetcherThreadTest { @Test def shouldMovePartitionsOutOfTruncatingLogState(): Unit = { - val config = KafkaConfig.fromProps(TestUtils.createBrokerConfig(1, "localhost:1234")) + val config = kafkaConfigNoTruncateOnFetch //Setup all stubs val quota: ReplicationQuotaManager = createNiceMock(classOf[ReplicationQuotaManager]) @@ -914,7 +793,7 @@ class ReplicaFetcherThreadTest { @Test def shouldFilterPartitionsMadeLeaderDuringLeaderEpochRequest(): Unit ={ - val config = KafkaConfig.fromProps(TestUtils.createBrokerConfig(1, "localhost:1234")) + val config = kafkaConfigNoTruncateOnFetch val truncateToCapture: Capture[Long] = newCapture(CaptureType.ALL) val initialLEO = 100 @@ -1066,4 +945,10 @@ class ReplicaFetcherThreadTest { expect(replicaManager.localLogOrException(t2p1)).andReturn(log).anyTimes() expect(replicaManager.nonOfflinePartition(t2p1)).andReturn(Some(partition)).anyTimes() } + + private def kafkaConfigNoTruncateOnFetch: KafkaConfig = { + val props = TestUtils.createBrokerConfig(1, "localhost:1234") + props.setProperty(KafkaConfig.InterBrokerProtocolVersionProp, KAFKA_2_6_IV0.version) + KafkaConfig.fromProps(props) + } } diff --git a/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala b/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala index cd0ca0c7c22b4..b95cf0c24b465 100644 --- a/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala +++ b/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala @@ -1544,7 +1544,7 @@ class ReplicaManagerTest { // add it here (it's a no-op if already added) val initialOffset = InitialFetchState( leader = new BrokerEndPoint(0, "localhost", 9092), - initOffset = 0L, currentLeaderEpoch = leaderEpochInLeaderAndIsr, lastFetchedEpoch = None) + initOffset = 0L, currentLeaderEpoch = leaderEpochInLeaderAndIsr) addPartitions(Map(new TopicPartition(topic, topicPartition) -> initialOffset)) super.doWork() diff --git a/jmh-benchmarks/src/main/java/org/apache/kafka/jmh/fetcher/ReplicaFetcherThreadBenchmark.java b/jmh-benchmarks/src/main/java/org/apache/kafka/jmh/fetcher/ReplicaFetcherThreadBenchmark.java index e14b2dded09dc..02bf2f6699cac 100644 --- a/jmh-benchmarks/src/main/java/org/apache/kafka/jmh/fetcher/ReplicaFetcherThreadBenchmark.java +++ b/jmh-benchmarks/src/main/java/org/apache/kafka/jmh/fetcher/ReplicaFetcherThreadBenchmark.java @@ -163,7 +163,7 @@ public void setup() throws IOException { partition.makeFollower(partitionState, offsetCheckpoints); pool.put(tp, partition); - initialFetchStates.put(tp, new InitialFetchState(new BrokerEndPoint(3, "host", 3000), 0, 0, Option.empty())); + initialFetchStates.put(tp, new InitialFetchState(new BrokerEndPoint(3, "host", 3000), 0, 0)); BaseRecords fetched = new BaseRecords() { @Override public int sizeInBytes() { From c88e3a79b4b5db49f8b81bd236314b3554454cd9 Mon Sep 17 00:00:00 2001 From: Rajini Sivaram Date: Wed, 2 Dec 2020 23:33:42 +0000 Subject: [PATCH 10/11] Address review comments --- .../kafka/server/AbstractFetcherManager.scala | 18 +++++++--- .../kafka/server/AbstractFetcherThread.scala | 33 ++++++++----------- .../server/AbstractFetcherManagerTest.scala | 2 +- .../server/AbstractFetcherThreadTest.scala | 26 ++++++++++----- .../server/ReplicaFetcherThreadTest.scala | 4 +-- 5 files changed, 47 insertions(+), 36 deletions(-) diff --git a/core/src/main/scala/kafka/server/AbstractFetcherManager.scala b/core/src/main/scala/kafka/server/AbstractFetcherManager.scala index b48253127616a..25917c79ff270 100755 --- a/core/src/main/scala/kafka/server/AbstractFetcherManager.scala +++ b/core/src/main/scala/kafka/server/AbstractFetcherManager.scala @@ -26,6 +26,7 @@ import org.apache.kafka.common.utils.Utils import scala.collection.mutable import scala.collection.{Map, Set} +import scala.jdk.CollectionConverters._ abstract class AbstractFetcherManager[T <: AbstractFetcherThread](val name: String, clientId: String, numFetchers: Int) extends Logging with KafkaMetricsGroup { @@ -64,11 +65,16 @@ abstract class AbstractFetcherManager[T <: AbstractFetcherThread](val name: Stri def resizeThreadPool(newSize: Int): Unit = { def migratePartitions(newSize: Int): Unit = { fetcherThreadMap.forKeyValue { (id, thread) => - val removedPartitions = thread.removeAllPartitions() - removeFetcherForPartitions(removedPartitions.keySet) // clear state for removed partitions + val partitionStates = removeFetcherForPartitions(thread.partitions.asScala) if (id.fetcherId >= newSize) thread.shutdown() - addFetcherForPartitions(removedPartitions) + val fetchStates = partitionStates.map { case (topicPartition, currentFetchState) => + val initialFetchState = InitialFetchState(thread.sourceBroker, + currentLeaderEpoch = currentFetchState.currentLeaderEpoch, + initOffset = currentFetchState.fetchOffset) + topicPartition -> initialFetchState + } + addFetcherForPartitions(fetchStates) } } lock synchronized { @@ -153,14 +159,16 @@ abstract class AbstractFetcherManager[T <: AbstractFetcherThread](val name: Stri info(s"Added fetcher to broker ${fetcherThread.sourceBroker.id} for partitions $initialOffsetAndEpochs") } - def removeFetcherForPartitions(partitions: Set[TopicPartition]): Unit = { + def removeFetcherForPartitions(partitions: Set[TopicPartition]): Map[TopicPartition, PartitionFetchState] = { + val fetchStates = mutable.Map.empty[TopicPartition, PartitionFetchState] lock synchronized { for (fetcher <- fetcherThreadMap.values) - fetcher.removePartitions(partitions) + fetchStates ++= fetcher.removePartitions(partitions) failedPartitions.removeAll(partitions) } if (partitions.nonEmpty) info(s"Removed fetcher for partitions $partitions") + fetchStates } def shutdownIdleFetcherThreads(): Unit = { diff --git a/core/src/main/scala/kafka/server/AbstractFetcherThread.scala b/core/src/main/scala/kafka/server/AbstractFetcherThread.scala index cf20b7b42dc08..82a1862364db5 100755 --- a/core/src/main/scala/kafka/server/AbstractFetcherThread.scala +++ b/core/src/main/scala/kafka/server/AbstractFetcherThread.scala @@ -462,8 +462,10 @@ abstract class AbstractFetcherThread(name: String, } else if (initialFetchState.initOffset < 0) { fetchOffsetAndTruncate(tp, initialFetchState.currentLeaderEpoch) } else if (isTruncationOnFetchSupported) { + // With old message format, `latestEpoch` will be empty and we use Truncating state + // to truncate to high watermark. val lastFetchedEpoch = latestEpoch(tp) - val state = if (lastFetchedEpoch.exists(_ != EpochEndOffset.UNDEFINED_EPOCH)) Fetching else Truncating + val state = if (lastFetchedEpoch.nonEmpty) Fetching else Truncating PartitionFetchState(initialFetchState.initOffset, None, initialFetchState.currentLeaderEpoch, state, lastFetchedEpoch) } else { @@ -694,13 +696,15 @@ abstract class AbstractFetcherThread(name: String, } finally partitionMapLock.unlock() } - def removePartitions(topicPartitions: Set[TopicPartition]): Unit = { + def removePartitions(topicPartitions: Set[TopicPartition]): Map[TopicPartition, PartitionFetchState] = { partitionMapLock.lockInterruptibly() try { - topicPartitions.foreach { topicPartition => + topicPartitions.map { topicPartition => + val state = partitionStates.stateValue(topicPartition) partitionStates.remove(topicPartition) fetcherLagStats.unregister(topicPartition) - } + topicPartition -> state + }.filter(_._2 != null).toMap } finally partitionMapLock.unlock() } @@ -710,26 +714,17 @@ abstract class AbstractFetcherThread(name: String, finally partitionMapLock.unlock() } + def partitions: util.Set[TopicPartition] = { + partitionMapLock.lockInterruptibly() + try new util.HashSet(partitionStates.partitionSet) + finally partitionMapLock.unlock() + } + // Visible for testing private[server] def fetchState(topicPartition: TopicPartition): Option[PartitionFetchState] = inLock(partitionMapLock) { Option(partitionStates.stateValue(topicPartition)) } - /** - * Removes all partitions assigned to this thread and returns current fetch state - * for each partition. This is used to reassign partitions when thread pool is resized. - */ - private[server] def removeAllPartitions(): Map[TopicPartition, InitialFetchState] = inLock(partitionMapLock) { - val fetchStates = partitionStates.partitionStateMap.asScala.map { case (topicPartition, currentFetchState) => - val initialFetchState = InitialFetchState(sourceBroker, - currentLeaderEpoch = currentFetchState.currentLeaderEpoch, - initOffset = currentFetchState.fetchOffset) - topicPartition -> initialFetchState - } - partitionStates.clear() - fetchStates - } - protected def toMemoryRecords(records: Records): MemoryRecords = { (records: @unchecked) match { case r: MemoryRecords => r diff --git a/core/src/test/scala/unit/kafka/server/AbstractFetcherManagerTest.scala b/core/src/test/scala/unit/kafka/server/AbstractFetcherManagerTest.scala index 497aa1ab334f5..847449b02e3f2 100644 --- a/core/src/test/scala/unit/kafka/server/AbstractFetcherManagerTest.scala +++ b/core/src/test/scala/unit/kafka/server/AbstractFetcherManagerTest.scala @@ -61,7 +61,7 @@ class AbstractFetcherManagerTest { .andReturn(Set(tp)) EasyMock.expect(fetcher.fetchState(tp)) .andReturn(Some(PartitionFetchState(fetchOffset, None, leaderEpoch, Truncating, lastFetchedEpoch = None))) - EasyMock.expect(fetcher.removePartitions(Set(tp))) + EasyMock.expect(fetcher.removePartitions(Set(tp))).andReturn(Map.empty) EasyMock.expect(fetcher.fetchState(tp)).andReturn(None) EasyMock.replay(fetcher) diff --git a/core/src/test/scala/unit/kafka/server/AbstractFetcherThreadTest.scala b/core/src/test/scala/unit/kafka/server/AbstractFetcherThreadTest.scala index 2a8b6ba9d3e50..d9afba9870bae 100644 --- a/core/src/test/scala/unit/kafka/server/AbstractFetcherThreadTest.scala +++ b/core/src/test/scala/unit/kafka/server/AbstractFetcherThreadTest.scala @@ -203,7 +203,7 @@ class AbstractFetcherThreadTest { // The replica's leader epoch is ahead of the leader val replicaState = MockFetcherThread.PartitionState(leaderEpoch = 1) fetcher.setReplicaState(partition, replicaState) - fetcher.addPartitions(Map(partition -> initialFetchState(0L, leaderEpoch = 1))) + fetcher.addPartitions(Map(partition -> initialFetchState(0L, leaderEpoch = 1)), forceTruncation = true) val batch = mkBatch(baseOffset = 0L, leaderEpoch = 0, new SimpleRecord("a".getBytes)) val leaderState = MockFetcherThread.PartitionState(Seq(batch), leaderEpoch = 0, highWatermark = 2L) @@ -407,7 +407,7 @@ class AbstractFetcherThreadTest { val replicaState = MockFetcherThread.PartitionState(leaderEpoch = 5) fetcher.setReplicaState(partition, replicaState) - fetcher.addPartitions(Map(partition -> initialFetchState(0L, leaderEpoch = 5))) + fetcher.addPartitions(Map(partition -> initialFetchState(0L, leaderEpoch = 5)), forceTruncation = true) val leaderLog = Seq( mkBatch(baseOffset = 0, leaderEpoch = 1, new SimpleRecord("a".getBytes)), @@ -650,7 +650,7 @@ class AbstractFetcherThreadTest { // leader epoch changes while fetching epochs from leader removePartitions(Set(partition)) setReplicaState(partition, MockFetcherThread.PartitionState(leaderEpoch = nextLeaderEpochOnFollower)) - addPartitions(Map(partition -> initialFetchState(0L, leaderEpoch = nextLeaderEpochOnFollower))) + addPartitions(Map(partition -> initialFetchState(0L, leaderEpoch = nextLeaderEpochOnFollower)), forceTruncation = true) fetchEpochsFromLeaderOnce = true } fetchedEpochs @@ -658,7 +658,7 @@ class AbstractFetcherThreadTest { } fetcher.setReplicaState(partition, MockFetcherThread.PartitionState(leaderEpoch = initialLeaderEpochOnFollower)) - fetcher.addPartitions(Map(partition -> initialFetchState(0L, leaderEpoch = initialLeaderEpochOnFollower))) + fetcher.addPartitions(Map(partition -> initialFetchState(0L, leaderEpoch = initialLeaderEpochOnFollower)), forceTruncation = true) val leaderLog = Seq( mkBatch(baseOffset = 0, leaderEpoch = initialLeaderEpochOnFollower, new SimpleRecord("c".getBytes))) @@ -736,7 +736,7 @@ class AbstractFetcherThreadTest { } fetcher.setReplicaState(partition, MockFetcherThread.PartitionState(leaderEpoch = 0)) - fetcher.addPartitions(Map(partition -> initialFetchState(0L, leaderEpoch = 0))) + fetcher.addPartitions(Map(partition -> initialFetchState(0L, leaderEpoch = 0)), forceTruncation = true) fetcher.setLeaderState(partition, MockFetcherThread.PartitionState(leaderEpoch = 0)) // first round of truncation should throw an exception @@ -776,11 +776,11 @@ class AbstractFetcherThreadTest { private def verifyFetcherThreadHandlingPartitionFailure(fetcher: MockFetcherThread): Unit = { fetcher.setReplicaState(partition1, MockFetcherThread.PartitionState(leaderEpoch = 0)) - fetcher.addPartitions(Map(partition1 -> initialFetchState(0L, leaderEpoch = 0))) + fetcher.addPartitions(Map(partition1 -> initialFetchState(0L, leaderEpoch = 0)), forceTruncation = true) fetcher.setLeaderState(partition1, MockFetcherThread.PartitionState(leaderEpoch = 0)) fetcher.setReplicaState(partition2, MockFetcherThread.PartitionState(leaderEpoch = 0)) - fetcher.addPartitions(Map(partition2 -> initialFetchState(0L, leaderEpoch = 0))) + fetcher.addPartitions(Map(partition2 -> initialFetchState(0L, leaderEpoch = 0)), forceTruncation = true) fetcher.setLeaderState(partition2, MockFetcherThread.PartitionState(leaderEpoch = 0)) // processing data fails for partition1 @@ -798,7 +798,7 @@ class AbstractFetcherThreadTest { // simulate a leader change fetcher.removePartitions(Set(partition1)) failedPartitions.removeAll(Set(partition1)) - fetcher.addPartitions(Map(partition1 -> initialFetchState(0L, leaderEpoch = 1))) + fetcher.addPartitions(Map(partition1 -> initialFetchState(0L, leaderEpoch = 1)), forceTruncation = true) // partition1 added back assertEquals(Some(Truncating), fetcher.fetchState(partition1).map(_.state)) @@ -871,6 +871,7 @@ class AbstractFetcherThreadTest { private val replicaPartitionStates = mutable.Map[TopicPartition, PartitionState]() private val leaderPartitionStates = mutable.Map[TopicPartition, PartitionState]() + private var latestEpochDefault: Option[Int] = Some(0) def setLeaderState(topicPartition: TopicPartition, state: PartitionState): Unit = { leaderPartitionStates.put(topicPartition, state) @@ -890,6 +891,13 @@ class AbstractFetcherThreadTest { throw new IllegalArgumentException(s"Unknown partition $topicPartition")) } + def addPartitions(initialFetchStates: Map[TopicPartition, InitialFetchState], forceTruncation: Boolean): Set[TopicPartition] = { + latestEpochDefault = if (forceTruncation) None else Some(0) + val partitions = super.addPartitions(initialFetchStates) + latestEpochDefault = Some (0) + partitions + } + override def processPartitionData(topicPartition: TopicPartition, fetchOffset: Long, partitionData: FetchData): Option[LogAppendInfo] = { @@ -980,7 +988,7 @@ class AbstractFetcherThreadTest { override def latestEpoch(topicPartition: TopicPartition): Option[Int] = { val state = replicaPartitionState(topicPartition) - state.log.lastOption.map(_.partitionLeaderEpoch).orElse(Some(EpochEndOffset.UNDEFINED_EPOCH)) + state.log.lastOption.map(_.partitionLeaderEpoch).orElse(latestEpochDefault) } override def logStartOffset(topicPartition: TopicPartition): Long = replicaPartitionState(topicPartition).logStartOffset diff --git a/core/src/test/scala/unit/kafka/server/ReplicaFetcherThreadTest.scala b/core/src/test/scala/unit/kafka/server/ReplicaFetcherThreadTest.scala index 2e4d85cf58d8e..ae2dbda3d3134 100644 --- a/core/src/test/scala/unit/kafka/server/ReplicaFetcherThreadTest.scala +++ b/core/src/test/scala/unit/kafka/server/ReplicaFetcherThreadTest.scala @@ -510,8 +510,8 @@ class ReplicaFetcherThreadTest { thread.addPartitions(Map(t1p0 -> initialFetchState(initialLEO), t1p1 -> initialFetchState(initialLEO))) val partitions = Set(t1p0, t1p1) - // Loop 1 -- both topic partitions skip epoch fetch and send fetch request since - // lastFetchedEpoch is set in initial fetch state. + // Loop 1 -- both topic partitions skip epoch fetch and send fetch request since we can truncate + // later based on diverging epochs in fetch response. thread.doWork() assertEquals(0, mockNetwork.epochFetchCount) assertEquals(1, mockNetwork.fetchCount) From be8b884ae2034c4ce6aef17d29a9a3f3429abe82 Mon Sep 17 00:00:00 2001 From: Jason Gustafson Date: Wed, 2 Dec 2020 16:43:25 -0800 Subject: [PATCH 11/11] Trivial style tweaks --- .../scala/kafka/server/AbstractFetcherManager.scala | 10 ++++------ .../scala/kafka/server/AbstractFetcherThread.scala | 4 ++-- .../unit/kafka/server/AbstractFetcherThreadTest.scala | 2 +- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/core/src/main/scala/kafka/server/AbstractFetcherManager.scala b/core/src/main/scala/kafka/server/AbstractFetcherManager.scala index 25917c79ff270..3d9189e1071dd 100755 --- a/core/src/main/scala/kafka/server/AbstractFetcherManager.scala +++ b/core/src/main/scala/kafka/server/AbstractFetcherManager.scala @@ -17,16 +17,14 @@ package kafka.server -import kafka.utils.Implicits._ -import kafka.utils.Logging import kafka.cluster.BrokerEndPoint import kafka.metrics.KafkaMetricsGroup +import kafka.utils.Implicits._ +import kafka.utils.Logging import org.apache.kafka.common.TopicPartition import org.apache.kafka.common.utils.Utils -import scala.collection.mutable -import scala.collection.{Map, Set} -import scala.jdk.CollectionConverters._ +import scala.collection.{Map, Set, mutable} abstract class AbstractFetcherManager[T <: AbstractFetcherThread](val name: String, clientId: String, numFetchers: Int) extends Logging with KafkaMetricsGroup { @@ -65,7 +63,7 @@ abstract class AbstractFetcherManager[T <: AbstractFetcherThread](val name: Stri def resizeThreadPool(newSize: Int): Unit = { def migratePartitions(newSize: Int): Unit = { fetcherThreadMap.forKeyValue { (id, thread) => - val partitionStates = removeFetcherForPartitions(thread.partitions.asScala) + val partitionStates = removeFetcherForPartitions(thread.partitions) if (id.fetcherId >= newSize) thread.shutdown() val fetchStates = partitionStates.map { case (topicPartition, currentFetchState) => diff --git a/core/src/main/scala/kafka/server/AbstractFetcherThread.scala b/core/src/main/scala/kafka/server/AbstractFetcherThread.scala index 82a1862364db5..2ef618a007674 100755 --- a/core/src/main/scala/kafka/server/AbstractFetcherThread.scala +++ b/core/src/main/scala/kafka/server/AbstractFetcherThread.scala @@ -714,9 +714,9 @@ abstract class AbstractFetcherThread(name: String, finally partitionMapLock.unlock() } - def partitions: util.Set[TopicPartition] = { + def partitions: Set[TopicPartition] = { partitionMapLock.lockInterruptibly() - try new util.HashSet(partitionStates.partitionSet) + try partitionStates.partitionSet.asScala.toSet finally partitionMapLock.unlock() } diff --git a/core/src/test/scala/unit/kafka/server/AbstractFetcherThreadTest.scala b/core/src/test/scala/unit/kafka/server/AbstractFetcherThreadTest.scala index d9afba9870bae..6a430b44bf012 100644 --- a/core/src/test/scala/unit/kafka/server/AbstractFetcherThreadTest.scala +++ b/core/src/test/scala/unit/kafka/server/AbstractFetcherThreadTest.scala @@ -894,7 +894,7 @@ class AbstractFetcherThreadTest { def addPartitions(initialFetchStates: Map[TopicPartition, InitialFetchState], forceTruncation: Boolean): Set[TopicPartition] = { latestEpochDefault = if (forceTruncation) None else Some(0) val partitions = super.addPartitions(initialFetchStates) - latestEpochDefault = Some (0) + latestEpochDefault = Some(0) partitions }