[GitHub] [kafka] ashmeet13 commented on a diff in pull request #12414: KAFKA-14073 Logging the reason for Snapshot

[GitBox <gi...@apache.org>]

ashmeet13 commented on code in PR #12414:
URL: https://github.com/apache/kafka/pull/12414#discussion_r924807677


##########
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:
   Thanks, I have made the change. I have handled it by returning a `Tuple` from the `shouldSnapshot` function.
   I am new to Scala, if there is another recommended way of doing it please let me know!



##########
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:
   Got it, made this change



-- 
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