Skip to content
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

[fix][broker] Restore solution for certain topic unloading race conditions #20527

Merged

Conversation

lhotari
Copy link
Member

@lhotari lhotari commented Jun 7, 2023

Motivation

It's possible to get into a situation where there are multiple topic instances for the same topic getting unloaded and loaded in the same broker concurrently.
A solution for addressing this problem was introduced in #17526.
However, the solution was removed in #17736 since the call brokerService.removeTopicFromCache(PersistentTopic.this) was replaced with the call brokerService.removeTopicFromCache(topic) since there was the old method signature available in the class.

Modifications

  • restore the call to brokerService.removeTopicFromCache(PersistentTopic.this)
  • remove the method with the incorrect method signature

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

… race conditions

- solution was introduced in apache#17526
- however, it was accidentially replaced with a call to the incorrect
  method signature in apache#17736
@lhotari lhotari added type/bug The PR fixed a bug or issue reported a bug area/broker ready-to-test release/3.0.1 labels Jun 7, 2023
@lhotari lhotari added this to the 3.1.0 milestone Jun 7, 2023
@lhotari lhotari self-assigned this Jun 7, 2023
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jun 7, 2023
Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

Great catch

Removing the old method is good as we won't fall again into the same issue

@lhotari lhotari merged commit 03f9167 into apache:master Jun 7, 2023
lhotari added a commit that referenced this pull request Jun 7, 2023
lhotari added a commit to datastax/pulsar that referenced this pull request Jun 7, 2023
lhotari added a commit that referenced this pull request Jun 8, 2023
lhotari added a commit that referenced this pull request Jun 8, 2023
lhotari added a commit that referenced this pull request Jun 8, 2023
@lhotari lhotari mentioned this pull request Jun 8, 2023
2 tasks
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.

3 participants