FrankYang0529 commented on code in PR #18012:
URL: https://github.com/apache/kafka/pull/18012#discussion_r1914017810
##########
storage/src/test/java/org/apache/kafka/storage/internals/log/LogSegmentTest.java:
##########
@@ -342,7 +344,9 @@ public void testTruncateFull() throws IOException {
MockTime time = new MockTime();
try (LogSegment seg = createSegment(40, time)) {
- seg.append(41, RecordBatch.NO_TIMESTAMP, -1L, v1Records(40,
"hello", "there"));
+ seg.append(41,
+ MemoryRecords.withRecords(RecordBatch.MAGIC_VALUE_V1, 40,
Compression.NONE, TimestampType.CREATE_TIME,
Review Comment:
> Why do we change from inserting 2 records to 1 record?
My mistake. Changed it back to 2 records.
> could we continue using `v1Records`?
Probably no. The old test rely on `largestTimestampMs` to assign
`rollingBasedTimestamp` as `RecordBatch.NO_TIMESTAMP`. In this PR, we assign
first batch max timestamp for `rollingBasedTimestamp`. In `v1Records`, it sets
record timestamp as `offset * 10`, so it's not `RecordBatch.NO_TIMESTAMP`.
https://github.com/apache/kafka/blob/da0c3beffa96944a899ea949a7ab5705f67be882/storage/src/test/java/org/apache/kafka/storage/internals/log/LogSegmentTest.java#L345
https://github.com/apache/kafka/blob/da0c3beffa96944a899ea949a7ab5705f67be882/storage/src/test/java/org/apache/kafka/storage/internals/log/LogSegmentTest.java#L95-L98
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
