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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
