chirag-wadhwa5 commented on code in PR #18696:
URL: https://github.com/apache/kafka/pull/18696#discussion_r1935585867
##########
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:
Hi, I just checked the code again and I think that check is required. Let's
take the following case ->
startOffset = 15
stateBatches = {(15, 20), (30, 40)} all of which are acked.
If we don't have this check at all, findLastOffsetAcknowledged will return
40, because there is no way to stop the loop once
initialReadGapOffset.gapStartOffset is exceeded. That would be wrong, because
we cannot move the startOffset post initialReadGapOffset.gapStartOffset
--
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
