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