dengziming commented on code in PR #12414: URL: https://github.com/apache/kafka/pull/12414#discussion_r923157385
########## core/src/main/scala/kafka/server/metadata/BrokerMetadataListener.scala: ########## @@ -128,7 +134,15 @@ class BrokerMetadataListener( } private def shouldSnapshot(): Boolean = { - (_bytesSinceLastSnapshot >= maxBytesBetweenSnapshots) || metadataVersionChanged() + if (_bytesSinceLastSnapshot >= maxBytesBetweenSnapshots) { + _reasonForSnapshot = "MaxBytesExceeded" Review Comment: since the log is for users, we can use "max bytes exceeded" style format instead of camel case ########## core/src/main/scala/kafka/server/metadata/BrokerMetadataListener.scala: ########## @@ -89,6 +89,12 @@ class BrokerMetadataListener( */ private var _bytesSinceLastSnapshot: Long = 0L + /** + * The reason as to why we are calling maybeStartSnapshot, can be either + * MaxBytesExceeded or MetadataVersionChanged + */ + private var _reasonForSnapshot: String = "" Review Comment: I think it's unnecessary to make "reason" a class variable, a local variable is necessary. -- 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