junrao commented on code in PR #15631:
URL: https://github.com/apache/kafka/pull/15631#discussion_r1556331481
##########
core/src/test/scala/unit/kafka/log/UnifiedLogTest.scala:
##########
@@ -952,8 +952,9 @@ class UnifiedLogTest {
assertEquals(0, lastSeq)
}
- @Test
- def testRetentionDeletesProducerStateSnapshots(): Unit = {
+ @ParameterizedTest(name = "testRetentionDeletesProducerStateSnapshots with
empty-active-segment: {0}")
+ @ValueSource(booleans = Array(true, false))
+ def testRetentionDeletesProducerStateSnapshots(isEmptyActiveSegment:
Boolean): Unit = {
Review Comment:
isEmptyActiveSegment => createEmptyActiveSegment and change the test name
accordingly.
##########
core/src/main/scala/kafka/log/UnifiedLog.scala:
##########
@@ -1532,10 +1532,14 @@ class UnifiedLog(@volatile var logStartOffset: Long,
}
}
localLog.checkIfMemoryMappedBufferClosed()
- // remove the segments for lookups
- localLog.removeAndDeleteSegments(segmentsToDelete, asyncDelete = true,
reason)
+ if (segmentsToDelete.nonEmpty) {
+ // increment the local-log-start-offset before removing the segment
for lookups
Review Comment:
We probably should say "increment local-log-start-offset or
log-start-offset" since `incrementStartOffset()` could update either one.
--
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
