Skip to content

KAFKA-19383: Handle the deleted topics when applying ClearElrRecord #19916

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 24, 2025

Conversation

CalvinConfluent
Copy link
Contributor

@CalvinConfluent CalvinConfluent commented Jun 6, 2025

https://issues.apache.org/jira/browse/KAFKA-19383 When applying the
ClearElrRecord, it may pick up the topicId in the image without checking
if the topic has been deleted. This can cause the creation of a new
TopicRecord with an old topic ID.

@github-actions github-actions bot added triage PRs from the community kraft labels Jun 6, 2025
topicId = createdTopics.get(record.topicName());
} else {
topicId = image.getTopic(record.topicName()).id();
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we guaranteed that image.getTopic(record.topicName()) is not null? The previous code checked that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Updated with a new UT.

@github-actions github-actions bot removed the triage PRs from the community label Jun 7, 2025
Copy link
Contributor

@ahuang98 ahuang98 left a comment

Choose a reason for hiding this comment

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

Thanks for the fix Calvin - non-test logic looks good to go, build is failing on your tests though

newTopicImage(
"foo",
fooId,
NULL_MIRROR_TOPIC_STATE,
Copy link
Contributor

Choose a reason for hiding this comment

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

😬
noticed the build failed, think we need to alter some of these tests

Copy link
Contributor

@ahuang98 ahuang98 left a comment

Choose a reason for hiding this comment

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

thanks for the test fixes!

Copy link
Contributor

@cmccabe cmccabe left a comment

Choose a reason for hiding this comment

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

LGTM

@cmccabe cmccabe merged commit dcc9320 into apache:trunk Jun 24, 2025
24 checks passed
CalvinConfluent added a commit to CalvinConfluent/kafka that referenced this pull request Jun 24, 2025
…pache#19916)

https://issues.apache.org/jira/browse/KAFKA-19383  When applying the
ClearElrRecord, it may pick up the topicId in the image without checking
if the topic has been deleted. This can cause the creation of a new
TopicRecord with an old topic ID.

Reviewers: Alyssa Huang <ahuang@confluent.io>, Artem Livshits <alivshits@confluent.io>, Colin P. McCabe <cmccabe@apache.org>
CalvinConfluent added a commit to CalvinConfluent/kafka that referenced this pull request Jun 24, 2025
…pache#19916)

https://issues.apache.org/jira/browse/KAFKA-19383  When applying the
ClearElrRecord, it may pick up the topicId in the image without checking
if the topic has been deleted. This can cause the creation of a new
TopicRecord with an old topic ID.

Reviewers: Alyssa Huang <ahuang@confluent.io>, Artem Livshits <alivshits@confluent.io>, Colin P. McCabe <cmccabe@apache.org>
CalvinConfluent added a commit to CalvinConfluent/kafka that referenced this pull request Jun 24, 2025
…pache#19916)

https://issues.apache.org/jira/browse/KAFKA-19383  When applying the
ClearElrRecord, it may pick up the topicId in the image without checking
if the topic has been deleted. This can cause the creation of a new
TopicRecord with an old topic ID.

Reviewers: Alyssa Huang <ahuang@confluent.io>, Artem Livshits <alivshits@confluent.io>, Colin P. McCabe <cmccabe@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.

5 participants