junrao commented on code in PR #15476:
URL: https://github.com/apache/kafka/pull/15476#discussion_r1523677272
##########
storage/src/main/java/org/apache/kafka/storage/internals/log/LogValidator.java:
##########
@@ -293,11 +293,11 @@ public ValidationResult
assignOffsetsNonCompressed(LongRef offsetCounter,
if (timestampType == TimestampType.LOG_APPEND_TIME) {
maxTimestamp = now;
- offsetOfMaxTimestamp = initialOffset;
- }
-
- if (toMagic >= RecordBatch.MAGIC_VALUE_V2) {
- offsetOfMaxTimestamp = offsetCounter.value - 1;
+ if (toMagic >= RecordBatch.MAGIC_VALUE_V2) {
+ offsetOfMaxTimestamp = offsetCounter.value - 1;
Review Comment:
> It seems KIP-734 does not define the case we are discussing - return the
initial/last offset when there are multi-records having same "max timestamp".
Yes, I agree that we need to be consistent. Using the initial offset makes
the most intuitive sense to me. We could update KIP-734 with this clarification
and also send it to the original vote thread to make sure there is no objection.
--
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]
