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