You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "xichen01 (via GitHub)" <gi...@apache.org> on 2026/01/06 17:46:33 UTC
[PR] HDDS-14236. Fix BackgroundService to follow scheduleWithFixedDelay mantics [ozone]
xichen01 opened a new pull request, #9595:
URL: https://github.com/apache/ozone/pull/9595
## What changes were proposed in this pull request?
### Context
If a `BackgroundService` task takes longer than the interval, the next task will be executed immediately after the previous task completes.
For previous discussion https://github.com/apache/ozone/pull/9185
Current PR Fix BackgroundService to make it follow scheduleWithFixedDelay mantics.
### Implementation:
Moving `future.join();` from `BackgroundService#PeriodicalTask#run` to after task submission, and using `future.join();` to wait for all submitted tasks to complete, effectively transforms `PeriodicalTask#run` into a blocking function. This enables it to execute according to the `scheduleWithFixedDelay` pattern.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-14236
## How was this patch tested?
unit test.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-14236. Fix BackgroundService to follow scheduleWithFixedDelay semantics [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #9595:
URL: https://github.com/apache/ozone/pull/9595#discussion_r2684663311
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/BackgroundService.java:
##########
@@ -118,7 +120,7 @@ public synchronized void start() {
}
LOG.info("Starting service {} with interval {} {}", serviceName,
interval, unit.name().toLowerCase());
- exec.scheduleWithFixedDelay(service, 0, interval, unit);
+ scheduler.scheduleWithFixedDelay(service, 0, interval, unit);
Review 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.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-14236. Fix BackgroundService to follow scheduleWithFixedDelay semantics [ozone]
Posted by "xichen01 (via GitHub)" <gi...@apache.org>.
xichen01 commented on code in PR #9595:
URL: https://github.com/apache/ozone/pull/9595#discussion_r2682859611
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/BackgroundService.java:
##########
@@ -118,7 +120,7 @@ public synchronized void start() {
}
LOG.info("Starting service {} with interval {} {}", serviceName,
interval, unit.name().toLowerCase());
- exec.scheduleWithFixedDelay(service, 0, interval, unit);
+ scheduler.scheduleWithFixedDelay(service, 0, interval, unit);
Review 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.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-14236. Fix BackgroundService to follow scheduleWithFixedDelay semantics [ozone]
Posted by "xichen01 (via GitHub)" <gi...@apache.org>.
xichen01 commented on code in PR #9595:
URL: https://github.com/apache/ozone/pull/9595#discussion_r2685082619
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/BackgroundService.java:
##########
@@ -118,7 +120,7 @@ public synchronized void start() {
}
LOG.info("Starting service {} with interval {} {}", serviceName,
interval, unit.name().toLowerCase());
- exec.scheduleWithFixedDelay(service, 0, interval, unit);
+ scheduler.scheduleWithFixedDelay(service, 0, interval, unit);
Review Comment:
Yes, but the queue of `ScheduledExecutorService ` will always not be full, so only `corePoolSize` thread will work.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-14236. Fix BackgroundService to follow scheduleWithFixedDelay semantics [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9595:
URL: https://github.com/apache/ozone/pull/9595#issuecomment-4008615542
This PR has been marked as stale due to 21 days of inactivity. Please comment or remove the stale label to keep it open. Otherwise, it will be automatically closed in 7 days.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-14236. Fix BackgroundService to follow scheduleWithFixedDelay semantics [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #9595: HDDS-14236. Fix BackgroundService to follow scheduleWithFixedDelay semantics
URL: https://github.com/apache/ozone/pull/9595
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-14236. Fix BackgroundService to follow scheduleWithFixedDelay semantics [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9595:
URL: https://github.com/apache/ozone/pull/9595#issuecomment-4058839123
Thank you for your contribution. This PR is being closed due to inactivity. Please contact a maintainer if you would like to reopen it.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-14236. Fix BackgroundService to follow scheduleWithFixedDelay semantics [ozone]
Posted by "xichen01 (via GitHub)" <gi...@apache.org>.
xichen01 commented on code in PR #9595:
URL: https://github.com/apache/ozone/pull/9595#discussion_r2685085830
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/BackgroundService.java:
##########
@@ -40,8 +41,9 @@ public abstract class BackgroundService {
protected static final Logger LOG =
LoggerFactory.getLogger(BackgroundService.class);
- // Executor to launch child tasks
- private ScheduledThreadPoolExecutor exec;
+ private ThreadPoolExecutor exec;
Review Comment:
`exec` is only responsible for executing tasks, so `ThreadPoolExecutor` is currently more suitable.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-14236. Fix BackgroundService to follow scheduleWithFixedDelay semantics [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9595:
URL: https://github.com/apache/ozone/pull/9595#issuecomment-3874576685
This PR has been marked as stale due to 21 days of inactivity. Please comment or remove the stale label to keep it open. Otherwise, it will be automatically closed in 7 days.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-14236. Fix BackgroundService to follow scheduleWithFixedDelay semantics [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #9595:
URL: https://github.com/apache/ozone/pull/9595#discussion_r2670762631
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/BackgroundService.java:
##########
@@ -118,7 +120,7 @@ public synchronized void start() {
}
LOG.info("Starting service {} with interval {} {}", serviceName,
interval, unit.name().toLowerCase());
- exec.scheduleWithFixedDelay(service, 0, interval, unit);
+ scheduler.scheduleWithFixedDelay(service, 0, interval, unit);
Review Comment:
@xichen01 , thanks for continue this improvement task.
Can we keep using the exec here? What's the extra benefit of introducing scheduler?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-14236. Fix BackgroundService to follow scheduleWithFixedDelay semantics [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #9595:
URL: https://github.com/apache/ozone/pull/9595#discussion_r2703715302
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/BackgroundService.java:
##########
@@ -118,7 +120,7 @@ public synchronized void start() {
}
LOG.info("Starting service {} with interval {} {}", serviceName,
interval, unit.name().toLowerCase());
- exec.scheduleWithFixedDelay(service, 0, interval, unit);
+ scheduler.scheduleWithFixedDelay(service, 0, interval, unit);
Review Comment:
PeriodicalTask#Run(), it will schedule new task for each BackgroundTask, executed with "exec".
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org