joseluisll commented on code in PR #8681:
URL: https://github.com/apache/hadoop/pull/8681#discussion_r3890290306
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/subcluster/TestMockRouter.java:
##########
@@ -73,18 +77,40 @@ public static void main(String[] args) throws YarnException
{
conf.set(YarnConfiguration.ROUTER_WEBAPP_ADDRESS,
getHostNameAndPort(pRouterWebAddressPort));
- RetryPolicy retryPolicy =
FederationStateStoreFacade.createRetryPolicy(conf);
-
+ // This class is launched as its own JVM by JavaProcess, and the parent
test
+ // terminates it with Process#destroy: SIGTERM on Linux and macOS, which
runs
+ // shutdown hooks. (On Windows destroy maps to TerminateProcess, which runs
+ // none, so there this is inert.) Router#main registers this hook itself;
+ // starting the Router directly, as here, skips it, so without this the JVM
+ // exits with the Router's services still running and none of them ever
+ // stopped.
+ ShutdownHookManager.get().addShutdownHook(
+ new CompositeServiceShutdownHook(router),
Router.SHUTDOWN_HOOK_PRIORITY);
router.init(conf);
router.start();
- FederationStateStore stateStore = (FederationStateStore)
- FederationStateStoreFacade.createRetryInstance(conf,
- YarnConfiguration.FEDERATION_STATESTORE_CLIENT_CLASS,
- YarnConfiguration.DEFAULT_FEDERATION_STATESTORE_CLIENT_CLASS,
- FederationStateStore.class, retryPolicy);
- stateStore.init(conf);
- FederationStateStoreFacade.getInstance().reinitialize(stateStore, conf);
+ // Starting the Router has already created and initialized the facade's
+ // store: RouterClientRMService#serviceStart builds a
+ // RouterDelegationTokenSecretManager whose constructor calls
+ // FederationStateStoreFacade#getInstance(Configuration). Building a second
+ // store here and handing it to reinitialize() would orphan that first one
-
+ // reinitialize swaps the reference, it does not close what it replaces -
so
+ // take the store the facade is already using.
+ FederationStateStore stateStore =
+ FederationStateStoreFacade.getInstance(conf).getStateStore();
+
+ // The facade holds this store but never closes it, so its ZooKeeper
Review Comment:
Thanks, you were right. Fixed in production code as you suggested.
8d426cf — RouterClientRMService#serviceStop now calls
routerDTSecretManager.stopThreads(). RMSecretManagerService has always done
this
for the RM; the Router just omitted it. Fixing it there instead of in the
mock
means the shutdown ordering is genuinely guaranteed, not assumed.
0ef4a45 — Same problem, second thread: Router#serviceStop never shut down the
scheduled executor running SubClusterCleaner. That one is enabled by default
and
hits the state store every 60s, so it was more likely than the token remover
to
be mid-call when the store closed. Now drained on stop.
TestYarnFederationWithCapacityScheduler: 38/38.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
