Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion core/src/main/scala/kafka/server/DelayedFetch.scala
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,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 +
"]"
}
}
Expand Down Expand Up @@ -77,6 +78,7 @@ class DelayedFetch(delayMs: Long,
* Case E: This broker is the leader, but the requested epoch is now fenced
* Case F: The fetch offset locates not on the last segment of the log
* Case G: The accumulated bytes from all the fetching partitions exceeds the minimum bytes
* Case H: A diverging epoch was found, return response to trigger truncation
* Upon completion, should return whatever data is available for each valid partition
*/
override def tryComplete(): Boolean = {
Expand All @@ -88,6 +90,13 @@ class DelayedFetch(delayMs: Long,
try {
if (fetchOffset != LogOffsetMetadata.UnknownOffsetMetadata) {
val partition = replicaManager.getPartitionOrException(topicPartition)

// Case H: Return diverging epoch in response to trigger truncation
if (fetchStatus.hasDivergingEpoch) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we are using the status from the original fetch. I am wondering if we need to recheck below since it is possible to get a truncation while a fetch is in purgatory.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hachikuji Thanks for the review. Makes sense, I have added a new check at the end instead of this one, not sure if there is a better way to check.

debug(s"Satisfying fetch $fetchMetadata since it has diverging epoch requiring truncation for partition $topicPartition.")
return forceComplete()
}

val offsetSnapshot = partition.fetchOffsetSnapshot(fetchLeaderEpoch, fetchMetadata.fetchOnlyLeader)

val endOffset = fetchMetadata.fetchIsolation match {
Expand Down
10 changes: 6 additions & 4 deletions core/src/main/scala/kafka/server/FetchSession.scala
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ class CachedPartition(val topic: String,
var highWatermark: Long,
var leaderEpoch: Optional[Integer],
var fetcherLogStartOffset: Long,
var localLogStartOffset: Long)
var localLogStartOffset: Long,
var lastFetchedEpoch: Optional[Integer] = Optional.empty[Integer])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to provide a default here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

extends ImplicitLinkedHashCollection.Element {

var cachedNext: Int = ImplicitLinkedHashCollection.INVALID_INDEX
Expand All @@ -96,21 +97,22 @@ class CachedPartition(val topic: String,

def this(part: TopicPartition, reqData: FetchRequest.PartitionData) =
this(part.topic, part.partition, reqData.maxBytes, reqData.fetchOffset, -1,
reqData.currentLeaderEpoch, reqData.logStartOffset, -1)
reqData.currentLeaderEpoch, reqData.logStartOffset, -1, reqData.lastFetchedEpoch)

def this(part: TopicPartition, reqData: FetchRequest.PartitionData,
respData: FetchResponse.PartitionData[Records]) =
this(part.topic, part.partition, reqData.maxBytes, reqData.fetchOffset, respData.highWatermark,
reqData.currentLeaderEpoch, reqData.logStartOffset, respData.logStartOffset)
reqData.currentLeaderEpoch, reqData.logStartOffset, respData.logStartOffset, reqData.lastFetchedEpoch)

def reqData = new FetchRequest.PartitionData(fetchOffset, fetcherLogStartOffset, maxBytes, leaderEpoch)
def reqData = new FetchRequest.PartitionData(fetchOffset, fetcherLogStartOffset, maxBytes, leaderEpoch, lastFetchedEpoch)

def updateRequestParams(reqData: FetchRequest.PartitionData): Unit = {
// Update our cached request parameters.
maxBytes = reqData.maxBytes
fetchOffset = reqData.fetchOffset
fetcherLogStartOffset = reqData.logStartOffset
leaderEpoch = reqData.currentLeaderEpoch
lastFetchedEpoch = reqData.lastFetchedEpoch
}

/**
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/scala/kafka/server/ReplicaManager.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1084,7 +1084,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))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm.. If the LogReadResult has a diverging epoch, wouldn't we want to respond immediately?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, yes, so we don't need to check the original result in DelayedFetch, we return immediately here. Updated.

})
}
val fetchMetadata: SFetchMetadata = SFetchMetadata(fetchMinBytes, fetchMaxBytes, hardMaxBytesLimit,
Expand Down
63 changes: 52 additions & 11 deletions core/src/test/scala/integration/kafka/server/DelayedFetchTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,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
Expand All @@ -70,7 +72,7 @@ class DelayedFetchTest extends EasyMockSupport {
.andThrow(new FencedLeaderEpochException("Requested epoch has been fenced"))
EasyMock.expect(replicaManager.isAddingReplica(EasyMock.anyObject(), EasyMock.anyInt())).andReturn(false)

expectReadFromReplicaWithError(replicaId, topicPartition, fetchStatus.fetchInfo, Errors.FENCED_LEADER_EPOCH)
expectReadFromReplica(replicaId, topicPartition, fetchStatus.fetchInfo, Errors.FENCED_LEADER_EPOCH)

replayAll()

Expand All @@ -92,7 +94,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
Expand All @@ -110,7 +113,7 @@ class DelayedFetchTest extends EasyMockSupport {

EasyMock.expect(replicaManager.getPartitionOrException(topicPartition))
.andThrow(new NotLeaderOrFollowerException(s"Replica for $topicPartition not available"))
expectReadFromReplicaWithError(replicaId, topicPartition, fetchStatus.fetchInfo, Errors.NOT_LEADER_OR_FOLLOWER)
expectReadFromReplica(replicaId, topicPartition, fetchStatus.fetchInfo, Errors.NOT_LEADER_OR_FOLLOWER)
EasyMock.expect(replicaManager.isAddingReplica(EasyMock.anyObject(), EasyMock.anyInt())).andReturn(false)

replayAll()
Expand All @@ -120,6 +123,44 @@ class DelayedFetchTest extends EasyMockSupport {
assertTrue(fetchResultOpt.isDefined)
}

@Test
def testDivergingEpoch(): Unit = {
val topicPartition = new TopicPartition("topic", 0)
val fetchOffset = 500L
val logStartOffset = 0L
val currentLeaderEpoch = Optional.of[Integer](10)
val replicaId = 1

val fetchStatus = FetchPartitionStatus(
startOffsetMetadata = LogOffsetMetadata(fetchOffset),
fetchInfo = new FetchRequest.PartitionData(fetchOffset, logStartOffset, maxBytes, currentLeaderEpoch),
hasDivergingEpoch = true)
val fetchMetadata = buildFetchMetadata(replicaId, topicPartition, fetchStatus)

var fetchResultOpt: Option[FetchPartitionData] = None
def callback(responses: Seq[(TopicPartition, FetchPartitionData)]): Unit = {
fetchResultOpt = Some(responses.head._2)
}

val delayedFetch = new DelayedFetch(
delayMs = 500,
fetchMetadata = fetchMetadata,
replicaManager = replicaManager,
quota = replicaQuota,
clientMetadata = None,
responseCallback = callback)

val partition: Partition = mock(classOf[Partition])
EasyMock.expect(replicaManager.getPartitionOrException(topicPartition)).andReturn(partition)
EasyMock.expect(replicaManager.isAddingReplica(EasyMock.anyObject(), EasyMock.anyInt())).andReturn(false)
expectReadFromReplica(replicaId, topicPartition, fetchStatus.fetchInfo, Errors.NONE)
replayAll()

assertTrue(delayedFetch.tryComplete())
assertTrue(delayedFetch.isCompleted)
assertTrue(fetchResultOpt.isDefined)
}

private def buildFetchMetadata(replicaId: Int,
topicPartition: TopicPartition,
fetchStatus: FetchPartitionStatus): FetchMetadata = {
Expand All @@ -133,10 +174,10 @@ class DelayedFetchTest extends EasyMockSupport {
fetchPartitionStatus = Seq((topicPartition, fetchStatus)))
}

private def expectReadFromReplicaWithError(replicaId: Int,
topicPartition: TopicPartition,
fetchPartitionData: FetchRequest.PartitionData,
error: Errors): Unit = {
private def expectReadFromReplica(replicaId: Int,
topicPartition: TopicPartition,
fetchPartitionData: FetchRequest.PartitionData,
error: Errors): Unit = {
EasyMock.expect(replicaManager.readFromLocalLog(
replicaId = replicaId,
fetchOnlyFromLeader = true,
Expand All @@ -146,12 +187,12 @@ class DelayedFetchTest extends EasyMockSupport {
readPartitionInfo = Seq((topicPartition, fetchPartitionData)),
clientMetadata = None,
quota = replicaQuota))
.andReturn(Seq((topicPartition, buildReadResultWithError(error))))
.andReturn(Seq((topicPartition, buildReadResult(error))))
}

private def buildReadResultWithError(error: Errors): LogReadResult = {
private def buildReadResult(error: Errors): LogReadResult = {
LogReadResult(
exception = Some(error.exception),
exception = if (error != Errors.NONE) Some(error.exception) else None,
info = FetchDataInfo(LogOffsetMetadata.UnknownOffsetMetadata, MemoryRecords.EMPTY),
divergingEpoch = None,
highWatermark = -1L,
Expand Down
61 changes: 61 additions & 0 deletions core/src/test/scala/unit/kafka/server/FetchSessionTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,67 @@ class FetchSessionTest {
assertEquals(Optional.of(3), epochs3(tp2))
}

@Test
def testLastFetchedEpoch(): Unit = {
val time = new MockTime()
val cache = new FetchSessionCache(10, 1000)
val fetchManager = new FetchManager(time, cache)

val tp0 = new TopicPartition("foo", 0)
val tp1 = new TopicPartition("foo", 1)
val tp2 = new TopicPartition("bar", 1)

def cachedLeaderEpochs(context: FetchContext): Map[TopicPartition, Optional[Integer]] = {
val mapBuilder = Map.newBuilder[TopicPartition, Optional[Integer]]
context.foreachPartition((tp, data) => mapBuilder += tp -> data.currentLeaderEpoch)
mapBuilder.result()
}

def cachedLastFetchedEpochs(context: FetchContext): Map[TopicPartition, Optional[Integer]] = {
val mapBuilder = Map.newBuilder[TopicPartition, Optional[Integer]]
context.foreachPartition((tp, data) => mapBuilder += tp -> data.lastFetchedEpoch)
mapBuilder.result()
}

val request1 = new util.LinkedHashMap[TopicPartition, FetchRequest.PartitionData]
request1.put(tp0, new FetchRequest.PartitionData(0, 0, 100, Optional.empty, Optional.empty))
request1.put(tp1, new FetchRequest.PartitionData(10, 0, 100, Optional.of(1), Optional.empty))
request1.put(tp2, new FetchRequest.PartitionData(10, 0, 100, Optional.of(2), Optional.of(1)))

val context1 = fetchManager.newContext(JFetchMetadata.INITIAL, request1, EMPTY_PART_LIST, false)
assertEquals(Map(tp0 -> Optional.empty, tp1 -> Optional.of(1), tp2 -> Optional.of(2)),
cachedLeaderEpochs(context1))
assertEquals(Map(tp0 -> Optional.empty, tp1 -> Optional.empty, tp2 -> Optional.of(1)),
cachedLastFetchedEpochs(context1))

val response = new util.LinkedHashMap[TopicPartition, FetchResponse.PartitionData[Records]]
response.put(tp0, new FetchResponse.PartitionData(Errors.NONE, 100, 100, 100, null, null))
response.put(tp1, new FetchResponse.PartitionData(Errors.NONE, 10, 10, 10, null, null))
response.put(tp2, new FetchResponse.PartitionData(Errors.NONE, 5, 5, 5, null, null))

val sessionId = context1.updateAndGenerateResponseData(response).sessionId()

// With no changes, the cached epochs should remain the same
val request2 = new util.LinkedHashMap[TopicPartition, FetchRequest.PartitionData]
val context2 = fetchManager.newContext(new JFetchMetadata(sessionId, 1), request2, EMPTY_PART_LIST, false)
assertEquals(Map(tp0 -> Optional.empty, tp1 -> Optional.of(1), tp2 -> Optional.of(2)), cachedLeaderEpochs(context2))
assertEquals(Map(tp0 -> Optional.empty, tp1 -> Optional.empty, tp2 -> Optional.of(1)),
cachedLastFetchedEpochs(context2))
context2.updateAndGenerateResponseData(response).sessionId()

// Now verify we can change the leader epoch and the context is updated
val request3 = new util.LinkedHashMap[TopicPartition, FetchRequest.PartitionData]
request3.put(tp0, new FetchRequest.PartitionData(0, 0, 100, Optional.of(6), Optional.of(5)))
request3.put(tp1, new FetchRequest.PartitionData(10, 0, 100, Optional.empty, Optional.empty))
request3.put(tp2, new FetchRequest.PartitionData(10, 0, 100, Optional.of(3), Optional.of(3)))

val context3 = fetchManager.newContext(new JFetchMetadata(sessionId, 2), request3, EMPTY_PART_LIST, false)
assertEquals(Map(tp0 -> Optional.of(6), tp1 -> Optional.empty, tp2 -> Optional.of(3)),
cachedLeaderEpochs(context3))
assertEquals(Map(tp0 -> Optional.of(5), tp1 -> Optional.empty, tp2 -> Optional.of(3)),
cachedLastFetchedEpochs(context2))
}

@Test
def testFetchRequests(): Unit = {
val time = new MockTime()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,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,
Expand Down