You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "smengcl (via GitHub)" <gi...@apache.org> on 2026/10/12 04:30:53 UTC
[PR] HDDS-16926. Cancel pending health check on NodeStateManager close [ozone]
smengcl opened a new pull request, #11480:
URL: https://github.com/apache/ozone/pull/11480
Generated-by: Claude Code (Opus 5.5)
## What changes were proposed in this pull request?
`NodeStateManager` schedules its health check as a one-shot task `ozone.scm.heartbeat.thread.interval` (3s by default) in the future, and reschedules it after each run. `close()` calls `executorService.shutdown()`, which still lets already scheduled delayed tasks run, and then `awaitTermination`. As a result `close()` blocks until the pending health check fires, up to 3s. This delays every SCM stop, and every test that closes a real `SCMNodeManager` pays the wait once per test.
This PR cancels the pending health check in `close()` before shutting down the executor.
Test class time in CI, four recent master runs compared to a run of this fix on my fork:
| Test class | master (s) | with fix (s) |
|---|---|---|
| `TestDeletedBlockLog` | 99 to 100 | 16.0 |
| `TestSCMNodeManager` | 118 to 119 | 54.5 |
| `TestSCMBlockProtocolServer` | 52 | 9.2 |
| `TestIncrementalContainerReportHandler` | 38 | 3.2 |
| `TestReconNodeManager` | 30 | 3.2 |
| `TestDeadNodeHandler` | 15 | 5.2 |
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-16926
## How was this patch tested?
- Added `TestNodeStateManager#testCloseDoesNotWaitForPendingHealthCheck`. It fails without the fix (`close()` takes 3s).
- CI on my fork for the fix without the new test (timings above): https://github.com/smengcl/hadoop-ozone/actions/runs/38185568206
- CI on my fork for this branch: https://github.com/smengcl/hadoop-ozone/actions/runs/38204072348
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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-16926. Cancel pending health check on NodeStateManager close [ozone]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #11480:
URL: https://github.com/apache/ozone/pull/11480#discussion_r4244710917
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeStateManager.java:
##########
@@ -885,6 +885,10 @@ private void fireHealthStateEvent(HddsProtos.NodeState health,
@Override
public void close() {
+ // Cancel the pending delayed health check, otherwise awaitTermination waits for it to fire.
+ if (healthCheckFuture != null) {
+ healthCheckFuture.cancel(false);
Review Comment:
`cancel(false)` does not stop a health check that has already started. If `run()` reaches `scheduleNextHealthCheck()` between this cancellation and `shutdown()`, it can enqueue a new delayed future; the executor's default shutdown policy still executes delayed tasks already queued, so `awaitTermination()` can again block for the full heartbeat interval. Please make cancellation/shutdown atomic with rescheduling (or otherwise disable execution of delayed tasks on shutdown) so close is bounded even when shutdown races with a health check.
--
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-16926. Cancel pending health check on NodeStateManager close [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on PR #11480:
URL: https://github.com/apache/ozone/pull/11480#issuecomment-6219581132
Merged. Thanks @smengcl
--
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-16926. Cancel pending health check on NodeStateManager close [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang merged PR #11480:
URL: https://github.com/apache/ozone/pull/11480
--
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-16926. Cancel pending health check on NodeStateManager close [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #11480:
URL: https://github.com/apache/ozone/pull/11480#discussion_r4244799063
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeStateManager.java:
##########
@@ -885,6 +885,10 @@ private void fireHealthStateEvent(HddsProtos.NodeState health,
@Override
public void close() {
+ // Cancel the pending delayed health check, otherwise awaitTermination waits for it to fire.
+ if (healthCheckFuture != null) {
+ healthCheckFuture.cancel(false);
Review Comment:
Fixed in f3052fcb0c0: the executor now has `setExecuteExistingDelayedTasksAfterShutdownPolicy(false)`, so `shutdown()` itself discards the delayed health check, including one rescheduled concurrently, and the separate `cancel` in `close()` is gone.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
--
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-16926. Cancel pending health check on NodeStateManager close [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on PR #11480:
URL: https://github.com/apache/ozone/pull/11480#issuecomment-6224728386
Thanks @jojochuang for the 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