mumrah commented on code in PR #12596:
URL: https://github.com/apache/kafka/pull/12596#discussion_r966480469


##########
core/src/main/scala/kafka/server/metadata/BrokerMetadataListener.scala:
##########
@@ -307,11 +322,18 @@ class BrokerMetadataListener(
 
   private def publish(publisher: MetadataPublisher): Unit = {
     val delta = _delta
-    _image = _delta.apply()
+    try {
+      _image = _delta.apply()
+    } catch {
+      case t: Throwable => metadataLoadingFaultHandler.handleFault(s"Error 
applying metadata delta $delta", t)
+    }
+
     _delta = new MetadataDelta(_image)
     if (isDebugEnabled) {
       debug(s"Publishing new metadata delta $delta at offset 
${_image.highestOffsetAndEpoch().offset}.")
     }
+
+    // This publish call is done with its own try-catch and fault handler
     publisher.publish(delta, _image)

Review Comment:
   Thanks, good catch. I missed a `throw` in the catch block above. If we can't 
apply the delta we should not publish the image.
   
   I agree that more tests would be very useful as we harden this code path. 
I'll see what I can come up with for this PR and we can continue adding more 
tests after 3.3



-- 
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-unsubscr...@kafka.apache.org

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

Reply via email to