You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "ykubota (via GitHub)" <gi...@apache.org> on 2023/04/18 09:07:33 UTC

[GitHub] [kafka] ykubota commented on a diff in pull request #13532: KAFKA-14887: No shutdown for ZK session expiration in feature processing

ykubota commented on code in PR #13532:
URL: https://github.com/apache/kafka/pull/13532#discussion_r1169733785


##########
core/src/main/scala/kafka/server/FinalizedFeatureChangeListener.scala:
##########
@@ -160,10 +160,12 @@ class FinalizedFeatureChangeListener(private val finalizedFeatureCache: ZkMetada
           // safe to ignore the exception if the thread is being shutdown. We raise the exception
           // here again, because, it is ignored by ShutdownableThread if it is shutting down.
           throw ie
-        case e: Exception => {
-          error("Failed to process feature ZK node change event. The broker will eventually exit.", e)
+        case cacheUpdateException: FeatureCacheUpdateException =>

Review Comment:
   Thank you for your patch! I'm not a reviewer but I checked because this can be a problem in our own environment, too.
   
   If `IllegalStateException` is thrown because an unknown `FeatureZNodeStatus` is found in the zk node or double invocations when a non-empty notifier was provided in the constructor, is it unnecessary to shut the broker down? 
   Or, how about outputting an error like `IllegalArgumentException` handling inside `updateLatestOrThrow`?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org