Skip to content

KAFKA-13499: Avoid restoring outdated records#21901

Merged
bbejeck merged 8 commits intoapache:trunkfrom
gabriellefu:revert_rw
Mar 31, 2026
Merged

KAFKA-13499: Avoid restoring outdated records#21901
bbejeck merged 8 commits intoapache:trunkfrom
gabriellefu:revert_rw

Conversation

@gabriellefu
Copy link
Copy Markdown
Contributor

@gabriellefu gabriellefu commented Mar 30, 2026

  1. Expose the retentionPeriod length to storeMetadata
  2. In prepareChangelogs(), switch it from always seektobeginning if
    checkpoint doesn't exist to seek to certain timestamp to avoid restoring
    outdated records.

Reviewers: TengYao Chi frankvicky@apache.org , Bill Bejeck
bbejeck@apache.org

@gabriellefu gabriellefu marked this pull request as ready for review March 30, 2026 22:03
@github-actions github-actions Bot added triage PRs from the community streams labels Mar 30, 2026
@frankvicky frankvicky added ci-approved and removed triage PRs from the community labels Mar 30, 2026
Copy link
Copy Markdown
Member

@bbejeck bbejeck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gabriellefu the AbstractDualSchemaRocksDBSegmentBytesStore needs to implement the WithRetentionPeriod interface

@bbejeck
Copy link
Copy Markdown
Member

bbejeck commented Mar 31, 2026

@gabriellefu same issue with AbstractRocksDBSegmentedBytesStore.java

Copy link
Copy Markdown
Member

@bbejeck bbejeck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @gabriellefu LGTM

@bbejeck bbejeck merged commit f55073b into apache:trunk Mar 31, 2026
21 checks passed
@bbejeck
Copy link
Copy Markdown
Member

bbejeck commented Mar 31, 2026

Merged #21901 into trunk

bbejeck pushed a commit that referenced this pull request Mar 31, 2026
1. Expose the retentionPeriod length to storeMetadata
2. In prepareChangelogs(), switch it from always seektobeginning if
checkpoint doesn't exist to seek to certain timestamp to avoid restoring
outdated records.

Reviewers: TengYao Chi <frankvicky@apache.org> , Bill Bejeck
 <bbejeck@apache.org>
@bbejeck
Copy link
Copy Markdown
Member

bbejeck commented Mar 31, 2026

cherry-picked to 4.3

mjsax added a commit that referenced this pull request Apr 9, 2026
@mjsax
Copy link
Copy Markdown
Member

mjsax commented Apr 9, 2026

It seems this PR did introduce some bug. Reverted it from 4.3 to not delay the release unnecessary. For trunk, let's see how we can fix forward.

It seems to be related to TimestampedToHeadersWindowStoreAdapter (that we added via KIP-1271/1285) which doesn't implement WrappedStateStore, und the newly introduced WithRetentionPeriod interface of this PR.

@mjsax
Copy link
Copy Markdown
Member

mjsax commented Apr 14, 2026

Also reverted in trunk now.

mjsax added a commit that referenced this pull request Apr 14, 2026
nileshkumar3 pushed a commit to nileshkumar3/kafka that referenced this pull request Apr 15, 2026
1. Expose the retentionPeriod length to storeMetadata
2. In prepareChangelogs(), switch it from always seektobeginning if
checkpoint doesn't exist to seek to certain timestamp to avoid restoring
outdated records.

Reviewers: TengYao Chi <frankvicky@apache.org> , Bill Bejeck
 <bbejeck@apache.org>
nileshkumar3 pushed a commit to nileshkumar3/kafka that referenced this pull request Apr 15, 2026
bbejeck pushed a commit that referenced this pull request May 4, 2026
1. Expose the retentionPeriod length to storeMetadata
2. In prepareChangelogs(), switch it from always seektobeginning if
checkpoint doesn't exist to seek to certain timestamp to avoid restoring
outdated records.
3. Change from [the ](#21901):
Instead of the wall clock, use the latest timestamp in the changelog as
the latest time, and seek from the timestamp of
latest_changelog_stamp_time-rention_period.

Reviewers: TengYao Chi <frankvicky@apache.org>, Bill Bejeck
 <bbejeck@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants