You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "m1a2st (via GitHub)" <gi...@apache.org> on 2025/02/11 14:07:49 UTC

[PR] [Testing]KAFKA-18606: Flaky test DeleteSegmentsByRetentionTimeTest#executeTieredStorageTest [kafka]

m1a2st opened a new pull request, #18861:
URL: https://github.com/apache/kafka/pull/18861

   *More detailed description of your change,
   if necessary. The PR title and PR message become
   the squashed commit message, so use a separate
   comment to ping reviewers.*
   
   *Summary of testing strategy (including rationale)
   for the feature or bug fix. Unit and/or integration
   tests are expected for any behaviour change and
   system tests should be considered for larger changes.*
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-18606: Flaky test DeleteSegmentsByRetentionTimeTest#executeTieredStorageTest [kafka]

Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #18861:
URL: https://github.com/apache/kafka/pull/18861#issuecomment-3646039878

   ugh, please ignore my previous comment. the patch was actually backported to 4.0 as well. see beda2f3917c7cca493b8f3228e1d40703359c9e4


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-18606: Flaky test DeleteSegmentsByRetentionTimeTest#executeTieredStorageTest [kafka]

Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on code in PR #18861:
URL: https://github.com/apache/kafka/pull/18861#discussion_r1994461137


##########
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:
   I think we should keep the flaky annotation for now and observe for a while. Once we confirm that no errors occur in the CI, we can proceed with its removal. WDYT



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-18606: Flaky test DeleteSegmentsByRetentionTimeTest#executeTieredStorageTest [kafka]

Posted by "junrao (via GitHub)" <gi...@apache.org>.
junrao merged PR #18861:
URL: https://github.com/apache/kafka/pull/18861


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-18606: Flaky test DeleteSegmentsByRetentionTimeTest#executeTieredStorageTest [kafka]

Posted by "junrao (via GitHub)" <gi...@apache.org>.
junrao commented on code in PR #18861:
URL: https://github.com/apache/kafka/pull/18861#discussion_r1989904345


##########
storage/src/test/java/org/apache/kafka/tiered/storage/integration/BaseDeleteSegmentsTest.java:
##########
@@ -55,7 +56,7 @@ 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()))
+                        new KeyValueSpec("k2", "v2"), new KeyValueSpec("k3", "v3", System.currentTimeMillis() + TimeUnit.HOURS.toMillis(1)))

