junrao commented on code in PR #18861:
URL: https://github.com/apache/kafka/pull/18861#discussion_r1994057549
##########
storage/src/test/java/org/apache/kafka/tiered/storage/actions/ConsumeAction.java:
##########
@@ -171,20 +171,48 @@ public void doExecute(TieredStorageTestContext context)
throws InterruptedExcept
expectedCountAndOp = new
RemoteFetchCount.FetchCountAndOp(-1, RemoteFetchCount.OperationType.EQUALS_TO);
}
- String message = String.format("Number of %s requests from broker
%d to the tier storage does not match the expected value for topic-partition
%s",
- eventType, remoteFetchSpec.getSourceBrokerId(),
remoteFetchSpec.getTopicPartition());
- if (expectedCountAndOp.getCount() != -1) {
- if (expectedCountAndOp.getOperationType() ==
RemoteFetchCount.OperationType.EQUALS_TO) {
- assertEquals(expectedCountAndOp.getCount(),
eventsInScope.size(), message);
- } else if (expectedCountAndOp.getOperationType() ==
RemoteFetchCount.OperationType.LESS_THAN_OR_EQUALS_TO) {
- assertTrue(eventsInScope.size() <=
expectedCountAndOp.getCount(), message);
+ RemoteFetchCount.OperationType exceptedOperationType =
expectedCountAndOp.getOperationType();
Review Comment:
Could we remove the flaky tag added for `DeleteSegmentsByRetentionTimeTest`?
##########
storage/src/test/java/org/apache/kafka/tiered/storage/integration/BaseDeleteSegmentsTest.java:
##########
@@ -55,7 +56,10 @@ protected void
writeTestSpecifications(TieredStorageTestBuilder builder) {
.expectSegmentToBeOffloaded(broker0, topicA, p0, 2, new
KeyValueSpec("k2", "v2"))
.expectEarliestLocalOffsetInLogDirectory(topicA, p0, 3L)
.produceWithTimestamp(topicA, p0, new KeyValueSpec("k0",
"v0"), new KeyValueSpec("k1", "v1"),
- new KeyValueSpec("k2", "v2"), new KeyValueSpec("k3",
"v3", System.currentTimeMillis()))
+ // Using a future timestamp to test the log segment
rolling mechanism.
+ // This simulates the passage of time, triggering
time-based log segment rolling conditions
+ // When remote storage is enabled, this helps test how
segments are marked for deletion
Review Comment:
This comment is not very clear to me. How about the following?
`DeleteSegmentsByRetentionTimeTest uses a really small retention time, which
could cause the active segment to be rolled and deleted. We use a future
timestamp to prevent that from happening.`
--
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
