FrankYang0529 commented on PR #15773:
URL: https://github.com/apache/kafka/pull/15773#issuecomment-2069404596

   > I think the better solution (or a workaround) for it, should be that we 
set the `nextDeleteDelayMs = Math.max(1, fileDeleteDelayMs)` if if 
logsToBeDeleted is empty. So, if `fileDeleteDelayMs=0`, we can jump out the 
while loop and schedule next task after 1ms (not 0ms, but should be 
acceptable). WDYT?
   
   Agree, we should avoid to pick an element from `logsToBeDeleted` if it's 
empty.
   
   > Also, why could we not be able to use logMangerTest unit test to test this 
behavior? It looks to me using integration test for this issue is overkill 
since we don't test any "integration behavior" here, just a simple scheduler 
behavior. WDYT?
   
   Yeah, I tried to use unit test. However, I didn't find a good way to check 
whether `kafka-delete-logs` task has run or not. I also tried to check whether 
scheduler can be stopped. However, the mock scheduler can still add new task 
during scheduler shutdown.
   
   ```scala
     @Test
     def testDeleteLogs(): Unit = {
       val tmpLogDir = TestUtils.tempDir()
       val tmpProperties = new Properties()
       tmpProperties.put(ServerLogConfigs.LOG_DELETE_DELAY_MS_CONFIG, "0")
       val mockTime = new MockTime()
       val tmpLogManager = TestUtils.createLogManager(
         defaultConfig = new LogConfig(tmpProperties),
         configRepository = new MockConfigRepository,
         logDirs = Seq(tmpLogDir),
         time = mockTime,
         recoveryThreadsPerDataDir = 1,
         initialTaskDelayMs = 0)
   
       tmpLogManager.startup(Set.empty)
       val stopLogManager: Executable = () => tmpLogManager.shutdown()
       val stopScheduler: Executable = () => mockTime.scheduler.shutdown()
       assertTimeoutPreemptively(Duration.ofMillis(5000), stopLogManager)
       assertTimeoutPreemptively(Duration.ofMillis(5000), stopScheduler)
     }
   ```
   


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