Review Comment:
   Could we add a comment on why we need to use a future timestamp?



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-18606: Flaky test DeleteSegmentsByRetentionTimeTest#executeTieredStorageTest [kafka]

Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on PR #18861:
URL: https://github.com/apache/kafka/pull/18861#issuecomment-2721018031

   Thanks for @junrao review, addressed all comments.


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-18606: Flaky test DeleteSegmentsByRetentionTimeTest#executeTieredStorageTest [kafka]

Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #18861:
URL: https://github.com/apache/kafka/pull/18861#issuecomment-3646000992

   > This flaky test is caused by https://github.com/apache/kafka/commit/23c459286bcf6c5a93fb629fd15f6398aa3e99ed
   
   the commit was shipped in 4.1, but the test is still flaky in 4.0.  @m1a2st WDYT?
   
   ```
   Gradle Test Run :storage:test > Gradle Test Executor 33 > DeleteSegmentsByRetentionTimeTest > executeTieredStorageTest(String, String) > executeTieredStorageTest(String, String).quorum=kraft.groupProtocol=classic FAILED
       org.opentest4j.AssertionFailedError: Too many records found in tiered storage from offset 0 for topicA-0. Expected: 0, Was: 1 ==> expected: <false> but was: <true>
           at app//org.apache.kafka.tiered.storage.actions.ConsumeAction.doExecute(ConsumeAction.java:119)
           at app//org.apache.kafka.tiered.storage.TieredStorageTestAction.execute(TieredStorageTestAction.java:25)
           at app//org.apache.kafka.tiered.storage.TieredStorageTestHarness.executeTieredStorageTest(TieredStorageTestHarness.java:116)
           at java.base@21.0.9/java.lang.reflect.Method.invoke(Method.java:580)
           at java.base@21.0.9/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:184)
           at java.base@21.0.9/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
           at java.base@21.0.9/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:179)
           at java.base@21.0.9/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
           at java.base@21.0.9/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:184)
           at java.base@21.0.9/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
           at java.base@21.0.9/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
           at java.base@21.0.9/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
           at java.base@21.0.9/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:184)
           at java.base@21.0.9/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
           at java.base@21.0.9/java.util.Spliterators$ArraySpliterator.forEachRemaining(Spliterators.java:1024)
           at java.base@21.0.9/java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:762)
           at java.base@21.0.9/java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:276)
           at java.base@21.0.9/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
           at java.base@21.0.9/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
           at java.base@21.0.9/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
           at java.base@21.0.9/java.util.Spliterators$ArraySpliterator.forEachRemaining(Spliterators.java:1024)
           at java.base@21.0.9/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
           at java.base@21.0.9/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
           at java.base@21.0.9/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151)
           at java.base@21.0.9/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174)
           at java.base@21.0.9/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
           at java.base@21.0.9/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596)
           at java.base@21.0.9/java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:276)
           at java.base@21.0.9/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
           at java.base@21.0.9/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
           at java.base@21.0.9/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
           at java.base@21.0.9/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1708)
           at java.base@21.0.9/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
           at java.base@21.0.9/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
           at java.base@21.0.9/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151)
           at java.base@21.0.9/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174)
           at java.base@21.0.9/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
           at java.base@21.0.9/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596)
           at java.base@21.0.9/java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:276)
           at java.base@21.0.9/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1708)
           at java.base@21.0.9/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
           at java.base@21.0.9/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
           at java.base@21.0.9/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151)
           at java.base@21.0.9/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174)
           at java.base@21.0.9/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
           at java.base@21.0.9/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596)
           at java.base@21.0.9/java.util.ArrayList.forEach(ArrayList.java:1596)
           at java.base@21.0.9/java.util.ArrayList.forEach(ArrayList.java:1596)
   ```


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-18606: Flaky test DeleteSegmentsByRetentionTimeTest#executeTieredStorageTest [kafka]

Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on PR #18861:
URL: https://github.com/apache/kafka/pull/18861#issuecomment-2724727714

   @m1a2st how close is this fix? We are seeing a lot of impact from this test across many PR builds. 
   <img width="893" alt="image" src="https://github.com/user-attachments/assets/97158bab-3e30-4b6a-b556-0388d82b3f2d" />
   
   This means 27% of PR builds are failing due to the flakiness of this test.


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-18606: Flaky test DeleteSegmentsByRetentionTimeTest#executeTieredStorageTest [kafka]

Posted by "junrao (via GitHub)" <gi...@apache.org>.
junrao commented on code in PR #18861:
URL: https://github.com/apache/kafka/pull/18861#discussion_r1992116002


##########
storage/src/test/java/org/apache/kafka/tiered/storage/integration/BaseDeleteSegmentsTest.java:
##########
@@ -55,7 +56,7 @@ 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()))
+                        new KeyValueSpec("k2", "v2"), new KeyValueSpec("k3", "v3", System.currentTimeMillis() + TimeUnit.HOURS.toMillis(1)))

