Skip to content

Conversation

@jiafu1115
Copy link
Contributor

When currentTime- largestTimestamp of segment = retentionMs, the behavior is different:

  1. Local Segment delete: Not delete
org.apache.kafka.storage.internals.log.UnifiedLog#deleteRetentionMsBreachedSegments   
boolean delete = startMs - segment.largestTimestamp() > retentionMs;
  1. Remote segement delete: Delete
org.apache.kafka.server.log.remote.storage.RemoteLogManager.RLMExpirationTask.RemoteLogRetentionHandler#isSegmentBreachedByRetentionTime
shouldDeleteSegment = metadata.maxTimestampMs() <= retentionTimeData.get().cleanupUntilMs;

//cleanupUntilMs is time.milliseconds() - retentionMs;
private Optional<RetentionTimeData> buildRetentionTimeData(long retentionMs) {
    long cleanupUntilMs = time.milliseconds() - retentionMs;
    return retentionMs > -1 && cleanupUntilMs >= 0
            ? Optional.of(new RetentionTimeData(retentionMs, cleanupUntilMs))
            : Optional.empty();
}

cc @kamalcph

Thanks for your comments on KIP-1241 .
While I thinking through one of the your comments and reading the code, I noticed a potential problem in this area, so I opened this PR. Many thanks.

@github-actions github-actions bot added triage PRs from the community storage Pull requests that target the storage module tiered-storage Related to the Tiered Storage feature labels Jan 24, 2026
@jiafu1115 jiafu1115 changed the title Fix inconsistency in time-based retention checks between remote and local segment deletion logic. Minor: Fix inconsistency in time-based retention checks between remote and local segment deletion logic. Jan 24, 2026
@github-actions github-actions bot added the small Small PRs label Jan 24, 2026
@jiafu1115 jiafu1115 changed the title Minor: Fix inconsistency in time-based retention checks between remote and local segment deletion logic. KAFKA-20091: Fix inconsistency in time-based retention checks between remote and local segment deletion logic. Jan 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

small Small PRs storage Pull requests that target the storage module tiered-storage Related to the Tiered Storage feature triage PRs from the community

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants