chia7712 commented on code in PR #15568:
URL: https://github.com/apache/kafka/pull/15568#discussion_r1536737154
##########
core/src/test/scala/unit/kafka/log/LogManagerTest.scala:
##########
@@ -1303,6 +1303,29 @@ class LogManagerTest {
createLeaderAndIsrRequestForStrayDetection(present),
onDisk.map(mockLog(_))).toSet)
}
+
+ /**
+ * Test LogManager takes file lock by default and the lock is released after
shutdown.
+ */
+ @Test
+ def testLock(): Unit = {
+ val tmpLogDir = TestUtils.tempDir()
+ val tmpLogManager = createLogManager(Seq(tmpLogDir))
+
+ // ${tmpLogDir}.lock is acquired by tmpLogManager
+ val fileLock = new FileLock(new File(tmpLogDir, LogManager.LockFileName))
+ try {
+ assertFalse(fileLock.tryLock())
+ } finally {
+ // ${tmpLogDir}.lock is removed after shutdown
+ tmpLogManager.shutdown()
+ val f = new File(tmpLogDir, LogManager.LockFileName)
+ assertFalse(f.exists())
+
+ fileLock.destroy()
Review Comment:
why we need to call `destroy` here?
##########
core/src/test/scala/unit/kafka/log/LogManagerTest.scala:
##########
@@ -1303,6 +1303,29 @@ class LogManagerTest {
createLeaderAndIsrRequestForStrayDetection(present),
onDisk.map(mockLog(_))).toSet)
}
+
+ /**
+ * Test LogManager takes file lock by default and the lock is released after
shutdown.
+ */
+ @Test
+ def testLock(): Unit = {
+ val tmpLogDir = TestUtils.tempDir()
+ val tmpLogManager = createLogManager(Seq(tmpLogDir))
+
+ // ${tmpLogDir}.lock is acquired by tmpLogManager
+ val fileLock = new FileLock(new File(tmpLogDir, LogManager.LockFileName))
+ try {
+ assertFalse(fileLock.tryLock())
+ } finally {
+ // ${tmpLogDir}.lock is removed after shutdown
+ tmpLogManager.shutdown()
+ val f = new File(tmpLogDir, LogManager.LockFileName)
+ assertFalse(f.exists())
+
+ fileLock.destroy()
+ Utils.delete(tmpLogDir)
Review Comment:
`TestUtils.tempDir` can delete the temp folder when the VM terminates, so
you don't need to delete the folder manually.
##########
core/src/main/scala/kafka/utils/FileLock.scala:
##########
@@ -65,8 +65,9 @@ class FileLock(val file: File) extends Logging {
def unlock(): Unit = {
this synchronized {
trace(s"Releasing lock on ${file.getAbsolutePath}")
- if (flock != null)
+ if (flock != null) {
Review Comment:
this change is unnecessary now.
##########
core/src/test/scala/unit/kafka/log/LogManagerTest.scala:
##########
@@ -1392,4 +1415,4 @@ object LogManagerTest {
data.topicStates().addAll(topics.values())
new LeaderAndIsrRequest(data, 7.toShort)
}
-}
\ No newline at end of file
+}
Review Comment:
ditto
--
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
