Jackie-Jiang commented on code in PR #19267:
URL: https://github.com/apache/pinot/pull/19267#discussion_r4448996579
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeSegmentDataManager.java:
##########
@@ -567,10 +567,10 @@ protected boolean consumeLoop()
_currentOffset);
}
// We check this flag again further down
- } else if (messageBatch.getUnfilteredMessageCount() > 0) {
+ } else if (messageBatch.getUnfilteredMessageCount() > 0
+ || messageBatch.getOffsetOfNextBatch().compareTo(_currentOffset) >
0) {
Review Comment:
[P1] Preserve the final offset when catching up across control records
This allows a replica to advance past control record 10 and commit at offset
11. Another replica at offset 10, told to catch up to 11, can receive a newly
committed visible record at offset 11. `processStreamEvents()` checks only the
previous current offset, so it indexes that record and advances to 12.
`endCriteriaReached()` then throws `Past max offset`, putting the consumer into
`ERROR`; online-transition catch-up instead falls back to downloading. Before
this change, the first replica's boundary remained 10 in this scenario.
Please make both empty-batch advancement and visible-record processing
respect the exclusive `_finalOffset`. Add regression coverage for `CATCHING_UP`
and `CONSUMING_TO_ONLINE` with current offset 10, final offset 11, and next
visible record 11, plus an empty batch whose next offset exceeds the final
offset.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
