KAFKA-16771 First log directory printed twice when formatting storage (…

…apache#16010) Reviewers: Chia-Ping Tsai <[email protected]>
apourchet · May 29, 2024 · 0f0c9ec · 0f0c9ec
1 parent 2d9994e
commit 0f0c9ec
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 7 deletions.
diff --git a/core/src/main/scala/kafka/tools/StorageTool.scala b/core/src/main/scala/kafka/tools/StorageTool.scala
@@ -253,7 +253,7 @@ object StorageTool extends Logging {
            .setServerKey(formatter.serverKey(saltedPassword))
            .setIterations(iterations)
     } catch {
-      case e: Throwable => 
+      case e: Throwable =>
         throw new TerseFailure(s"Error attempting to create UserScramCredentialRecord: ${e.getMessage}")
     }
     myrecord
@@ -464,8 +464,8 @@ object StorageTool extends Logging {
         copier.setWriteErrorHandler((logDir, e) => {
           throw new TerseFailure(s"Error while writing meta.properties file $logDir: ${e.getMessage}")
         })
-        copier.writeLogDirChanges()
       })
+      copier.writeLogDirChanges()
     }
     0
   }

diff --git a/core/src/test/scala/unit/kafka/tools/StorageToolTest.scala b/core/src/test/scala/unit/kafka/tools/StorageToolTest.scala
@@ -206,12 +206,15 @@ Found problem:
 
   @Test
   def testFormatSucceedsIfAllDirectoriesAreAvailable(): Unit = {
-    val availableDir1 = TestUtils.tempDir()
-    val availableDir2 = TestUtils.tempDir()
+    val availableDirs = Seq(TestUtils.tempDir(), TestUtils.tempDir(), TestUtils.tempDir()).map(dir => dir.toString)
     val stream = new ByteArrayOutputStream()
-    assertEquals(0, runFormatCommand(stream, Seq(availableDir1.toString, availableDir2.toString)))
-    assertTrue(stream.toString().contains("Formatting %s".format(availableDir1)))
-    assertTrue(stream.toString().contains("Formatting %s".format(availableDir2)))
+    assertEquals(0, runFormatCommand(stream, availableDirs))
+    val actual = stream.toString().split("\\r?\\n")
+    val expect = availableDirs.map("Formatting %s".format(_))
+    assertEquals(availableDirs.size, actual.size)
+    expect.foreach(dir => {
+      assertEquals(1, actual.count(_.startsWith(dir)))
+    })
   }
 
   @Test
@@ -489,3 +492,4 @@ Found problem:
     }
   }
 }
+