npawar commented on pull request #8309: URL: https://github.com/apache/pinot/pull/8309#issuecomment-1064565659
> Thinking more about the other scenario we discussed offline (OOR due to broker trim, such that startOffset > latest). This patch will result in 1. going to earliest 2. possibly filtering everything out (assuming none of the rows of the batch are > latest) 3. keep incrementing an offset (data loss). On closer inspection, I realized that this will happen even in current state of code. Current code will result in 1. going to latest (instead of earliest) 2. could still filter everything out (though latest will be closer than earliest so lesser chances of that happening) 3. keep incrementing offset. The only way to recover from this case, is to wait till expected startOffset shows up again in stream. > > This is a rare occurrence, and I feel it is okay to have the slight inefficiency because of setting OffsetResetStrategy to earliest. > > The other option for us is: #8321. Explicitly checking for out of range where `startOffset < beginning ` But this one introduces an additional call to consumer API in every fetchMessages call. any thoughts on which way we should address this @xiangfu0 @mayankshriv ^^ -- 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: commits-unsubscr...@pinot.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
