KAFKA-18494-3: solution for the bug relating to gaps in the share partition cachedStates post initialization #18696
Conversation
…are partition cachedState after initialization
github-actions
bot
added
triage
| @@ -5752,13 +5752,13 @@ public void testMaybeInitializeWhenReadStateRpcReturnsZeroAvailableRecords() { | |||
| for (int i = 0; i < 500; i++) { | |||
| stateBatches.add(new PersisterStateBatch(234L + i, 234L + i, RecordState.ACKNOWLEDGED.id, (short) 1)); | |||
| } | |||
| stateBatches.add(new PersisterStateBatch(232L, 232L, RecordState.ARCHIVED.id, (short) 1)); | |||
| stateBatches.add(new PersisterStateBatch(233L, 233L, RecordState.ARCHIVED.id, (short) 1)); | |||
There was a problem hiding this comment.
Don't we require more tests as previously we didn't find the problem in share partition?
There was a problem hiding this comment.
Thanks for the review. Added unit test cases in the recent commit
| @@ -462,6 +468,10 @@ public CompletableFuture<Void> maybeInitialize() { | |||
| // in the cached state are not missed | |||
| findNextFetchOffset.set(true); | |||
| endOffset = cachedState.lastEntry().getValue().lastOffset(); | |||
| // initialReadGapOffset is not required, if there are no gaps in the read state response | |||
| if (isGapPresentInCachedState()) { | |||
There was a problem hiding this comment.
Rather iterating in the metod again, though it's a low cost thing, should we have a variable in earlier for loop in the method to check if gap exists, wdyt?
There was a problem hiding this comment.
Thanks for the review. Made this change in the recent commit
| if (lastAcquiredOffset > endOffset) { | ||
| endOffset = lastAcquiredOffset; | ||
| } |
There was a problem hiding this comment.
Can we write comment here reqarding when this can happen given we are acquirung new batch.
There was a problem hiding this comment.
Thanks for the review. I have added the comment explaining this scenario.
This PR fixes the bug introduced by gaps in the read state response during share partition initialization. A window is maintained tracking the gaps in the cachedStates. This window is updated post acquisition of the records present in the gaps.