Review Comment:
   Also, the error message in ConsumerAction is not very clear. It would be useful to include the expected count, the actual count and the OperationType.
   
   `Number of FETCH_OFFSET_INDEX requests from broker 0 to the tier storage does not match the expected value for topic-partition topicA-0 ==> expected: <true> but was: <false>`



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-18606: Flaky test DeleteSegmentsByRetentionTimeTest#executeTieredStorageTest [kafka]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18861:
URL: https://github.com/apache/kafka/pull/18861#issuecomment-2676533009

   A label of 'needs-attention' was automatically added to this PR in order to raise the
   attention of the committers. Once this issue has been triaged, the `triage` label
   should be removed to prevent this automation from happening again.


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-18606: Flaky test DeleteSegmentsByRetentionTimeTest#executeTieredStorageTest [kafka]

Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on PR #18861:
URL: https://github.com/apache/kafka/pull/18861#issuecomment-2724751172

   Hello @mumrah, I believe this PR is complete. If @junrao has no further comments, we should be able to merge it today.


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-18606: Flaky test DeleteSegmentsByRetentionTimeTest#executeTieredStorageTest [kafka]

Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on PR #18861:
URL: https://github.com/apache/kafka/pull/18861#issuecomment-2674892860

   This test still flaky on CI [gradle report](https://develocity.apache.org/scans/tests?search.relativeStartTime=P28D&search.rootProjectNames=kafka&search.tags=github&search.timeZoneId=Asia%2FTaipei&tests.container=org.apache.kafka.tiered.storage.integration.DeleteSegmentsByRetentionTimeTest)


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-18606: Flaky test DeleteSegmentsByRetentionTimeTest#executeTieredStorageTest [kafka]

Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on PR #18861:
URL: https://github.com/apache/kafka/pull/18861#issuecomment-2653706965

   It seems this test not flaky these days on trunk, I will observe for few days, if  alaways pass on trunk, I will close this PR.
   
   https://develocity.apache.org/scans/tests?search.rootProjectNames=kafka&search.tags=github%2Ctrunk&search.timeZoneId=Asia%2FTaipei&tests.container=org.apache.kafka.tiered.storage.integration.DeleteSegmentsByRetentionTimeTest


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-18606: Flaky test DeleteSegmentsByRetentionTimeTest#executeTieredStorageTest [kafka]

Posted by "junrao (via GitHub)" <gi...@apache.org>.
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-18606: Flaky test DeleteSegmentsByRetentionTimeTest#executeTieredStorageTest [kafka]

Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on code in PR #18861:
URL: https://github.com/apache/kafka/pull/18861#discussion_r1995588880


##########
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:
   Yes, I would prefer to keep the Flaky annotation for a little while to make sure this does not impact builds.



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-18606: Flaky test DeleteSegmentsByRetentionTimeTest#executeTieredStorageTest [kafka]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18861:
URL: https://github.com/apache/kafka/pull/18861#issuecomment-2667405716

   A label of 'needs-attention' was automatically added to this PR in order to raise the
   attention of the committers. Once this issue has been triaged, the `triage` label
   should be removed to prevent this automation from happening again.


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-18606: Flaky test DeleteSegmentsByRetentionTimeTest#executeTieredStorageTest [kafka]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18861:
URL: https://github.com/apache/kafka/pull/18861#issuecomment-2680305127

   A label of 'needs-attention' was automatically added to this PR in order to raise the
   attention of the committers. Once this issue has been triaged, the `triage` label
   should be removed to prevent this automation from happening again.


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-18606: Flaky test DeleteSegmentsByRetentionTimeTest#executeTieredStorageTest [kafka]

Posted by "junrao (via GitHub)" <gi...@apache.org>.
junrao commented on code in PR #18861:
URL: https://github.com/apache/kafka/pull/18861#discussion_r1990146496


##########
storage/src/test/java/org/apache/kafka/tiered/storage/integration/BaseDeleteSegmentsTest.java:
##########
@@ -55,7 +56,7 @@ 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()))
+                        new KeyValueSpec("k2", "v2"), new KeyValueSpec("k3", "v3", System.currentTimeMillis() + TimeUnit.HOURS.toMillis(1)))

Review Comment:
   When remote storage is enabled, we will roll a segment subject for deletion due to retention. This could eventually lead to the tiering and the deletion of the segment.
   
   ```
           // Roll the active segment when it breaches the configured retention policy. The rolled segment will be
           // eligible for deletion and gets removed in the next iteration.
           if (predicateResult && remoteLogEnabled() && nextSegmentOpt.isEmpty && segment.size > 0) {
             shouldRoll = true
           }
   ```



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org