HDDS-14236. Fix BackgroundService to follow scheduleWithFixedDelay semantics #9595
Conversation
ivandika3
changed the title
| LOG.info("Starting service {} with interval {} {}", serviceName, | ||
| interval, unit.name().toLowerCase()); | ||
| exec.scheduleWithFixedDelay(service, 0, interval, unit); | ||
| scheduler.scheduleWithFixedDelay(service, 0, interval, unit); |
There was a problem hiding this comment.
@xichen01 , thanks for continue this improvement task.
Can we keep using the exec here? What's the extra benefit of introducing scheduler?
There was a problem hiding this comment.
PeriodicalTask#run is now a blocking method. If exec only has one thread, when the PeriodicalTask#run blocks, Then the task submitted to the queue will remain without a thread to execute it.
Therefore, we introduce a separate thread scheduler responsible for submitting tasks, while exec is only responsible for executing the tasks.
There was a problem hiding this comment.
”exec“ is a ScheduledExecutorService thread pool with no upper thread limit. I also just found it during the discussion of your last proposal patch's review.
There was a problem hiding this comment.
Yes, but the queue of ScheduledExecutorService will always not be full, so only corePoolSize thread will work.
|
|
||
| // Executor to launch child tasks | ||
| private ScheduledThreadPoolExecutor exec; | ||
| private ThreadPoolExecutor exec; |
There was a problem hiding this comment.
exec is only responsible for executing tasks, so ThreadPoolExecutor is currently more suitable.
3 participants
What changes were proposed in this pull request?
Context
If a
BackgroundServicetask takes longer than the interval, the next task will be executed immediately after the previous task completes.For previous discussion #9185
Current PR Fix BackgroundService to make it follow scheduleWithFixedDelay mantics.
Implementation:
Moving
future.join();fromBackgroundService#PeriodicalTask#runto after task submission, and usingfuture.join();to wait for all submitted tasks to complete, effectively transformsPeriodicalTask#runinto a blocking function. This enables it to execute according to thescheduleWithFixedDelaypattern.What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-14236
How was this patch tested?
unit test.