brandboat commented on PR #16932: URL: https://github.com/apache/kafka/pull/16932#issuecomment-2307035331
In the DeleteSegmentsByRetentionTimeTest, we set `retention.ms` to 1ms and leave `local.retention.ms` at its default value of -2. Before this PR, segment deletion was never triggered by `retention.ms` alone if `local.retention.ms` was left at -2. In the TieredStorageTestUtils#createTopicConfigForRemoteStorage, we set local.retention.bytes to 1 [^1], which effectively triggers segment deletion based on size rather than time. It's important to note that `[local.]retention.ms` deletes active segments, while `[local.]retention.bytes` does not delete active segments if `[local.]retention.bytes` > 0. This difference is the root cause of the flakiness observed in DeleteSegmentsByRetentionTimeTest after this PR. With the changes, `local.retention.ms` is now working as expected, and active segments are being deleted during the test. To address this, https://github.com/apache/kafka/pull/16932/commits/31a36f01310fd17dc2de31c2e186cda9b99b2d2b make the latest record use a timestamp from the future to avoid retention.ms segment deletion. [^1]: https://github.com/apache/kafka/blob/61a661ec5e627217e8b4e4c009d65ee0e0e938ba/storage/src/test/java/org/apache/kafka/tiered/storage/utils/TieredStorageTestUtils.java#L186 -- 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
