[GitHub] [kafka] jolshan edited a comment on pull request #9590: KAFKA-7556: KafkaConsumer.beginningOffsets does not return actual first offsets

[GitBox <gi...@apache.org>]

jolshan edited a comment on pull request #9590:
URL: https://github.com/apache/kafka/pull/9590#issuecomment-793061818


   @hachikuji updated based on the conversations here + those we had offline. Quick summary is that we will only have one swap file upon crash in the log cleaning case, so we will just accept that there may be segment(s) before or after the cleaned segment that won't get replaced upon crash recovery, but will upon subsequent cleanings.
   
   Two things about the latest commit:
   1) I decided that in the failure case the logStartOffset will be updated outside of replace segments. In the recovery path there is already code to update the logStartOffset. It must follow changes to update the log end offset so it made sense to handle that all there. Added a test for this scenario. 
   
   2) Let me know if any of the tests seem unnecessary. I think we should definitely keep `testRecoveryAfterCrashBaseOffsetUpdatedFirstSegmentChunk` and `testLogStartOffsetUpdatedUponFailureRecovery`. Let me know if any others seem superfluous.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org