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

[LI-CHERRY-PICK] KAFKA-14887: FinalizedFeatureChangeListener should not shut down when ZK session expires #484

Merged
merged 4 commits into from
Nov 1, 2023

Conversation

lmr3796
Copy link

@lmr3796 lmr3796 commented Oct 30, 2023

FinalizedFeatureChangeListener shuts the broker down when it encounters an issue trying to process feature change events. However, it does not distinguish between issues related to feature changes actually failing and other exceptions like ZooKeeper session expiration. This introduces the possibility that Zookeeper session expiration could cause the broker to shutdown, which is not intended. This patch updates the code to distinguish between these two types of exceptions. In the case of something like a ZK session expiration it logs a warning and continues. We shutdown the broker only for FeatureCacheUpdateException.

Reviewers: Kamal Chandraprakash [email protected], Christo Lolov [email protected], Colin P. McCabe [email protected]

…ot shut down when ZK session expires

FinalizedFeatureChangeListener shuts the broker down when it encounters an issue trying to process feature change
events. However, it does not distinguish between issues related to feature changes actually failing and other
exceptions like ZooKeeper session expiration. This introduces the possibility that Zookeeper session expiration
could cause the broker to shutdown, which is not intended. This patch updates the code to distinguish between
these two types of exceptions. In the case of something like a ZK session expiration it logs a warning and continues.
We shutdown the broker only for FeatureCacheUpdateException.

Reviewers: Kamal Chandraprakash <[email protected]>, Christo Lolov <[email protected]>, Colin P. McCabe <[email protected]>
Copy link

@CCisGG CCisGG left a comment

Choose a reason for hiding this comment

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

Overall looks good. Left a non-blocking comment.

@lmr3796 lmr3796 merged commit ea526ac into 3.0-li Nov 1, 2023
25 checks passed
@lmr3796 lmr3796 deleted the tlin-patch7 branch November 1, 2023 03:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants