From bc1b76d040f6cd6609189081f423b130c3ec4b30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jose=20Luis=20L=C3=B3pez?= Date: Thu, 13 Aug 2026 15:22:28 +0200 Subject: [PATCH 1/3] YARN-11984. Federation mock sub-cluster and router do not stop cleanly on SIGTERM. TestMockSubCluster and TestMockRouter are not tests - they have no @Test methods. TestFederationSubCluster launches them as separate JVMs through JavaProcess and stops them with Process#destroy: SIGTERM on Linux and macOS, which runs shutdown hooks. Neither registered one, so the JVM died with its ResourceManager, NodeManagers and Router services still running: no service stop, and the ZooKeeper session was left to expire on its own. On Windows destroy maps to TerminateProcess, which runs no hooks, so nothing changes there. Both now register a CompositeServiceShutdownHook before init/start, so a failure part-way through startup still tears down whatever came up. Router#main already does exactly this; TestMockRouter starts the Router directly and so skipped it. Each uses the shutdown hook priority of the daemon it actually runs - ResourceManager's for the sub-cluster, Router's for the router - which every YARN daemon defines as 30. TestMockRouter additionally closes the ZooKeeper-backed FederationStateStore, which the facade never closes; that hook runs at a lower priority than the Router's - ShutdownHookManager runs the highest priority first - so the Router is fully stopped before its store goes away. It closes the store the facade already holds rather than building a second one: RouterClientRMService#serviceStart creates a RouterDelegationTokenSecretManager whose constructor initializes the facade, and reinitialize() swaps the reference without closing what it replaces, so a second store would orphan the first. The Router process opens one ZooKeeper session and closes it, rather than opening two and closing one. The sub-cluster hook gets an explicit 60s budget. Its three NodeManagers each spend a hardcoded 10s in DeletionService#serviceStop, so the teardown takes 33s and ShutdownHookManager's 30s default abandoned it part-way through on every run. Zeroing the NodeManager container grace period, which these long-lived mock clusters never need, trims a further 6s per NodeManager. Not a mini-cluster leak (see YARN-11983): these clusters are meant to outlive the method that starts them. The defect was unclean termination. Test-scope only; no production code is touched. Verified on Linux with TestYarnFederationWithCapacityScheduler: 38/38 pass, all three child JVMs run their hooks to completion and close their ZooKeeper session, and no hook hits its timeout. The suite costs 49s on trunk, 116s with the hooks. Contains content generated by Claude Code. Generated-by: Claude Code (Opus 5) Co-Authored-By: Claude Opus 5 --- .../router/subcluster/TestMockRouter.java | 48 ++++++++++++++----- .../router/subcluster/TestMockSubCluster.java | 29 +++++++++++ 2 files changed, 66 insertions(+), 11 deletions(-) diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/subcluster/TestMockRouter.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/subcluster/TestMockRouter.java index 751a3fed6487d2..8f4eca0533ebc1 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/subcluster/TestMockRouter.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/subcluster/TestMockRouter.java @@ -19,7 +19,8 @@ import org.apache.commons.lang3.ArrayUtils; import org.apache.hadoop.fs.CommonConfigurationKeys; -import org.apache.hadoop.io.retry.RetryPolicy; +import org.apache.hadoop.service.CompositeService.CompositeServiceShutdownHook; +import org.apache.hadoop.util.ShutdownHookManager; import org.apache.hadoop.yarn.conf.YarnConfiguration; import org.apache.hadoop.yarn.exceptions.YarnException; import org.apache.hadoop.yarn.server.MiniYARNCluster; @@ -30,7 +31,10 @@ import org.slf4j.LoggerFactory; /** - * Tests {@link Router}. + * Process entry point that starts a {@link Router} for the federation + * end-to-end suites. This is not a test: it declares no test methods. + * {@link TestFederationSubCluster} launches it as a separate JVM and stops it + * with Process#destroy. */ public class TestMockRouter { @@ -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 + // connection outlives the process unless we close it ourselves. Registered + // at a lower priority than the Router hook above: ShutdownHookManager runs + // the highest priority first, so the Router is fully stopped before the + // store it may still be using goes away. + ShutdownHookManager.get().addShutdownHook(() -> { + try { + stateStore.close(); + } catch (Exception e) { + LOG.warn("Error closing FederationStateStore.", e); + } + }, Router.SHUTDOWN_HOOK_PRIORITY - 5); } private static String getHostNameAndPort(int port) { diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/subcluster/TestMockSubCluster.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/subcluster/TestMockSubCluster.java index d3ab90f3105e78..8a154ee83a54d5 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/subcluster/TestMockSubCluster.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/subcluster/TestMockSubCluster.java @@ -20,20 +20,33 @@ import org.apache.commons.lang3.ArrayUtils; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.CommonConfigurationKeys; +import org.apache.hadoop.service.CompositeService.CompositeServiceShutdownHook; import org.apache.hadoop.test.GenericTestUtils; +import org.apache.hadoop.util.ShutdownHookManager; import org.apache.hadoop.util.Time; import org.apache.hadoop.yarn.conf.YarnConfiguration; import org.apache.hadoop.yarn.server.MiniYARNCluster; +import org.apache.hadoop.yarn.server.resourcemanager.ResourceManager; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.io.File; +import java.util.concurrent.TimeUnit; import static org.apache.hadoop.yarn.server.router.subcluster.TestFederationSubCluster.ZK_FEDERATION_STATESTORE; public class TestMockSubCluster { private static final Logger LOG = LoggerFactory.getLogger(TestMockSubCluster.class); + + /** + * Shutdown budget for the whole sub-cluster. Each NodeManager spends a + * hardcoded 10s in DeletionService#serviceStop on top of its container grace + * period, so three of them do not fit ShutdownHookManager's 30s default and + * the hook would be abandoned part-way through the teardown. + */ + private static final long SHUTDOWN_TIMEOUT_SECONDS = 60; + private Configuration conf; private String subClusterId; @@ -51,6 +64,16 @@ private static String getHostNameAndPort(int port) { public void startYarnSubCluster() { MiniYARNCluster yrCluster = new MiniYARNCluster(subClusterId, 3, 1, 1, false); + // 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.) MiniYARNCluster registers no hook of its + // own, so without this the JVM exits with its ResourceManager and + // NodeManagers still running and no service ever stopped. Registered before + // init/start so a failure part-way through startup still tears down + // whatever came up. + ShutdownHookManager.get().addShutdownHook(new CompositeServiceShutdownHook(yrCluster), + ResourceManager.SHUTDOWN_HOOK_PRIORITY, SHUTDOWN_TIMEOUT_SECONDS, TimeUnit.SECONDS); yrCluster.init(conf); yrCluster.start(); } @@ -87,6 +110,12 @@ public static void main(String[] args) { getHostNameAndPort(pRmTrackerAddressPort)); conf.set(YarnConfiguration.RM_WEBAPP_ADDRESS, getHostNameAndPort(pRmWebAddressPort)); conf.setBoolean(YarnConfiguration.YARN_MINICLUSTER_FIXED_PORTS, true); + // These sub-clusters keep applications running for the lifetime of the + // suite, so on shutdown every NodeManager would otherwise wait out its full + // container grace period - 250ms + 5000ms + 1s of slop, rounded up to 7s by + // the one-second poll in ContainerManagerImpl - before it stops. + conf.setLong(YarnConfiguration.NM_SLEEP_DELAY_BEFORE_SIGKILL_MS, 0); + conf.setLong(YarnConfiguration.NM_PROCESS_KILL_WAIT_MS, 0); conf.setBoolean(YarnConfiguration.FEDERATION_ENABLED, true); conf.set(YarnConfiguration.FEDERATION_STATESTORE_CLIENT_CLASS, ZK_FEDERATION_STATESTORE); conf.set(CommonConfigurationKeys.ZK_ADDRESS, pZkAddress); From 795f7313e039765bc60a8fc387c08118a034be33 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 17:51:11 +0000 Subject: [PATCH 2/3] YARN-11984. Stop the Router delegation token secret manager on service 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. This is also what makes TestMockRouter's shutdown hook ordering sound. That hook closes the federation state store at a lower priority than the Router's, on the assumption that router.stop() has stopped every user of the store. The token remover was a user it had not stopped, so the store could be closed while that thread was still using it. Comment there updated to record the dependency. Co-Authored-By: Claude Opus 5 --- .../yarn/server/router/clientrm/RouterClientRMService.java | 7 +++++++ .../yarn/server/router/subcluster/TestMockRouter.java | 5 ++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/main/java/org/apache/hadoop/yarn/server/router/clientrm/RouterClientRMService.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/main/java/org/apache/hadoop/yarn/server/router/clientrm/RouterClientRMService.java index 3d45ad207cf403..490b8ff6480f15 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/main/java/org/apache/hadoop/yarn/server/router/clientrm/RouterClientRMService.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/main/java/org/apache/hadoop/yarn/server/router/clientrm/RouterClientRMService.java @@ -202,6 +202,13 @@ protected void serviceStop() throws Exception { if (this.server != null) { this.server.stop(); } + // serviceStart() started the secret manager's ExpiredTokenRemover; stop it + // here, once the server is down and no request can reach it, so it does not + // outlive this service and keep using the federation state store. Mirrors + // RMSecretManagerService#serviceStop. + if (this.routerDTSecretManager != null) { + this.routerDTSecretManager.stopThreads(); + } userPipelineMap.clear(); super.serviceStop(); } diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/subcluster/TestMockRouter.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/subcluster/TestMockRouter.java index 8f4eca0533ebc1..8dca6484a5a57a 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/subcluster/TestMockRouter.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/subcluster/TestMockRouter.java @@ -103,7 +103,10 @@ public static void main(String[] args) throws YarnException { // connection outlives the process unless we close it ourselves. Registered // at a lower priority than the Router hook above: ShutdownHookManager runs // the highest priority first, so the Router is fully stopped before the - // store it may still be using goes away. + // store it may still be using goes away. That ordering is only sound + // because RouterClientRMService#serviceStop stops the delegation token + // secret manager's ExpiredTokenRemover, which would otherwise still be + // reaching the facade after the Router has stopped. ShutdownHookManager.get().addShutdownHook(() -> { try { stateStore.close(); From 9ff0cc64393f336398e6955835d5748953f8f2f0 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 17:57:56 +0000 Subject: [PATCH 3/3] YARN-11984. Shut down the Router scheduled executor on service stop. 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. Same defect as the delegation token secret manager's ExpiredTokenRemover, but far more likely to be caught mid-call: the cleaner touches the store once a minute, where the token remover's scans default to hourly and daily. TestMockRouter's shutdown hook closes the federation state store once the Router has stopped, on the assumption that nothing is using it any more. This is the second user that assumption did not cover. 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. Co-Authored-By: Claude Opus 5 --- .../apache/hadoop/yarn/server/router/Router.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/main/java/org/apache/hadoop/yarn/server/router/Router.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/main/java/org/apache/hadoop/yarn/server/router/Router.java index 8f3c4d0fe577e0..8d66977a4f4de3 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/main/java/org/apache/hadoop/yarn/server/router/Router.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/main/java/org/apache/hadoop/yarn/server/router/Router.java @@ -48,6 +48,7 @@ import org.apache.hadoop.util.StringUtils; import org.apache.hadoop.util.VersionInfo; import org.apache.hadoop.util.GenericOptionsParser; +import org.apache.hadoop.util.concurrent.HadoopExecutors; import org.apache.hadoop.yarn.YarnUncaughtExceptionHandler; import org.apache.hadoop.yarn.api.records.ApplicationId; import org.apache.hadoop.yarn.conf.YarnConfiguration; @@ -124,6 +125,13 @@ public class Router extends CompositeService { private static final String UI2_WEBAPP_NAME = "/ui2"; + /** + * How long serviceStop waits for the scheduled executor to drain before it + * interrupts what is still running. One SubClusterCleaner run is a scan of + * the state store, so this only has to cover a single pass. + */ + private static final long SCHEDULED_EXECUTOR_SHUTDOWN_TIMEOUT_SECONDS = 10; + private ScheduledThreadPoolExecutor scheduledExecutorService; private SubClusterCleaner subClusterCleaner; @@ -196,6 +204,12 @@ protected void serviceStop() throws Exception { if (isStopping.getAndSet(true)) { return; } + // serviceStart() may have scheduled the SubClusterCleaner on this executor. + // Shut it down before the services below, and wait for an in-flight run to + // finish: the cleaner reaches the federation state store on every run, so + // it must not still be running once the Router has stopped. + HadoopExecutors.shutdown(scheduledExecutorService, LOG, + SCHEDULED_EXECUTOR_SHUTDOWN_TIMEOUT_SECONDS, TimeUnit.SECONDS); super.serviceStop(); DefaultMetricsSystem.shutdown(); WebServiceClient.destroy();