Repository navigation
YARN-12003. Shut down the Router scheduled executor on service stop - #8785
Merged
slfan1989 merged 4 commits intoOct 10, 2026
Merged
Conversation
joseluisll
force-pushed
the
YARN-11984-3-router-executor-stop
branch
from
October 3, 2026 08:37
c0ce1a1 to
b516741
Compare
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
3 of 6 tasks
joseluisll
force-pushed
the
YARN-11984-3-router-executor-stop
branch
from
October 6, 2026 12:24
b516741 to
c79b87c
Compare
slfan1989
self-requested a review
October 6, 2026 13:23
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
Router#serviceStart schedules SubClusterCleaner on scheduledExecutorService at a fixed rate, enabled by default (DEFAULT_ROUTER_DEREGISTER_SUBCLUSTER_ENABLED) with a 60s interval. serviceStop never shut that executor down, so the cleaner outlived the Router and kept calling FederationStateStoreFacade#getSubClusters and #deregisterSubCluster against a state store nothing was maintaining. Uses HadoopExecutors#shutdown - graceful shutdown, bounded wait, then force - rather than shutdownNow(), so an in-flight scan finishes before the store it is reading can be closed. It waits up to 5s for the executor to drain, then interrupts it and waits up to 5s more, so this adds at most 10s to stopping the Router. TestRouter covers it through a new @VisibleForTesting getter for the executor: once the Router has stopped, the executor is terminated. Split out of YARN-11984, whose federation mock Router closes the state store once the Router has stopped and so relies on the cleaner having stopped. Contains content generated by Claude Code. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
joseluisll
force-pushed
the
YARN-11984-3-router-executor-stop
branch
from
October 7, 2026 10:52
b94c91e to
7131c20
Compare
|
💔 -1 overall
This message was automatically generated. |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
🎊 +1 overall
This message was automatically generated. |
Contributor
Author
|
@slfan1989 This PR is CI green and Yetus green, and ready to review. |
slfan1989
requested changes
Oct 7, 2026
slfan1989
left a comment
slfan1989
left a comment
Contributor
There was a problem hiding this comment.
LGTM.
One small, non-blocking suggestion: could you reword "it must not still be running once the Router has stopped" to describe the bounded shutdown?
HadoopExecutors.shutdown() may return after the timeout if a task does not respond to interruption.
For example:
// Shut down the executor before stopping child services, allowing an
// in-flight cleaner run to finish within the configured timeout.
3 of 6 tasks
…top. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Author
|
Thanks @slfan1989, reworded in b816940 to describe the bounded, best-effort shutdown. Could you re-review when you get a chance? The review is still marked "changes requested". |
|
🎊 +1 overall
This message was automatically generated. |
slfan1989
approved these changes
Oct 9, 2026
slfan1989
left a comment
slfan1989
left a comment
Contributor
There was a problem hiding this comment.
LGTM.
Thanks for updating the comment to accurately describe the bounded, best-effort shutdown. My previous concern has been addressed.
This PR is ready to merge from my side.
Contributor
|
@joseluisll Thanks for the contribution! Merged into trunk. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Depends on (has to be merged first):
clears a CI failure: it edits TestRouterWebServicesREST.java:719-742, where TestFederationWebApp fails at line 737 in this PR's precommit, still red in the latest run
Description of PR
JIRA: YARN-12003
Router#serviceStartschedulesSubClusterCleaneronscheduledExecutorServiceat a fixed rate. This is on by default (yarn.router.deregister.subcluster.enabled) and runs every 60s.Router#serviceStopnever shuts that executor down. As a result, the cleaner outlives the Router and keeps callingFederationStateStoreFacade#getSubClustersand#deregisterSubClusteron a state store that nothing maintains any more. Its thread is not a daemon thread, so it can also keep the JVM alive. This shows up when a Router is stopped inside a JVM that keeps running: tests, embedded or mock federation setups, or a stop/start in the same process.Changes:
Router#serviceStopnow callsHadoopExecutors#shutdown(scheduledExecutorService, LOG, 5, SECONDS)before stopping the child services. This is a graceful shutdown with a bounded wait. It waits up to 5s for an in-flight cleaner run to finish, then callsshutdownNow()and waits up to 5s more, so stopping the Router takes at most 10s longer. We don't callshutdownNow()straight away, so that a scan already in progress can finish before the state store it is reading is closed.HadoopExecutors#shutdowndoes nothing when the executor is null, so stopping a Router that was never initialised is still safe.@VisibleForTestinggetter,Router#getScheduledExecutorService().TestRouter#testServiceStopShutsDownScheduledExecutor.How was this patch tested?
TestRouteron Linux with JDK 21: 6/6 tests pass, including the newtestServiceStopShutsDownScheduledExecutor.HadoopExecutors.shutdowncall removed fromserviceStop, the new test fails withexpected: <true> but was: <false>. So the test does catch the missing shutdown.For code changes:
declared according to the connector-specific documentation? Note: Automated CI
testing doesn't cover all cases so manual testing with cloud storage is still
required.
LICENSE,LICENSE-binary,NOTICE-binaryfiles?AI Tooling
If an AI tool was used:
where is the name of the AI tool used.
https://www.apache.org/legal/generative-tooling.html
Contains content generated by Claude Code.