Repository navigation
YARN-12002. Stop the Router delegation token secret manager on service stop. - #8808
Merged
Merged
Conversation
…e stop. RouterClientRMService#serviceStart calls routerDTSecretManager.startThreads(), which spawns AbstractDelegationTokenSecretManager's ExpiredTokenRemover daemon. serviceStop never called stopThreads(), so that thread outlived the service and kept reaching the FederationStateStoreFacade to roll master keys and remove expired tokens - after the Router, and everything it owns, had stopped. RMSecretManagerService#serviceStop already does this for the ResourceManager's equivalent; the Router simply omitted it. serviceStop also no longer throws NullPointerException when the service never started: userPipelineMap is created in serviceStart, so clear() is now guarded like the secret manager. TestRouterClientRMService covers both: stopping a started service stops the secret manager's threads, and a service that was only initialized stops cleanly. Split out of YARN-11984, whose federation mock Router closes the state store once the Router has stopped and so relies on this thread having stopped. Contains content generated by Claude Code. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
3 of 6 tasks
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
slfan1989
approved these changes
Oct 10, 2026
slfan1989
left a comment
slfan1989
left a comment
Contributor
There was a problem hiding this comment.
LGTM.
The shutdown sequence looks correct, and the null guard handles stopping a service that was initialized but never started. The comment also accurately describes the limitation regarding in-flight RPC handlers.
Build and Yetus are green.
This PR is ready to merge from my side.
Contributor
|
@joseluisll Thanks for the contribution! Merged into trunk. |
3 of 6 tasks
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.
Description of PR
JIRA: YARN-12002
RouterClientRMService#serviceStartcallsrouterDTSecretManager.startThreads(), which starts theExpiredTokenRemoverdaemon thread ofAbstractDelegationTokenSecretManager.serviceStopnever calledstopThreads(), so the thread outlived the service. It kept calling theFederationStateStoreFacadeto roll master keys and remove expired tokens after the Router and everything it owns had stopped.RMSecretManagerService#serviceStopalready does this for the ResourceManager; the Router left it out.serviceStopalso threw aNullPointerExceptionwhen the service was initialized but never started, becauseuserPipelineMapis only created inserviceStart.Changes:
RouterClientRMService#serviceStop: after the RPC server stops, callrouterDTSecretManager.stopThreads()if the secret manager is not null.RouterClientRMService#serviceStop: guarduserPipelineMap.clear()against null.TestRouterClientRMService: two new tests,testServiceStopStopsDelegationTokenSecretManagerandtestServiceStopWithoutStart.This was split out of YARN-11984 (#8681), whose federation mock Router closes the state store after the Router stops and so depends on this thread having stopped. This PR is independent of the other PRs split out of YARN-11984 and can be reviewed and merged on its own.
How was this patch tested?
Ran
TestRouterClientRMServiceon Linux with JDK 21: 6/6 tests pass.Reverted the fix in
RouterClientRMServiceand kept the new tests. Both new tests fail:testServiceStopStopsDelegationTokenSecretManager:expected: <false> but was: <true>. The secret manager is still running afterstop().testServiceStopWithoutStart:NullPointerExceptiononuserPipelineMap.clear().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?Object storage, new dependencies and LICENSE/NOTICE do not apply: this patch touches only the YARN Router and adds no dependencies.
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.