jsancio commented on code in PR #12596:
URL: https://github.com/apache/kafka/pull/12596#discussion_r966459040
##########
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:
Is this correct? If there was an error in `_image = _delta.apply()`,
`_image` will be the previous image that was published while `delta` is the new
`_delta` that was not applied. Also, note that I am pretty sure that the code
`publisher.publish` assumes that this layer doesn't send duplicate deltas and
images.
Is there a way we can write tests for this code and scenarios so that we can
increase our confidence that this code behaves as we expect?
--
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
