You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2022/12/19 17:33:22 UTC

[GitHub] [kafka] mumrah opened a new pull request, #13019: KAFKA-14531 Fix controller snapshot interval

mumrah opened a new pull request, #13019:
URL: https://github.com/apache/kafka/pull/13019

   Previously, we were passing in the `metadata.log.max.snapshot.interval.ms` config value as nanoseconds to SnapshotGenerator. With the default value of 1 hour (3600000 milliseconds), this results in a snapshot ever 0.0036 seconds. 


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


[GitHub] [kafka] jsancio commented on a diff in pull request #13019: KAFKA-14531 Fix controller snapshot interval

Posted by GitBox <gi...@apache.org>.
jsancio commented on code in PR #13019:
URL: https://github.com/apache/kafka/pull/13019#discussion_r1053638059


##########
core/src/test/scala/integration/kafka/server/KRaftClusterTest.scala:
##########
@@ -917,4 +919,34 @@ class KRaftClusterTest {
       cluster.close()
     }
   }
+
+  @Test
+  def testSnapshotCount(): Unit = {
+    val cluster = new KafkaClusterTestKit.Builder(
+      new TestKitNodes.Builder().
+        setNumBrokerNodes(0).
+        setNumControllerNodes(1).build())
+      .setConfigProp("metadata.log.max.snapshot.interval.ms", "500")
+      .setConfigProp("metadata.max.idle.interval.ms", "50") // Set this low to generate metadata
+      .build()
+
+    try {
+      cluster.format()
+      cluster.startup()
+      def snapshotCounter(path: Path): Long = {
+       path.toFile.listFiles((_: File, name: String) => {
+          name.toLowerCase.endsWith("checkpoint")
+        }).length
+      }
+
+      val metaLog = FileSystems.getDefault.getPath(cluster.controllers().get(3000).config.metadataLogDir, "__cluster_metadata-0")
+      TestUtils.waitUntilTrue(() => { snapshotCounter(metaLog) > 0 }, "Failed to see at least one snapshot")
+      Thread.sleep(500 * 10) // Sleep for 10 snapshot intervals

Review Comment:
   Instead of sleeping the thread did you consider waiting for x number of snapshots get generated and see how long that took?



##########
core/src/test/scala/integration/kafka/server/KRaftClusterTest.scala:
##########
@@ -917,4 +919,34 @@ class KRaftClusterTest {
       cluster.close()
     }
   }
+
+  @Test
+  def testSnapshotCount(): Unit = {
+    val cluster = new KafkaClusterTestKit.Builder(
+      new TestKitNodes.Builder().
+        setNumBrokerNodes(0).
+        setNumControllerNodes(1).build())
+      .setConfigProp("metadata.log.max.snapshot.interval.ms", "500")
+      .setConfigProp("metadata.max.idle.interval.ms", "50") // Set this low to generate metadata
+      .build()
+
+    try {
+      cluster.format()
+      cluster.startup()
+      def snapshotCounter(path: Path): Long = {
+       path.toFile.listFiles((_: File, name: String) => {
+          name.toLowerCase.endsWith("checkpoint")
+        }).length
+      }
+
+      val metaLog = FileSystems.getDefault.getPath(cluster.controllers().get(3000).config.metadataLogDir, "__cluster_metadata-0")

Review Comment:
   I assume that `3000` comes from a hard-coded value in `KafkaClusterTestKit` can we expose that value instead of also hard-coding it here?



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


[GitHub] [kafka] mumrah commented on pull request #13019: KAFKA-14531 Fix controller snapshot interval

Posted by GitBox <gi...@apache.org>.
mumrah commented on PR #13019:
URL: https://github.com/apache/kafka/pull/13019#issuecomment-1358156533

   @jsancio I added an integration test, PTAL


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


[GitHub] [kafka] mumrah merged pull request #13019: KAFKA-14531 Fix controller snapshot interval

Posted by GitBox <gi...@apache.org>.
mumrah merged PR #13019:
URL: https://github.com/apache/kafka/pull/13019


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