KAFKA-14307; Add tests for SnapshotReason

core/src/test/scala/unit/kafka/server/metadata/BrokerMetadataSnapshotterTest.scala

@@ -95,7 +95,7 @@ class BrokerMetadataSnapshotterTest {
   def testCreateSnapshot(): Unit = {
     val writerBuilder = new MockSnapshotWriterBuilder()
     val snapshotter = new BrokerMetadataSnapshotter(0, Time.SYSTEM, None, writerBuilder)
-    
+
     try {
       val blockingEvent = new BlockingEvent()
       val reasons = Set(SnapshotReason.UNKNOWN_REASON)

metadata/src/main/java/org/apache/kafka/controller/QuorumController.java

@@ -1558,6 +1558,10 @@ private void maybeGenerateSnapshot(long batchSizeInBytes) {
                 // Starting a snapshot invalidates any scheduled snapshot generation
                 cancelNextGenerateSnapshot();
             }
+        } else {
+            // TODO: Document why it is safe to skip snapshotting if there is a snapshot in progress.
+            // This has to do with the fact that the controller checks if a new snapshot is required after the current
+            // snapshot completes
         }
     }
 
@@ -2218,7 +2222,7 @@ public CompletableFuture<Long> beginWritingSnapshot() {
                     "Generating a snapshot that includes (epoch={}, offset={}) because, {}.",
                     lastCommittedEpoch,
                     lastCommittedOffset,
-                    SnapshotReason.UNKNOWN_REASON
+                    SnapshotReason.UNKNOWN
                 );
                 snapshotGeneratorManager.createSnapshotGenerator(
                     lastCommittedOffset,

metadata/src/main/java/org/apache/kafka/metadata/util/SnapshotReason.java

@@ -21,17 +21,16 @@
 import org.apache.kafka.server.common.MetadataVersion;
 
 // TODO: Document this type
-// TODO: Write tests for this
 public final class SnapshotReason {
-    static public final SnapshotReason UNKNOWN_REASON = new SnapshotReason("unknown reason");
+    static public final SnapshotReason UNKNOWN = new SnapshotReason("unknown reason");
 
     static public SnapshotReason maxBytesExceeded(long bytes, long maxBytes) {
-        return new SnapshotReason(String.format("%s bytes exceeded the max bytes of %s", bytes, maxBytes));
+        return new SnapshotReason(String.format("%s bytes exceeded the maximum bytes of %s", bytes, maxBytes));
     }
 
     static public SnapshotReason maxIntervalExceeded(long interval, long maxInterval) {
         return new SnapshotReason(
-            String.format("%s ms exceeded the max snapshot interval of %s ms", interval, maxInterval)
+            String.format("%s ms exceeded the maximum snapshot interval of %s ms", interval, maxInterval)
         );
     }
 

metadata/src/test/java/org/apache/kafka/metadata/util/SnapshotReasonTest.java

@@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kafka.metadata.util;
+
+import java.util.Arrays;
+import java.util.List;
+import org.apache.kafka.server.common.MetadataVersion;
+import org.junit.jupiter.api.Test;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public final class SnapshotReasonTest {
+    @Test
+    public void testUnknown() {
+        assertEquals("unknown reason", SnapshotReason.UNKNOWN.toString());
+    }
+
+    @Test
+    public void testMaxBytesExceeded() {
+        long bytes = 1000;
+        long maxBytes = 900;
+        String expectedMessage = "1000 bytes exceeded the maximum bytes of 900";
+
+        assertEquals(expectedMessage, SnapshotReason.maxBytesExceeded(bytes, maxBytes).toString());
+    }
+
+    @Test
+    public void testMaxIntervalExceeded() {
+        long interval = 1000;
+        long maxInterval = 900;
+        String expectedMessage = "1000 ms exceeded the maximum snapshot interval of 900 ms";
+
+        assertEquals(expectedMessage, SnapshotReason.maxIntervalExceeded(interval, maxInterval).toString());
+    }
+
+    @Test
+    public void testMetadataVersionChanged() {
+        MetadataVersion metadataVersion = MetadataVersion.IBP_3_3_IV3;
+        String expectedMessage = "metadata version was changed to 3.3-IV3";
+
+        assertEquals(expectedMessage, SnapshotReason.metadataVersionChanged(metadataVersion).toString());
+    }
+
+    @Test
+    public void testJoinReasons() {
+        long bytes = 1000;
+        long maxBytes = 900;
+        long interval = 1000;
+        long maxInterval = 900;
+        MetadataVersion metadataVersion = MetadataVersion.IBP_3_3_IV3;
+
+        List<SnapshotReason> reasons = Arrays.asList(
+            SnapshotReason.UNKNOWN,
+            SnapshotReason.maxBytesExceeded(bytes, maxBytes),
+            SnapshotReason.maxIntervalExceeded(interval, maxInterval),
+            SnapshotReason.metadataVersionChanged(metadataVersion)
+        );
+
+        String joinedReasons = SnapshotReason.stringFromReasons(reasons);
+
+        assertTrue(joinedReasons.contains("unknown reason"), joinedReasons);
+        assertTrue(joinedReasons.contains("1000 bytes exceeded the maximum bytes of 900"), joinedReasons);
+        assertTrue(joinedReasons.contains("1000 ms exceeded the maximum snapshot interval of 900 ms"), joinedReasons);
+        assertTrue(joinedReasons.contains("metadata version was changed to 3.3-IV3"), joinedReasons);
+    }
+}