chirag-wadhwa5 commented on code in PR #18696: URL: https://github.com/apache/kafka/pull/18696#discussion_r1935492051
########## core/src/main/java/kafka/server/share/SharePartition.java: ########## @@ -2160,6 +2234,43 @@ private long startOffsetDuringInitialization(long partitionDataStartOffset) thro } } + // Visible for testing + long findLastOffsetAcknowledged() { + lock.readLock().lock(); + long lastOffsetAcknowledged = -1; + try { + for (NavigableMap.Entry<Long, InFlightBatch> entry : cachedState.entrySet()) { + InFlightBatch inFlightBatch = entry.getValue(); + if (inFlightBatch.offsetState() == null) { + if (!isRecordStateAcknowledged(inFlightBatch.batchState())) { + return lastOffsetAcknowledged; + } + // If initialReadGapOffset.gapStartOffset is less than or equal to the last offset of the batch + // then we cannot identify the current inFlightBatch as acknowledged. All the offsets between + // initialReadGapOffset.gapStartOffset and initialReadGapOffset.endOffset should always be present + // in the cachedState + if (initialReadGapOffset != null && inFlightBatch.lastOffset() >= initialReadGapOffset.gapStartOffset()) { + return lastOffsetAcknowledged; + } Review Comment: Sure. For example, if we have a scenario like following -> startOffset = 11 inFlightStateBatches = {(11, 20) (21, 30) (41, 50)}, and all are acked initialReadGapOffset = {gapStartOffset = 25} In this case, when we reach the batch 21-30, we see it is acknowledged, but we cannot simply move the start offset to 31, because initialReadGapOffset.gapStartOffset is 25. I'm not sure if this istuation could ever occur or not, but added this check as a precautionary measure -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org