showuon commented on code in PR #15568:
URL: https://github.com/apache/kafka/pull/15568#discussion_r1533195347


##########
core/src/test/scala/unit/kafka/log/LogManagerTest.scala:
##########
@@ -1303,6 +1303,27 @@ class LogManagerTest {
         createLeaderAndIsrRequestForStrayDetection(present),
         onDisk.map(mockLog(_))).toSet)
   }
+
+  @Test
+  def testLock(): Unit = {
+    val tmpLogDir = TestUtils.tempDir()
+    val tmpLogManager = createLogManager(Seq(tmpLogDir))
+    tmpLogManager.startup(Set.empty)
+
+    // ${tmpLogDir}.lock is acquired by tmpLogManager
+    val fileLock = new FileLock(new File(tmpLogDir, ".lock"))

Review Comment:
   We should use `LogManager.LockFileName` instead.



##########
core/src/test/scala/unit/kafka/log/LogManagerTest.scala:
##########
@@ -1303,6 +1303,27 @@ class LogManagerTest {
         createLeaderAndIsrRequestForStrayDetection(present),
         onDisk.map(mockLog(_))).toSet)
   }
+
+  @Test
+  def testLock(): Unit = {
+    val tmpLogDir = TestUtils.tempDir()
+    val tmpLogManager = createLogManager(Seq(tmpLogDir))
+    tmpLogManager.startup(Set.empty)
+
+    // ${tmpLogDir}.lock is acquired by tmpLogManager
+    val fileLock = new FileLock(new File(tmpLogDir, ".lock"))
+    try {
+      assertFalse(fileLock.tryLock())
+    } finally {
+      fileLock.destroy()
+    }
+
+    // ${tmpLogDir}.lock is removed after shutdown
+    tmpLogManager.shutdown()
+    val f = new File(tmpLogDir, ".lock")
+    assertFalse(f.exists())
+    Utils.delete(tmpLogDir)

Review Comment:
   We should also wrap the logmanager.shutdown and tmpLogDir deletion in the 
`finally` block to make sure resources are released.



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

Reply via email to