From b154129845236e3899a7ba143b8e6704ba47323c Mon Sep 17 00:00:00 2001 From: Aleksandr Savonin Date: Sat, 27 Jun 2026 21:48:25 +0200 Subject: [PATCH 01/22] [FLINK-40019][core][runtime] Support per-job delegation tokens --- .../generated/security_configuration.html | 6 + .../security_delegation_token_section.html | 6 + .../flink/configuration/SecurityOptions.java | 13 +- .../token/DelegationTokenManagerCallback.java | 42 +++ .../token/DelegationTokenProvider.java | 78 ++++ .../flink/runtime/jobmaster/JobMaster.java | 1 + .../resourcemanager/ResourceManager.java | 24 +- .../ResourceManagerGateway.java | 34 ++ .../token/DefaultDelegationTokenManager.java | 310 +++++++++++++--- .../token/DelegationTokenManager.java | 35 ++ .../ResourceManagerJobMasterTest.java | 55 +++ .../TestingResourceManagerService.java | 9 +- .../utils/TestingResourceManagerGateway.java | 2 + .../DefaultDelegationTokenManagerTest.java | 344 ++++++++++++++++++ ...eptionThrowingDelegationTokenProvider.java | 51 +++ 15 files changed, 959 insertions(+), 51 deletions(-) create mode 100644 flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenManagerCallback.java diff --git a/docs/layouts/shortcodes/generated/security_configuration.html b/docs/layouts/shortcodes/generated/security_configuration.html index 757e8ff979dc52..b3793c039f3ac3 100644 --- a/docs/layouts/shortcodes/generated/security_configuration.html +++ b/docs/layouts/shortcodes/generated/security_configuration.html @@ -44,6 +44,12 @@ Double Ratio of the tokens's expiration time when new credentials should be re-obtained. + +
security.delegation.tokens.reobtain.cooldown
+ 30 s + Duration + Minimum time between two consecutive on-demand token re-obtain cycles, such as those triggered when a job is registered. Requests arriving within the cooldown are coalesced and deferred until it elapses. Does not affect the periodic renewal. +
security.kerberos.access.hadoopFileSystems
(none) diff --git a/docs/layouts/shortcodes/generated/security_delegation_token_section.html b/docs/layouts/shortcodes/generated/security_delegation_token_section.html index 0aa8ab815d7b7d..13fc3a58309446 100644 --- a/docs/layouts/shortcodes/generated/security_delegation_token_section.html +++ b/docs/layouts/shortcodes/generated/security_delegation_token_section.html @@ -32,6 +32,12 @@ Double Ratio of the tokens's expiration time when new credentials should be re-obtained. + +
security.delegation.tokens.reobtain.cooldown
+ 30 s + Duration + Minimum time between two consecutive on-demand token re-obtain cycles, such as those triggered when a job is registered. Requests arriving within the cooldown are coalesced and deferred until it elapses. Does not affect the periodic renewal. +
security.delegation.token.provider.<serviceName>.enabled
true diff --git a/flink-core/src/main/java/org/apache/flink/configuration/SecurityOptions.java b/flink-core/src/main/java/org/apache/flink/configuration/SecurityOptions.java index 441b07f569ec1e..6e10b3a1dd02a6 100644 --- a/flink-core/src/main/java/org/apache/flink/configuration/SecurityOptions.java +++ b/flink-core/src/main/java/org/apache/flink/configuration/SecurityOptions.java @@ -180,8 +180,19 @@ public class SecurityOptions { .withDescription( "Ratio of the tokens's expiration time when new credentials should be re-obtained."); - @Documentation.SuffixOption(DELEGATION_TOKEN_PROVIDER_PREFIX) @Documentation.Section(value = Documentation.Sections.SECURITY_DELEGATION_TOKEN, position = 5) + public static final ConfigOption DELEGATION_TOKENS_REOBTAIN_COOLDOWN = + key("security.delegation.tokens.reobtain.cooldown") + .durationType() + .defaultValue(Duration.ofSeconds(30)) + .withDescription( + "Minimum time between two consecutive on-demand token re-obtain " + + "cycles, such as those triggered when a job is registered. " + + "Requests arriving within the cooldown are coalesced and " + + "deferred until it elapses. Does not affect the periodic renewal."); + + @Documentation.SuffixOption(DELEGATION_TOKEN_PROVIDER_PREFIX) + @Documentation.Section(value = Documentation.Sections.SECURITY_DELEGATION_TOKEN, position = 6) public static final ConfigOption DELEGATION_TOKEN_PROVIDER_ENABLED = key("enabled") .booleanType() diff --git a/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenManagerCallback.java b/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenManagerCallback.java new file mode 100644 index 00000000000000..dff36a791465cc --- /dev/null +++ b/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenManagerCallback.java @@ -0,0 +1,42 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.core.security.token; + +import org.apache.flink.annotation.Experimental; + +/** + * Handed to a {@link DelegationTokenProvider} at {@link + * DelegationTokenProvider#init(org.apache.flink.configuration.Configuration, + * DelegationTokenManagerCallback) init} time, giving the provider a way to ask the delegation token + * manager to re-obtain tokens. The provider may retain the callback and invoke it later, outside + * the {@code init}/{@code registerJob} call stack. + */ +@Experimental +public interface DelegationTokenManagerCallback { + + /** + * Requests an asynchronous token re-obtain and redistribution to all receivers, + * bringing the next obtain cycle forward instead of waiting for the periodic renewal. + * + *

May be called from any thread at any time after {@code init}. The manager coalesces + * requests and may apply a cooldown, so a call does not necessarily map to one obtain + * cycle. Returns immediately and does not wait for completion. + */ + void reobtainDelegationTokens(); +} diff --git a/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenProvider.java b/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenProvider.java index 676148564b682d..d6d364771711ad 100644 --- a/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenProvider.java +++ b/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenProvider.java @@ -19,6 +19,7 @@ package org.apache.flink.core.security.token; import org.apache.flink.annotation.Experimental; +import org.apache.flink.api.common.JobID; import org.apache.flink.configuration.Configuration; import java.util.Optional; @@ -28,6 +29,17 @@ * DelegationTokenManager through service loader. Basically the implementation of this interface is * responsible to produce the serialized form of tokens which will be handled by {@link * DelegationTokenReceiver} instances both on JobManager and TaskManager side. + * + *

Threading contract. A single instance per provider implementation is created and + * {@link #init(Configuration, DelegationTokenManagerCallback) initialized} once and then shared + * for the lifetime of the manager. {@link #obtainDelegationTokens()} runs on the manager's IO + * executor, while {@link + * #registerJob(JobID, Configuration)} and {@link #unregisterJob(JobID)} are invoked from the + * ResourceManager main thread; these can therefore run concurrently. {@link + * DelegationTokenManagerCallback#reobtainDelegationTokens()} may be invoked from any thread. + * Implementations must keep any per-job state thread-safe, and {@code registerJob}/{@code + * unregisterJob} must be non-blocking so they do not stall the ResourceManager — defer real work + * to {@link #obtainDelegationTokens()}. */ @Experimental public interface DelegationTokenProvider { @@ -75,6 +87,25 @@ default String serviceConfigPrefix() { */ void init(Configuration configuration) throws Exception; + /** + * Called by DelegationTokenManager to initialize the provider after construction, additionally + * handing it a {@link DelegationTokenManagerCallback} it can use to request a token re-obtain. + * + *

This is the entry point the manager actually calls. The default implementation ignores the + * callback and delegates to {@link #init(Configuration)}, so providers that do not need to + * trigger re-obtains keep implementing only {@link #init(Configuration)}. A provider that wants + * to request re-obtains overrides this method, retains the callback, and invokes {@link + * DelegationTokenManagerCallback#reobtainDelegationTokens()} when needed. + * + * @param configuration Configuration to initialize the provider. + * @param callback Used to ask the manager to re-obtain tokens; may be retained and called + * later. + */ + default void init(Configuration configuration, DelegationTokenManagerCallback callback) + throws Exception { + init(configuration); + } + /** * Return whether delegation tokens are required for this service. * @@ -88,4 +119,51 @@ default String serviceConfigPrefix() { * @return the obtained delegation tokens. */ ObtainedDelegationTokens obtainDelegationTokens() throws Exception; + + /** + * Called when a job has started, before its tasks are scheduled, with its configuration. + * + *

To get the job's tokens distributed without waiting for the periodic renewal, call {@link + * DelegationTokenManagerCallback#reobtainDelegationTokens()} on the callback handed to {@link + * #init(Configuration, DelegationTokenManagerCallback)} to request an immediate obtain cycle. + * + *

A provider that requests a re-obtain must record this job's per-job state before + * invoking {@link DelegationTokenManagerCallback#reobtainDelegationTokens()}. That call merely + * schedules (or coalesces into) an obtain cycle that runs later on another thread; recording + * first establishes the happens-before that lets the serving cycle observe this job's state. + * Recording afterwards races with the cycle and the job's tokens may be skipped until the next + * periodic renewal. + * + *

Must be idempotent: it may be called more than once for the same {@code jobId} (e.g. on + * JobManager or ResourceManager failover, when the JobMaster re-registers). + * + *

Should not throw: a thrown (unchecked) exception rejects the job's registration (the job + * does not start) and triggers {@link #unregisterJob(JobID)} on all providers to roll back. + * Prefer deferring the real fetch to the (retrying) obtain cycle over a synchronous fetch, so a + * transient failure does not fail the job. + * + * @param jobId The job id of the job. + * @param jobConfiguration The job configuration. + */ + default void registerJob(JobID jobId, Configuration jobConfiguration) {} + + /** + * Called when the job is being removed — it reached a globally terminal state, or its + * job-leader registration timed out — and its per-job state should be released. Must be + * idempotent. Exceptions are caught and logged by the framework (one provider's failure does + * not abort cleanup of the others), but implementations should still avoid throwing. + * + * @param jobId The job id of the job. + */ + default void unregisterJob(JobID jobId) {} + + /** + * Stops the provider. Any resources should be closed. + * + *

Called once during manager shutdown. Note that an obtain-and-broadcast cycle started just + * before shutdown may still be running on another thread when this is invoked, so {@code + * stop()} may overlap an in-flight {@link #obtainDelegationTokens()}; implementations must + * release resources in a way that is safe with respect to that overlap. + */ + default void stop() {} } diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/jobmaster/JobMaster.java b/flink-runtime/src/main/java/org/apache/flink/runtime/jobmaster/JobMaster.java index 39a29191b9761b..6c674a7357a196 100644 --- a/flink-runtime/src/main/java/org/apache/flink/runtime/jobmaster/JobMaster.java +++ b/flink-runtime/src/main/java/org/apache/flink/runtime/jobmaster/JobMaster.java @@ -1599,6 +1599,7 @@ protected CompletableFuture invokeRegistration( jobManagerResourceID, jobManagerRpcAddress, jobID, + executionPlan.getJobConfiguration(), timeout); } }; diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/ResourceManager.java b/flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/ResourceManager.java index 5cfb06b3bef50c..db4d6aefbf1ccf 100755 --- a/flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/ResourceManager.java +++ b/flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/ResourceManager.java @@ -22,6 +22,7 @@ import org.apache.flink.api.common.JobID; import org.apache.flink.api.common.JobStatus; import org.apache.flink.api.java.tuple.Tuple2; +import org.apache.flink.configuration.Configuration; import org.apache.flink.configuration.ThreadDumpMode; import org.apache.flink.runtime.blob.TransientBlobKey; import org.apache.flink.runtime.blocklist.BlockedNode; @@ -116,8 +117,8 @@ *

It offers the following methods as part of its rpc interface to interact with him remotely: * *

    - *
  • {@link #registerJobMaster(JobMasterId, ResourceID, String, JobID, Duration)} registers a - * {@link JobMaster} at the resource manager + *
  • {@link #registerJobMaster(JobMasterId, ResourceID, String, JobID, Configuration, Duration)} + * registers a {@link JobMaster} at the resource manager *
*/ public abstract class ResourceManager @@ -367,12 +368,14 @@ public CompletableFuture registerJobMaster( final ResourceID jobManagerResourceId, final String jobManagerAddress, final JobID jobId, + final Configuration jobConfiguration, final Duration timeout) { checkNotNull(jobMasterId); checkNotNull(jobManagerResourceId); checkNotNull(jobManagerAddress); checkNotNull(jobId); + checkNotNull(jobConfiguration); try (MdcCloseable ignored = MdcUtils.withContext(MdcUtils.asContextData(jobId))) { if (!jobLeaderIdService.containsJob(jobId)) { @@ -428,6 +431,14 @@ public CompletableFuture registerJobMaster( jobMasterIdFuture, (JobMasterGateway jobMasterGateway, JobMasterId leadingJobMasterId) -> { if (Objects.equals(leadingJobMasterId, jobMasterId)) { + // Register with the delegation token manager first; a + // provider failure rejects this registration so the job + // never starts without the tokens it requires. + try { + delegationTokenManager.registerJob(jobId, jobConfiguration); + } catch (Exception e) { + return new RegistrationResponse.Failure(e); + } return registerJobMasterInternal( jobMasterGateway, jobId, @@ -1224,6 +1235,15 @@ protected void removeJob(JobID jobId, Exception cause) { if (jobManagerRegistrations.containsKey(jobId)) { closeJobManagerConnection(jobId, ResourceRequirementHandling.CLEAR, cause); } + + try { + delegationTokenManager.unregisterJob(jobId); + } catch (Exception e) { + log.warn( + "Could not properly remove the job {} from the delegation token manager.", + jobId, + e); + } } protected void jobLeaderLostLeadership(JobID jobId, JobMasterId oldJobMasterId) { diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/ResourceManagerGateway.java b/flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/ResourceManagerGateway.java index 9c722604ebc74e..73f6563192eaca 100644 --- a/flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/ResourceManagerGateway.java +++ b/flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/ResourceManagerGateway.java @@ -21,6 +21,7 @@ import org.apache.flink.api.common.JobID; import org.apache.flink.api.common.JobStatus; import org.apache.flink.api.java.tuple.Tuple2; +import org.apache.flink.configuration.Configuration; import org.apache.flink.configuration.ThreadDumpMode; import org.apache.flink.runtime.blob.BlobServer; import org.apache.flink.runtime.blob.TransientBlobKey; @@ -64,10 +65,42 @@ public interface ResourceManagerGateway /** * Register a {@link JobMaster} at the resource manager. * + *

Backward-compatible overload that registers without a job configuration. Equivalent to + * calling {@link #registerJobMaster(JobMasterId, ResourceID, String, JobID, Configuration, + * Duration)} with an empty configuration. + * + * @param jobMasterId The fencing token for the JobMaster leader + * @param jobMasterResourceId The resource ID of the JobMaster that registers + * @param jobMasterAddress The address of the JobMaster that registers + * @param jobId The Job ID of the JobMaster that registers + * @param timeout Timeout for the future to complete + * @return Future registration response + */ + default CompletableFuture registerJobMaster( + JobMasterId jobMasterId, + ResourceID jobMasterResourceId, + String jobMasterAddress, + JobID jobId, + @RpcTimeout Duration timeout) { + return registerJobMaster( + jobMasterId, + jobMasterResourceId, + jobMasterAddress, + jobId, + new Configuration(), + timeout); + } + + /** + * Register a {@link JobMaster} at the resource manager, supplying the job's {@link + * Configuration} so implementations can perform per-job initialization (e.g. obtaining + * job-scoped delegation tokens). + * * @param jobMasterId The fencing token for the JobMaster leader * @param jobMasterResourceId The resource ID of the JobMaster that registers * @param jobMasterAddress The address of the JobMaster that registers * @param jobId The Job ID of the JobMaster that registers + * @param jobConfiguration The job's configuration, used for per-job initialization * @param timeout Timeout for the future to complete * @return Future registration response */ @@ -76,6 +109,7 @@ CompletableFuture registerJobMaster( ResourceID jobMasterResourceId, String jobMasterAddress, JobID jobId, + Configuration jobConfiguration, @RpcTimeout Duration timeout); /** diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java index d1abfefd8669de..b9f19cb9a8727a 100644 --- a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java +++ b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java @@ -20,9 +20,11 @@ import org.apache.flink.annotation.Internal; import org.apache.flink.annotation.VisibleForTesting; +import org.apache.flink.api.common.JobID; import org.apache.flink.configuration.Configuration; import org.apache.flink.configuration.SecurityOptions; import org.apache.flink.core.plugin.PluginManager; +import org.apache.flink.core.security.token.DelegationTokenManagerCallback; import org.apache.flink.core.security.token.DelegationTokenProvider; import org.apache.flink.core.security.token.DelegationTokenReceiver; import org.apache.flink.util.FlinkRuntimeException; @@ -45,6 +47,7 @@ import java.util.ServiceLoader; import java.util.Set; import java.util.concurrent.ExecutorService; +import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.TimeUnit; @@ -54,6 +57,7 @@ import static org.apache.flink.configuration.SecurityOptions.DELEGATION_TOKENS_RENEWAL_RETRY_INITIAL_BACKOFF; import static org.apache.flink.configuration.SecurityOptions.DELEGATION_TOKENS_RENEWAL_RETRY_MAX_BACKOFF; import static org.apache.flink.configuration.SecurityOptions.DELEGATION_TOKENS_RENEWAL_TIME_RATIO; +import static org.apache.flink.configuration.SecurityOptions.DELEGATION_TOKENS_REOBTAIN_COOLDOWN; import static org.apache.flink.configuration.SecurityOptions.DELEGATION_TOKEN_PROVIDER_ENABLED; import static org.apache.flink.util.Preconditions.checkNotNull; import static org.apache.flink.util.Preconditions.checkState; @@ -78,6 +82,8 @@ public class DefaultDelegationTokenManager implements DelegationTokenManager { private static final Logger LOG = LoggerFactory.getLogger(DefaultDelegationTokenManager.class); + private static final long NO_PREVIOUS_REOBTAIN = Long.MIN_VALUE; + private final Configuration configuration; @Nullable private final PluginManager pluginManager; @@ -92,6 +98,18 @@ public class DefaultDelegationTokenManager implements DelegationTokenManager { @VisibleForTesting long lastKnownNextRenewal = Long.MAX_VALUE; + private final long reobtainCooldownMillis; + + /** Clock used for cooldown bookkeeping; overridable in tests. */ + private volatile Clock clock = Clock.systemDefaultZone(); + + /** + * Serializes the obtain-and-broadcast cycle so that, even though {@code cancel(true)} does not + * wait for an in-flight cycle and the IO executor is multi-threaded, two cycles can never run + * concurrently and broadcast tokens out of order. + */ + private final Object obtainLock = new Object(); + @VisibleForTesting final Map delegationTokenProviders; private final DelegationTokenReceiverRepository delegationTokenReceiverRepository; @@ -106,6 +124,34 @@ public class DefaultDelegationTokenManager implements DelegationTokenManager { @Nullable private ScheduledFuture tokensUpdateFuture; + /** + * Clock time (millis) at which {@link #tokensUpdateFuture} is scheduled to fire, or {@link + * Long#MAX_VALUE} when no cycle is pending. Lets an on-demand re-obtain only ever bring the + * next obtain cycle forward and never push an already-scheduled (e.g. periodic) + * renewal later, which could otherwise let a short-lived token expire before it is renewed. + */ + @GuardedBy("tokensUpdateFutureLock") + private long nextScheduledAtMillis = Long.MAX_VALUE; + + /** Whether an on-demand re-obtain is scheduled but has not started executing yet (dedupe). */ + @GuardedBy("tokensUpdateFutureLock") + private boolean reobtainScheduled; + + /** + * Clock time (millis) of the last on-demand re-obtain request, used to enforce the cooldown; + * {@link #NO_PREVIOUS_REOBTAIN} until the first request. Only on-demand re-obtains update this + * (not the periodic renewal), so the cooldown spaces requests, not obtain executions. + */ + @GuardedBy("tokensUpdateFutureLock") + private long lastReobtainAtMillis = NO_PREVIOUS_REOBTAIN; + + /** + * Whether {@link #stop()} has been called. Reset by {@link #start(Listener)}. Prevents a late + * provider callback or an in-flight obtain cycle from scheduling new work after shutdown. + */ + @GuardedBy("tokensUpdateFutureLock") + private boolean stopped; + @Nullable private Listener listener; public DefaultDelegationTokenManager( @@ -121,6 +167,8 @@ public DefaultDelegationTokenManager( this.renewalRetryMaxBackoff = configuration.get(DELEGATION_TOKENS_RENEWAL_RETRY_MAX_BACKOFF).toMillis(); this.currentRetryBackoff = renewalRetryInitialBackoff; + this.reobtainCooldownMillis = + configuration.get(DELEGATION_TOKENS_REOBTAIN_COOLDOWN).toMillis(); this.delegationTokenProviders = loadProviders(); this.delegationTokenReceiverRepository = new DelegationTokenReceiverRepository(configuration, pluginManager); @@ -139,12 +187,15 @@ public DefaultDelegationTokenManager( private Map loadProviders() { LOG.info("Loading delegation token providers"); + // Handed to every provider so it can request an immediate re-obtain later, from any + // thread, decoupled from the registerJob call stack. + final DelegationTokenManagerCallback callback = this::reobtainDelegationTokens; Map providers = new HashMap<>(); Consumer loadProvider = (provider) -> { try { if (isProviderEnabled(configuration, provider.serviceName())) { - provider.init(configuration); + provider.init(configuration, callback); LOG.info( "Delegation token provider {} loaded and initialized", provider.serviceName()); @@ -310,6 +361,7 @@ public void start(Listener listener) throws Exception { this.listener = checkNotNull(listener, "Listener must not be null"); synchronized (tokensUpdateFutureLock) { checkState(tokensUpdateFuture == null, "Manager is already started"); + stopped = false; } startTokensUpdate(); @@ -317,57 +369,119 @@ public void start(Listener listener) throws Exception { @VisibleForTesting void startTokensUpdate() { - try { - LOG.info("Starting tokens update task"); - DelegationTokenContainer container = new DelegationTokenContainer(); - Optional nextRenewal = obtainDelegationTokensAndGetNextRenewal(container); - - if (container.hasTokens()) { - delegationTokenReceiverRepository.onNewTokensObtained(container); - - LOG.info("Notifying listener about new tokens"); - checkNotNull(listener, "Listener must not be null"); - listener.onNewTokensObtained(InstantiationUtil.serializeObject(container)); - LOG.info("Listener notified successfully"); - } else { - LOG.warn("No tokens obtained so skipping notifications"); + synchronized (tokensUpdateFutureLock) { + // The obtain cycle is starting: clear the dedupe flag so later on-demand requests can + // schedule a fresh cycle. + reobtainScheduled = false; + // If stop() ran before this cycle (already handed to the IO executor) began, skip the + // obtain/broadcast: the providers may already be stopped. Safe via this lock's + // happens-before with stop(). The dedupe flag is cleared above, so it is never stuck. + if (stopped) { + return; } + } + // Serialize the obtain-and-broadcast so a re-obtain racing the periodic renewal cannot run + // two cycles concurrently on the (multi-threaded) IO executor and broadcast out of order. + synchronized (obtainLock) { + try { + LOG.info("Starting tokens update task"); + DelegationTokenContainer container = new DelegationTokenContainer(); + Optional nextRenewal = obtainDelegationTokensAndGetNextRenewal(container); + + if (container.hasTokens()) { + delegationTokenReceiverRepository.onNewTokensObtained(container); + + LOG.info("Notifying listener about new tokens"); + checkNotNull(listener, "Listener must not be null"); + listener.onNewTokensObtained(InstantiationUtil.serializeObject(container)); + LOG.info("Listener notified successfully"); + } else { + LOG.warn("No tokens obtained so skipping notifications"); + } - if (nextRenewal.isPresent()) { - lastKnownNextRenewal = nextRenewal.get(); - currentRetryBackoff = renewalRetryInitialBackoff; - long renewalDelay = - calculateRenewalDelay(Clock.systemDefaultZone(), nextRenewal.get()); - synchronized (tokensUpdateFutureLock) { - tokensUpdateFuture = - scheduledExecutor.schedule( - () -> ioExecutor.execute(this::startTokensUpdate), - renewalDelay, - TimeUnit.MILLISECONDS); + if (nextRenewal.isPresent()) { + lastKnownNextRenewal = nextRenewal.get(); + currentRetryBackoff = renewalRetryInitialBackoff; + long renewalDelay = calculateRenewalDelay(clock, nextRenewal.get()); + maybeScheduleRenewal(renewalDelay); + LOG.info( + "Tokens update task started with {} delay", + TimeUtils.formatWithHighestUnit(Duration.ofMillis(renewalDelay))); + } else { + LOG.warn( + "Tokens update task not started because either no tokens obtained or none of the tokens specified its renewal date"); } - LOG.info( - "Tokens update task started with {} delay", - TimeUtils.formatWithHighestUnit(Duration.ofMillis(renewalDelay))); - } else { + } catch (InterruptedException e) { + // Ignore, may happen if shutting down. + LOG.debug("Interrupted", e); + } catch (Exception e) { + long delay = calculateRetryDelay(clock); + maybeScheduleRenewal(delay); LOG.warn( - "Tokens update task not started because either no tokens obtained or none of the tokens specified its renewal date"); + "Failed to update tokens, will try again in {}", + TimeUtils.formatWithHighestUnit(Duration.ofMillis(delay)), + e); } - } catch (InterruptedException e) { - // Ignore, may happen if shutting down. - LOG.debug("Interrupted", e); - } catch (Exception e) { - long delay = calculateRetryDelay(Clock.systemDefaultZone()); - synchronized (tokensUpdateFutureLock) { - tokensUpdateFuture = - scheduledExecutor.schedule( - () -> ioExecutor.execute(this::startTokensUpdate), - delay, - TimeUnit.MILLISECONDS); + } + } + + /** + * Schedules a one-shot token-obtain-and-broadcast cycle after {@code delayMs}, replacing any + * pending renewal; a delay of {@code 0} brings the next cycle forward to now. Must only be + * called after {@link #start(Listener)} (the scheduled and IO executors are non-null then) and + * while holding {@link #tokensUpdateFutureLock}. + */ + @GuardedBy("tokensUpdateFutureLock") + private void scheduleRenewalLocked(long delayMs) { + stopTokensUpdate(); + nextScheduledAtMillis = clock.millis() + delayMs; + try { + tokensUpdateFuture = + scheduledExecutor.schedule( + () -> { + try { + ioExecutor.execute(this::startTokensUpdate); + } catch (RejectedExecutionException e) { + // IO executor is shutting down: drop the cycle but release the + // dedupe flag so it cannot get stuck if the manager is reused. + synchronized (tokensUpdateFutureLock) { + reobtainScheduled = false; + } + LOG.debug("Tokens update task rejected by IO executor", e); + } + }, + delayMs, + TimeUnit.MILLISECONDS); + } catch (RejectedExecutionException e) { + // Scheduled executor is shutting down: no cycle will run, so undo the bookkeeping this + // method set. Clearing reobtainScheduled keeps a coalesced re-obtain from getting stuck; + // nextScheduledAtMillis returns to the no-cycle-pending marker (stopTokensUpdate() above + // already nulled tokensUpdateFuture). + reobtainScheduled = false; + nextScheduledAtMillis = Long.MAX_VALUE; + LOG.debug("Tokens update task rejected by scheduled executor", e); + } + } + + /** + * Schedules the next periodic renewal at the end of a completed obtain cycle, unless the + * manager was stopped or an on-demand re-obtain was scheduled while this cycle ran. A pending + * on-demand cycle already re-establishes the renewal schedule, so it is left in place rather + * than cancelled: the periodic renewal is folded into it, never dropped. + */ + @VisibleForTesting + void maybeScheduleRenewal(long delayMs) { + synchronized (tokensUpdateFutureLock) { + if (stopped) { + return; + } + if (reobtainScheduled) { + LOG.debug( + "An on-demand re-obtain is already scheduled; leaving it in place instead " + + "of overwriting it with the periodic renewal."); + return; } - LOG.warn( - "Failed to update tokens, will try again in {}", - TimeUtils.formatWithHighestUnit(Duration.ofMillis(delay)), - e); + scheduleRenewalLocked(delayMs); } } @@ -377,6 +491,7 @@ void stopTokensUpdate() { if (tokensUpdateFuture != null) { tokensUpdateFuture.cancel(true); tokensUpdateFuture = null; + nextScheduledAtMillis = Long.MAX_VALUE; } } } @@ -416,13 +531,114 @@ long calculateRenewalDelay(Clock clock, long nextRenewal) { return renewalDelay; } + @VisibleForTesting + void setClock(Clock clock) { + this.clock = clock; + } + /** Stops re-occurring token obtain task. */ @Override public void stop() { LOG.info("Stopping credential renewal"); - stopTokensUpdate(); + synchronized (tokensUpdateFutureLock) { + // Mark stopped, cancel the pending cycle, and reset on-demand re-obtain bookkeeping + // atomically, so a concurrent reobtainDelegationTokens() cannot leave a live future + // orphaned after stop and a later start() does not inherit stale state. + stopped = true; + stopTokensUpdate(); + reobtainScheduled = false; + lastReobtainAtMillis = NO_PREVIOUS_REOBTAIN; + } + + for (DelegationTokenProvider provider : delegationTokenProviders.values()) { + try { + provider.stop(); + } catch (Throwable t) { + LOG.error("Failed to stop delegation token provider {}", provider.serviceName(), t); + } + } LOG.info("Stopped credential renewal"); } + + @Override + public void reobtainDelegationTokens() { + synchronized (tokensUpdateFutureLock) { + if (scheduledExecutor == null || ioExecutor == null) { + LOG.debug( + "A re-obtain of delegation tokens was requested but the manager was " + + "constructed without executors (one-shot obtain path); the " + + "request is ignored."); + return; + } + if (stopped) { + LOG.debug( + "A re-obtain of delegation tokens was requested after the manager was " + + "stopped; the request is ignored."); + return; + } + // Dedupe: if an on-demand re-obtain is already scheduled and has not started yet, the + // newly registered job(s) will be covered by it, so coalesce this request into it. + if (reobtainScheduled) { + LOG.debug("A re-obtain of delegation tokens is already scheduled; coalescing."); + return; + } + // Cooldown: bound how often on-demand re-obtains can run by deferring this cycle until + // at least reobtainCooldownMillis have passed since the previous on-demand re-obtain. + long now = clock.millis(); + long delayMillis = + lastReobtainAtMillis == NO_PREVIOUS_REOBTAIN + ? 0L + : Math.max(0L, lastReobtainAtMillis + reobtainCooldownMillis - now); + // Only bring the next cycle forward: if a cycle (e.g. the periodic renewal) is still + // pending and scheduled to fire sooner than the cooldown-deferred time, fire at that + // earlier time instead of pushing it later — otherwise a short-lived token could expire + // before it is renewed. The nextScheduledAtMillis > now guard skips an already-fired + // future that has not yet been re-armed, so this never bypasses the cooldown. + if (tokensUpdateFuture != null + && nextScheduledAtMillis > now + && nextScheduledAtMillis - now < delayMillis) { + delayMillis = nextScheduledAtMillis - now; + } + lastReobtainAtMillis = now; + reobtainScheduled = true; + LOG.debug( + "Re-obtain of delegation tokens requested; scheduling an obtain cycle in {}", + TimeUtils.formatWithHighestUnit(Duration.ofMillis(delayMillis))); + scheduleRenewalLocked(delayMillis); + } + } + + @Override + public void registerJob(JobID jobId, Configuration jobConfiguration) throws Exception { + try { + for (DelegationTokenProvider provider : delegationTokenProviders.values()) { + provider.registerJob(jobId, jobConfiguration); + } + } catch (Exception e) { + // If any of the providers fail to register, then unregister the job from them all. + // unregisterJob is idempotent, so it is safe to call it for providers that were never + // (or only partially) registered for this job before the failure. The rollback must + // never mask the original failure, so swallow any rollback exception. + try { + unregisterJob(jobId); + } catch (Exception rollbackException) { + LOG.error("Failed to roll back registration of job {}", jobId, rollbackException); + } + LOG.error("Failed to register job {}", jobId, e); + throw e; + } + } + + @Override + public void unregisterJob(JobID jobId) throws Exception { + for (DelegationTokenProvider provider : delegationTokenProviders.values()) { + try { + provider.unregisterJob(jobId); + } catch (Exception e) { + LOG.error("Failed to unregister job for provider {}", provider.serviceName(), e); + } + } + } } diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DelegationTokenManager.java b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DelegationTokenManager.java index 3452cf5dcce0ab..ccdeb36d0d1ee0 100644 --- a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DelegationTokenManager.java +++ b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DelegationTokenManager.java @@ -19,6 +19,8 @@ package org.apache.flink.runtime.security.token; import org.apache.flink.annotation.Internal; +import org.apache.flink.api.common.JobID; +import org.apache.flink.configuration.Configuration; /** * Manager for delegation tokens in a Flink cluster. @@ -61,4 +63,37 @@ interface Listener { /** Stops re-occurring token obtain task. */ void stop(); + + /** + * Requests an immediate, asynchronous token-obtain-and-distribute cycle, bringing the next + * cycle forward instead of waiting for the periodic renewal. May be called from any thread; + * it is a no-op on a manager constructed without executors (the one-shot obtain path). + * Concurrent requests are coalesced and a configurable cooldown may apply, so a call does + * not necessarily map to exactly one obtain. + * + *

Backs {@link + * org.apache.flink.core.security.token.DelegationTokenManagerCallback#reobtainDelegationTokens()}. + */ + default void reobtainDelegationTokens() {} + + /** + * Called when a job has started. Fans the event out to all loaded {@link + * org.apache.flink.core.security.token.DelegationTokenProvider}s. On failure the job is + * unregistered from all providers and the exception is rethrown so the caller can reject the + * job's registration. A provider that needs the new job's tokens distributed immediately + * requests it via {@link + * org.apache.flink.core.security.token.DelegationTokenManagerCallback#reobtainDelegationTokens()}. + * + * @param jobId The job id which just started. + * @param jobConfiguration The job's configuration. + */ + default void registerJob(JobID jobId, Configuration jobConfiguration) throws Exception {} + + /** + * Called when a job is being removed. Fans the event out to all loaded providers. Must be + * idempotent. + * + * @param jobId The job id of the job. + */ + default void unregisterJob(JobID jobId) throws Exception {} } diff --git a/flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/ResourceManagerJobMasterTest.java b/flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/ResourceManagerJobMasterTest.java index 180b1af58441b4..8830bdea6396e6 100644 --- a/flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/ResourceManagerJobMasterTest.java +++ b/flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/ResourceManagerJobMasterTest.java @@ -19,6 +19,7 @@ package org.apache.flink.runtime.resourcemanager; import org.apache.flink.api.common.JobID; +import org.apache.flink.configuration.Configuration; import org.apache.flink.runtime.clusterframework.types.ResourceID; import org.apache.flink.runtime.highavailability.HighAvailabilityServices; import org.apache.flink.runtime.jobmaster.JobMaster; @@ -34,6 +35,8 @@ import org.apache.flink.runtime.rpc.RpcUtils; import org.apache.flink.runtime.rpc.TestingRpcService; import org.apache.flink.runtime.rpc.exceptions.FencingTokenException; +import org.apache.flink.runtime.security.token.DelegationTokenManager; +import org.apache.flink.runtime.security.token.NoOpDelegationTokenManager; import org.apache.flink.runtime.taskexecutor.TaskExecutorGateway; import org.apache.flink.runtime.taskexecutor.TestingTaskExecutorGatewayBuilder; import org.apache.flink.util.FlinkRuntimeException; @@ -93,10 +96,16 @@ private void createAndRegisterJobMasterGateway() { } private void createAndStartResourceManagerService() throws Exception { + createAndStartResourceManagerService(new NoOpDelegationTokenManager()); + } + + private void createAndStartResourceManagerService(DelegationTokenManager delegationTokenManager) + throws Exception { final TestingLeaderElection leaderElection = new TestingLeaderElection(); resourceManagerService = TestingResourceManagerService.newBuilder() .setRpcService(rpcService) + .setDelegationTokenManager(delegationTokenManager) .setJmLeaderRetrieverFunction( requestedJobId -> { if (requestedJobId.equals(jobId)) { @@ -152,6 +161,37 @@ void testRegisterJobMaster() { .isInstanceOf(JobMasterRegistrationSuccess.class); } + /** + * FLIP-588: if the delegation token manager rejects the job (its {@code registerJob} throws), + * the ResourceManager must reject the JobMaster registration so the job does not start without + * the tokens it requires. This also exercises the widened (6-arg) {@code registerJobMaster} RPC + * that carries the job {@link Configuration}. + */ + @Test + void testRegisterJobMasterRejectedWhenDelegationTokenRegistrationFails() throws Exception { + // Rebuild the RM service with a delegation token manager that rejects registerJob. + resourceManagerService.rethrowFatalErrorIfAny(); + resourceManagerService.cleanUp(); + final FlinkRuntimeException failure = + new FlinkRuntimeException("registerJob rejected by provider"); + createAndStartResourceManagerService(new RejectingDelegationTokenManager(failure)); + + final CompletableFuture registrationFuture = + resourceManagerGateway.registerJobMaster( + jobMasterGateway.getFencingToken(), + jobMasterResourceId, + jobMasterGateway.getAddress(), + jobId, + new Configuration(), + TIMEOUT); + + final RegistrationResponse response = + registrationFuture.get(TIMEOUT.toMillis(), TimeUnit.MILLISECONDS); + assertThat(response).isInstanceOf(RegistrationResponse.Failure.class); + assertThat(((RegistrationResponse.Failure) response).getReason().getMessage()) + .contains("registerJob rejected by provider"); + } + @Test void testDisconnectTaskManagerInResourceManager() throws ExecutionException, InterruptedException, TimeoutException { @@ -295,4 +335,19 @@ void testRegisterJobMasterWithFailureLeaderListener() { // ignore the reported error resourceManagerService.ignoreFatalErrors(); } + + /** A {@link DelegationTokenManager} whose {@code registerJob} always throws. */ + private static final class RejectingDelegationTokenManager extends NoOpDelegationTokenManager { + + private final Exception failure; + + private RejectingDelegationTokenManager(Exception failure) { + this.failure = failure; + } + + @Override + public void registerJob(JobID jobId, Configuration jobConfiguration) throws Exception { + throw failure; + } + } } diff --git a/flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/TestingResourceManagerService.java b/flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/TestingResourceManagerService.java index e4e094d8da4ee0..7510f0122e44f6 100644 --- a/flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/TestingResourceManagerService.java +++ b/flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/TestingResourceManagerService.java @@ -33,6 +33,7 @@ import org.apache.flink.runtime.rpc.RpcEndpoint; import org.apache.flink.runtime.rpc.RpcService; import org.apache.flink.runtime.rpc.TestingRpcService; +import org.apache.flink.runtime.security.token.DelegationTokenManager; import org.apache.flink.runtime.security.token.NoOpDelegationTokenManager; import org.apache.flink.runtime.util.TestingFatalErrorHandler; import org.apache.flink.util.concurrent.FutureUtils; @@ -149,6 +150,7 @@ public static class Builder { private boolean needStopRpcService = true; private TestingLeaderElection rmLeaderElection = null; private Function jmLeaderRetrieverFunction = null; + private DelegationTokenManager delegationTokenManager = new NoOpDelegationTokenManager(); public Builder setRpcService(RpcService rpcService) { this.rpcService = checkNotNull(rpcService); @@ -167,6 +169,11 @@ public Builder setJmLeaderRetrieverFunction( return this; } + public Builder setDelegationTokenManager(DelegationTokenManager delegationTokenManager) { + this.delegationTokenManager = checkNotNull(delegationTokenManager); + return this; + } + public TestingResourceManagerService build() throws Exception { rpcService = rpcService != null ? rpcService : new TestingRpcService(); rmLeaderElection = @@ -189,7 +196,7 @@ public TestingResourceManagerService build() throws Exception { rpcService, haServices, new TestingHeartbeatServices(), - new NoOpDelegationTokenManager(), + delegationTokenManager, fatalErrorHandler, new ClusterInformation("localhost", 1234), null, diff --git a/flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/utils/TestingResourceManagerGateway.java b/flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/utils/TestingResourceManagerGateway.java index bd38d99eb7f55f..8d01ce72d3aeb3 100644 --- a/flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/utils/TestingResourceManagerGateway.java +++ b/flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/utils/TestingResourceManagerGateway.java @@ -22,6 +22,7 @@ import org.apache.flink.api.common.JobStatus; import org.apache.flink.api.java.tuple.Tuple2; import org.apache.flink.api.java.tuple.Tuple3; +import org.apache.flink.configuration.Configuration; import org.apache.flink.configuration.ThreadDumpMode; import org.apache.flink.runtime.blob.TransientBlobKey; import org.apache.flink.runtime.blocklist.BlockedNode; @@ -275,6 +276,7 @@ public CompletableFuture registerJobMaster( ResourceID jobMasterResourceId, String jobMasterAddress, JobID jobId, + Configuration jobConfiguration, Duration timeout) { final QuadFunction< JobMasterId, diff --git a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java index 5f78c79a89db8c..0e46beeb06e3ff 100644 --- a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java +++ b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java @@ -18,6 +18,7 @@ package org.apache.flink.runtime.security.token; +import org.apache.flink.api.common.JobID; import org.apache.flink.configuration.Configuration; import org.apache.flink.core.security.token.DelegationTokenProvider; import org.apache.flink.core.security.token.DelegationTokenReceiver; @@ -31,11 +32,22 @@ import java.time.Clock; import java.time.Duration; import java.time.ZoneId; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Map; +import java.util.Optional; import java.util.Set; +import java.util.concurrent.BrokenBarrierException; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.CyclicBarrier; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import static java.time.Instant.ofEpochMilli; @@ -43,7 +55,9 @@ import static org.apache.flink.configuration.SecurityOptions.DELEGATION_TOKENS_RENEWAL_RETRY_INITIAL_BACKOFF; import static org.apache.flink.configuration.SecurityOptions.DELEGATION_TOKENS_RENEWAL_RETRY_MAX_BACKOFF; import static org.apache.flink.configuration.SecurityOptions.DELEGATION_TOKENS_RENEWAL_TIME_RATIO; +import static org.apache.flink.configuration.SecurityOptions.DELEGATION_TOKENS_REOBTAIN_COOLDOWN; import static org.apache.flink.core.security.token.DelegationTokenProvider.CONFIG_PREFIX; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -300,4 +314,334 @@ public void calculateRetryDelayShouldCapToTtlBound() { assertTrue(delay <= Duration.ofSeconds(10).toMillis()); assertTrue(delay >= 0); } + + @Test + public void registerJobShouldTriggerImmediateRenewalAndTrackJob() throws Exception { + final ManuallyTriggeredScheduledExecutor scheduledExecutor = + new ManuallyTriggeredScheduledExecutor(); + final ManuallyTriggeredScheduledExecutorService scheduler = + new ManuallyTriggeredScheduledExecutorService(); + + Configuration configuration = new Configuration(); + configuration.set(getBooleanConfigOption(CONFIG_PREFIX + ".throw.enabled"), true); + AtomicInteger startTokensUpdateCallCount = new AtomicInteger(0); + DefaultDelegationTokenManager delegationTokenManager = + new DefaultDelegationTokenManager( + configuration, null, scheduledExecutor, scheduler) { + @Override + void startTokensUpdate() { + startTokensUpdateCallCount.incrementAndGet(); + super.startTokensUpdate(); + } + }; + // Ask the provider to request an immediate refresh when the job is registered. + ExceptionThrowingDelegationTokenProvider.shouldReobtainOnRegister.set(true); + + JobID jobId = JobID.generate(); + delegationTokenManager.registerJob(jobId, new Configuration()); + scheduledExecutor.triggerScheduledTasks(); + scheduler.triggerAll(); + + assertEquals(1, startTokensUpdateCallCount.get()); + assertEquals(1, ExceptionThrowingDelegationTokenProvider.registeredJobs.get().size()); + + delegationTokenManager.unregisterJob(jobId); + assertEquals(0, ExceptionThrowingDelegationTokenProvider.registeredJobs.get().size()); + } + + @Test + public void stopShouldStopProviders() { + Configuration configuration = new Configuration(); + DefaultDelegationTokenManager delegationTokenManager = + new DefaultDelegationTokenManager(configuration, null, null, null); + + delegationTokenManager.stop(); + + assertTrue(ExceptionThrowingDelegationTokenProvider.stopped.get()); + } + + @Test + public void registerJobShouldRollBackAndRethrowWhenProviderThrows() throws Exception { + Configuration configuration = new Configuration(); + DefaultDelegationTokenManager delegationTokenManager = + new DefaultDelegationTokenManager(configuration, null, null, null); + + JobID jobId = JobID.generate(); + delegationTokenManager.registerJob(jobId, new Configuration()); + assertEquals(1, ExceptionThrowingDelegationTokenProvider.registeredJobs.get().size()); + + // A provider that throws during registration must cause the job to be unregistered from + // all providers and the exception to be rethrown. + ExceptionThrowingDelegationTokenProvider.throwInRegister.set(true); + assertThrows( + IllegalArgumentException.class, + () -> delegationTokenManager.registerJob(jobId, new Configuration())); + assertEquals(0, ExceptionThrowingDelegationTokenProvider.registeredJobs.get().size()); + } + + @Test + public void unregisterJobShouldSwallowProviderFailure() throws Exception { + Configuration configuration = new Configuration(); + DefaultDelegationTokenManager delegationTokenManager = + new DefaultDelegationTokenManager(configuration, null, null, null); + + JobID jobId = JobID.generate(); + delegationTokenManager.registerJob(jobId, new Configuration()); + + // A provider that throws during unregistration must not prevent cleanup from completing. + ExceptionThrowingDelegationTokenProvider.throwInUnregister.set(true); + assertDoesNotThrow(() -> delegationTokenManager.unregisterJob(jobId)); + } + + @Test + public void reobtainShouldCoalesceConcurrentRequests() { + final ManuallyTriggeredScheduledExecutor scheduledExecutor = + new ManuallyTriggeredScheduledExecutor(); + final ManuallyTriggeredScheduledExecutorService scheduler = + new ManuallyTriggeredScheduledExecutorService(); + + AtomicInteger startTokensUpdateCallCount = new AtomicInteger(0); + DefaultDelegationTokenManager delegationTokenManager = + new DefaultDelegationTokenManager( + new Configuration(), null, scheduledExecutor, scheduler) { + @Override + void startTokensUpdate() { + startTokensUpdateCallCount.incrementAndGet(); + super.startTokensUpdate(); + } + }; + + // Two requests before the cycle runs must be coalesced into a single scheduled obtain. + delegationTokenManager.reobtainDelegationTokens(); + delegationTokenManager.reobtainDelegationTokens(); + assertEquals(1, scheduledExecutor.getActiveScheduledTasks().size()); + // The second request must be a true no-op: it must not cancel and reschedule a new future + // (which would also leave a single *active* task). getAllScheduledTasks() includes + // cancelled futures, so it stays 1 only if the second request was genuinely coalesced. + assertEquals(1, scheduledExecutor.getAllScheduledTasks().size()); + + scheduledExecutor.triggerScheduledTasks(); + scheduler.triggerAll(); + assertEquals(1, startTokensUpdateCallCount.get()); + } + + @Test + public void periodicRenewalMustNotCancelPendingOnDemandReobtain() { + final ManuallyTriggeredScheduledExecutor scheduledExecutor = + new ManuallyTriggeredScheduledExecutor(); + final ManuallyTriggeredScheduledExecutorService scheduler = + new ManuallyTriggeredScheduledExecutorService(); + + DefaultDelegationTokenManager delegationTokenManager = + new DefaultDelegationTokenManager( + new Configuration(), null, scheduledExecutor, scheduler); + + // An on-demand re-obtain is scheduled (e.g. a freshly registered job). + delegationTokenManager.reobtainDelegationTokens(); + assertEquals(1, scheduledExecutor.getAllScheduledTasks().size()); + assertEquals(0L, onlyScheduledDelayMillis(scheduledExecutor)); + + // A periodic obtain cycle that was already running completes and tries to install its own + // renewal. It must NOT cancel the pending on-demand re-obtain (regression test for the + // lost-reobtain race that also latched the dedupe flag). + delegationTokenManager.maybeScheduleRenewal(999_999L); + + // No cancel+reschedule happened (still a single schedule call) and the pending future is + // still the immediate on-demand one, not the 999_999ms periodic renewal. + assertEquals(1, scheduledExecutor.getAllScheduledTasks().size()); + assertEquals(0L, onlyScheduledDelayMillis(scheduledExecutor)); + + // Once the on-demand cycle has run and cleared the dedupe flag, a periodic renewal can be + // scheduled normally again. + scheduledExecutor.triggerScheduledTasks(); + scheduler.triggerAll(); + delegationTokenManager.maybeScheduleRenewal(123L); + assertEquals(123L, onlyScheduledDelayMillis(scheduledExecutor)); + } + + @Test + public void reobtainShouldRunImmediatelyAfterCooldownWindowElapses() { + final ManuallyTriggeredScheduledExecutor scheduledExecutor = + new ManuallyTriggeredScheduledExecutor(); + final ManuallyTriggeredScheduledExecutorService scheduler = + new ManuallyTriggeredScheduledExecutorService(); + + Configuration configuration = hermeticCooldownConfig(Duration.ofMillis(60_000)); + DefaultDelegationTokenManager delegationTokenManager = + new DefaultDelegationTokenManager( + configuration, null, scheduledExecutor, scheduler); + + long t0 = 1_000_000L; + delegationTokenManager.setClock(Clock.fixed(ofEpochMilli(t0), ZoneId.systemDefault())); + delegationTokenManager.reobtainDelegationTokens(); + assertEquals(0L, onlyScheduledDelayMillis(scheduledExecutor)); + scheduledExecutor.triggerScheduledTasks(); + scheduler.triggerAll(); + + // A request arriving after the full cooldown window has elapsed runs immediately again. + delegationTokenManager.setClock( + Clock.fixed(ofEpochMilli(t0 + 70_000L), ZoneId.systemDefault())); + delegationTokenManager.reobtainDelegationTokens(); + assertEquals(0L, onlyScheduledDelayMillis(scheduledExecutor)); + } + + @Test + public void stopShouldResetCooldownForSubsequentStart() throws Exception { + final ManuallyTriggeredScheduledExecutor scheduledExecutor = + new ManuallyTriggeredScheduledExecutor(); + final ManuallyTriggeredScheduledExecutorService scheduler = + new ManuallyTriggeredScheduledExecutorService(); + + Configuration configuration = hermeticCooldownConfig(Duration.ofMillis(60_000)); + DefaultDelegationTokenManager delegationTokenManager = + new DefaultDelegationTokenManager( + configuration, null, scheduledExecutor, scheduler); + + long t0 = 1_000_000L; + delegationTokenManager.setClock(Clock.fixed(ofEpochMilli(t0), ZoneId.systemDefault())); + delegationTokenManager.reobtainDelegationTokens(); + assertEquals(0L, onlyScheduledDelayMillis(scheduledExecutor)); + scheduledExecutor.triggerScheduledTasks(); + scheduler.triggerAll(); + + // 10s later a re-obtain is deferred by the cooldown. + delegationTokenManager.setClock( + Clock.fixed(ofEpochMilli(t0 + 10_000L), ZoneId.systemDefault())); + delegationTokenManager.reobtainDelegationTokens(); + assertEquals(50_000L, onlyScheduledDelayMillis(scheduledExecutor)); + + // stop() clears the cooldown anchor (and the dedupe/stopped state). After a restart, the + // next re-obtain runs immediately instead of inheriting the stale cooldown. + delegationTokenManager.stop(); + delegationTokenManager.start(tokens -> {}); + delegationTokenManager.setClock( + Clock.fixed(ofEpochMilli(t0 + 15_000L), ZoneId.systemDefault())); + delegationTokenManager.reobtainDelegationTokens(); + assertEquals(0L, onlyScheduledDelayMillis(scheduledExecutor)); + } + + @Test + public void reobtainShouldRespectCooldown() { + final ManuallyTriggeredScheduledExecutor scheduledExecutor = + new ManuallyTriggeredScheduledExecutor(); + final ManuallyTriggeredScheduledExecutorService scheduler = + new ManuallyTriggeredScheduledExecutorService(); + + Configuration configuration = hermeticCooldownConfig(Duration.ofMillis(60_000)); + DefaultDelegationTokenManager delegationTokenManager = + new DefaultDelegationTokenManager( + configuration, null, scheduledExecutor, scheduler); + + long t0 = 1_000_000L; + delegationTokenManager.setClock(Clock.fixed(ofEpochMilli(t0), ZoneId.systemDefault())); + + // First re-obtain after a quiet period runs immediately (no cooldown applies). + delegationTokenManager.reobtainDelegationTokens(); + assertEquals(0L, onlyScheduledDelayMillis(scheduledExecutor)); + scheduledExecutor.triggerScheduledTasks(); + scheduler.triggerAll(); + + // A second re-obtain 10s later must be deferred until the 60s cooldown elapses. + delegationTokenManager.setClock( + Clock.fixed(ofEpochMilli(t0 + 10_000L), ZoneId.systemDefault())); + delegationTokenManager.reobtainDelegationTokens(); + assertEquals(50_000L, onlyScheduledDelayMillis(scheduledExecutor)); + } + + @Test + public void reobtainShouldBeIgnoredWhenNotStarted() { + // Constructed with null executors -> not started; a re-obtain request must be a safe no-op. + DefaultDelegationTokenManager delegationTokenManager = + new DefaultDelegationTokenManager(new Configuration(), null, null, null); + + assertDoesNotThrow(delegationTokenManager::reobtainDelegationTokens); + } + + @Test + public void registerJobShouldBeIdempotent() throws Exception { + DefaultDelegationTokenManager delegationTokenManager = + new DefaultDelegationTokenManager(new Configuration(), null, null, null); + + // Re-registering the same job (e.g. on JobManager/ResourceManager failover) must not + // accumulate duplicate per-job state in the providers. + JobID jobId = JobID.generate(); + delegationTokenManager.registerJob(jobId, new Configuration()); + delegationTokenManager.registerJob(jobId, new Configuration()); + + assertEquals(1, ExceptionThrowingDelegationTokenProvider.registeredJobs.get().size()); + } + + @Test + public void obtainLockSerializesConcurrentObtainCycles() throws Exception { + final ManuallyTriggeredScheduledExecutor scheduledExecutor = + new ManuallyTriggeredScheduledExecutor(); + final ExecutorService ioExecutor = Executors.newFixedThreadPool(2); + try { + // The barrier trips only if two obtain cycles are inside the obtain/broadcast + // section at the same time. obtainLock must serialize them, so each should time out. + final CyclicBarrier barrier = new CyclicBarrier(2); + final AtomicBoolean concurrentObtainDetected = new AtomicBoolean(false); + final CountDownLatch done = new CountDownLatch(2); + + DefaultDelegationTokenManager delegationTokenManager = + new DefaultDelegationTokenManager( + new Configuration(), null, scheduledExecutor, ioExecutor) { + @Override + protected Optional obtainDelegationTokensAndGetNextRenewal( + DelegationTokenContainer container) { + try { + barrier.await(200, TimeUnit.MILLISECONDS); + // Reached only if both cycles met here concurrently. + concurrentObtainDetected.set(true); + } catch (TimeoutException | BrokenBarrierException serialized) { + // Expected: the other cycle never entered within the window. + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + return Optional.empty(); + } + }; + + ioExecutor.execute( + () -> { + delegationTokenManager.startTokensUpdate(); + done.countDown(); + }); + ioExecutor.execute( + () -> { + delegationTokenManager.startTokensUpdate(); + done.countDown(); + }); + + assertTrue(done.await(10, TimeUnit.SECONDS)); + assertFalse( + concurrentObtainDetected.get(), + "obtainLock must prevent two obtain cycles from running concurrently"); + } finally { + ioExecutor.shutdownNow(); + } + } + + /** + * Configuration for cooldown-scheduling tests: sets the cooldown and disables all providers + * that could fail the obtain cycle (hadoopfs/hbase need a real Hadoop setup; the throw + * provider fails on demand). A failed cycle schedules a jittered retry, and the bring-forward + * clamp would coalesce the on-demand request into that retry instead of deferring by the + * cooldown, making delay assertions nondeterministic. + */ + private static Configuration hermeticCooldownConfig(Duration cooldown) { + Configuration configuration = new Configuration(); + configuration.set(DELEGATION_TOKENS_REOBTAIN_COOLDOWN, cooldown); + configuration.set(getBooleanConfigOption(CONFIG_PREFIX + ".throw.enabled"), false); + configuration.set(getBooleanConfigOption(CONFIG_PREFIX + ".hadoopfs.enabled"), false); + configuration.set(getBooleanConfigOption(CONFIG_PREFIX + ".hbase.enabled"), false); + return configuration; + } + + private static long onlyScheduledDelayMillis( + ManuallyTriggeredScheduledExecutor scheduledExecutor) { + Collection> tasks = scheduledExecutor.getActiveScheduledTasks(); + assertEquals(1, tasks.size()); + return tasks.iterator().next().getDelay(TimeUnit.MILLISECONDS); + } } diff --git a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/ExceptionThrowingDelegationTokenProvider.java b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/ExceptionThrowingDelegationTokenProvider.java index 3a4300423c00c3..292a716bb2ee91 100644 --- a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/ExceptionThrowingDelegationTokenProvider.java +++ b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/ExceptionThrowingDelegationTokenProvider.java @@ -18,10 +18,14 @@ package org.apache.flink.runtime.security.token; +import org.apache.flink.api.common.JobID; import org.apache.flink.configuration.Configuration; +import org.apache.flink.core.security.token.DelegationTokenManagerCallback; import org.apache.flink.core.security.token.DelegationTokenProvider; +import java.util.HashSet; import java.util.Optional; +import java.util.Set; /** * An example implementation of {@link DelegationTokenProvider} which throws exception when enabled. @@ -36,14 +40,31 @@ public class ExceptionThrowingDelegationTokenProvider implements DelegationToken ThreadLocal.withInitial(() -> Boolean.FALSE); public static volatile ThreadLocal constructed = ThreadLocal.withInitial(() -> Boolean.FALSE); + public static volatile ThreadLocal shouldReobtainOnRegister = + ThreadLocal.withInitial(() -> Boolean.FALSE); + public static volatile ThreadLocal throwInRegister = + ThreadLocal.withInitial(() -> Boolean.FALSE); + public static volatile ThreadLocal throwInUnregister = + ThreadLocal.withInitial(() -> Boolean.FALSE); + public static volatile ThreadLocal stopped = + ThreadLocal.withInitial(() -> Boolean.FALSE); + public static volatile ThreadLocal> registeredJobs = + ThreadLocal.withInitial(HashSet::new); public static void reset() { throwInInit.set(false); throwInUsage.set(false); addToken.set(false); constructed.set(false); + shouldReobtainOnRegister.set(false); + throwInRegister.set(false); + throwInUnregister.set(false); + stopped.set(false); + registeredJobs.get().clear(); } + private DelegationTokenManagerCallback callback; + public ExceptionThrowingDelegationTokenProvider() { constructed.set(true); } @@ -60,6 +81,12 @@ public void init(Configuration configuration) { } } + @Override + public void init(Configuration configuration, DelegationTokenManagerCallback callback) { + this.callback = callback; + init(configuration); + } + @Override public boolean delegationTokensRequired() { if (throwInUsage.get()) { @@ -79,4 +106,28 @@ public ObtainedDelegationTokens obtainDelegationTokens() { return null; } } + + @Override + public void registerJob(JobID jobId, Configuration jobConfiguration) { + if (throwInRegister.get()) { + throw new IllegalArgumentException(); + } + registeredJobs.get().add(jobId); + if (shouldReobtainOnRegister.get()) { + callback.reobtainDelegationTokens(); + } + } + + @Override + public void unregisterJob(JobID jobId) { + if (throwInUnregister.get()) { + throw new IllegalArgumentException(); + } + registeredJobs.get().remove(jobId); + } + + @Override + public void stop() { + stopped.set(true); + } } From 6f3db83720769b262ac64cc37f2c214e27853248 Mon Sep 17 00:00:00 2001 From: Aleksandr Savonin Date: Tue, 7 Jul 2026 21:34:16 +0200 Subject: [PATCH 02/22] [FLINK-40019][core][runtime] Polish comments and apply spotless formatting --- .../token/DelegationTokenManagerCallback.java | 8 ++++---- .../token/DelegationTokenProvider.java | 19 +++++++++---------- .../resourcemanager/ResourceManager.java | 2 +- .../token/DefaultDelegationTokenManager.java | 18 +++++++++--------- .../token/DelegationTokenManager.java | 8 ++++---- .../DefaultDelegationTokenManagerTest.java | 5 +++-- 6 files changed, 30 insertions(+), 30 deletions(-) diff --git a/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenManagerCallback.java b/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenManagerCallback.java index dff36a791465cc..f1f11b1c771d33 100644 --- a/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenManagerCallback.java +++ b/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenManagerCallback.java @@ -31,12 +31,12 @@ public interface DelegationTokenManagerCallback { /** - * Requests an asynchronous token re-obtain and redistribution to all receivers, - * bringing the next obtain cycle forward instead of waiting for the periodic renewal. + * Requests an asynchronous token re-obtain and redistribution to all receivers, bringing the + * next obtain cycle forward instead of waiting for the periodic renewal. * *

May be called from any thread at any time after {@code init}. The manager coalesces - * requests and may apply a cooldown, so a call does not necessarily map to one obtain - * cycle. Returns immediately and does not wait for completion. + * requests and may apply a cooldown, so a call does not necessarily map to one obtain cycle. + * Returns immediately and does not wait for completion. */ void reobtainDelegationTokens(); } diff --git a/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenProvider.java b/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenProvider.java index d6d364771711ad..ea606d1524b5c3 100644 --- a/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenProvider.java +++ b/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenProvider.java @@ -30,16 +30,15 @@ * responsible to produce the serialized form of tokens which will be handled by {@link * DelegationTokenReceiver} instances both on JobManager and TaskManager side. * - *

Threading contract. A single instance per provider implementation is created and - * {@link #init(Configuration, DelegationTokenManagerCallback) initialized} once and then shared - * for the lifetime of the manager. {@link #obtainDelegationTokens()} runs on the manager's IO - * executor, while {@link - * #registerJob(JobID, Configuration)} and {@link #unregisterJob(JobID)} are invoked from the - * ResourceManager main thread; these can therefore run concurrently. {@link + *

Threading contract. A single instance per provider implementation is created and {@link + * #init(Configuration, DelegationTokenManagerCallback) initialized} once and then shared for the + * lifetime of the manager. {@link #obtainDelegationTokens()} runs on the manager's IO executor, + * while {@link #registerJob(JobID, Configuration)} and {@link #unregisterJob(JobID)} are invoked + * from the ResourceManager main thread. These can therefore run concurrently. {@link * DelegationTokenManagerCallback#reobtainDelegationTokens()} may be invoked from any thread. * Implementations must keep any per-job state thread-safe, and {@code registerJob}/{@code - * unregisterJob} must be non-blocking so they do not stall the ResourceManager — defer real work - * to {@link #obtainDelegationTokens()}. + * unregisterJob} must be non-blocking so they do not stall the ResourceManager — defer real work to + * {@link #obtainDelegationTokens()}. */ @Experimental public interface DelegationTokenProvider { @@ -98,7 +97,7 @@ default String serviceConfigPrefix() { * DelegationTokenManagerCallback#reobtainDelegationTokens()} when needed. * * @param configuration Configuration to initialize the provider. - * @param callback Used to ask the manager to re-obtain tokens; may be retained and called + * @param callback Used to ask the manager to re-obtain tokens. May be retained and called * later. */ default void init(Configuration configuration, DelegationTokenManagerCallback callback) @@ -129,7 +128,7 @@ default void init(Configuration configuration, DelegationTokenManagerCallback ca * *

A provider that requests a re-obtain must record this job's per-job state before * invoking {@link DelegationTokenManagerCallback#reobtainDelegationTokens()}. That call merely - * schedules (or coalesces into) an obtain cycle that runs later on another thread; recording + * schedules (or coalesces into) an obtain cycle that runs later on another thread. Recording * first establishes the happens-before that lets the serving cycle observe this job's state. * Recording afterwards races with the cycle and the job's tokens may be skipped until the next * periodic renewal. diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/ResourceManager.java b/flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/ResourceManager.java index db4d6aefbf1ccf..257253319c45d8 100755 --- a/flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/ResourceManager.java +++ b/flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/ResourceManager.java @@ -431,7 +431,7 @@ public CompletableFuture registerJobMaster( jobMasterIdFuture, (JobMasterGateway jobMasterGateway, JobMasterId leadingJobMasterId) -> { if (Objects.equals(leadingJobMasterId, jobMasterId)) { - // Register with the delegation token manager first; a + // Register with the delegation token manager first. A // provider failure rejects this registration so the job // never starts without the tokens it requires. try { diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java index b9f19cb9a8727a..cf97db2b015efb 100644 --- a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java +++ b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java @@ -138,9 +138,9 @@ public class DefaultDelegationTokenManager implements DelegationTokenManager { private boolean reobtainScheduled; /** - * Clock time (millis) of the last on-demand re-obtain request, used to enforce the cooldown; - * {@link #NO_PREVIOUS_REOBTAIN} until the first request. Only on-demand re-obtains update this - * (not the periodic renewal), so the cooldown spaces requests, not obtain executions. + * Clock time (millis) of the last on-demand re-obtain request, used to enforce the cooldown. + * Holds {@link #NO_PREVIOUS_REOBTAIN} until the first request. Only on-demand re-obtains update + * this (not the periodic renewal), so the cooldown spaces requests, not obtain executions. */ @GuardedBy("tokensUpdateFutureLock") private long lastReobtainAtMillis = NO_PREVIOUS_REOBTAIN; @@ -427,7 +427,7 @@ void startTokensUpdate() { /** * Schedules a one-shot token-obtain-and-broadcast cycle after {@code delayMs}, replacing any - * pending renewal; a delay of {@code 0} brings the next cycle forward to now. Must only be + * pending renewal. A delay of {@code 0} brings the next cycle forward to now. Must only be * called after {@link #start(Listener)} (the scheduled and IO executors are non-null then) and * while holding {@link #tokensUpdateFutureLock}. */ @@ -453,10 +453,10 @@ private void scheduleRenewalLocked(long delayMs) { delayMs, TimeUnit.MILLISECONDS); } catch (RejectedExecutionException e) { - // Scheduled executor is shutting down: no cycle will run, so undo the bookkeeping this - // method set. Clearing reobtainScheduled keeps a coalesced re-obtain from getting stuck; - // nextScheduledAtMillis returns to the no-cycle-pending marker (stopTokensUpdate() above - // already nulled tokensUpdateFuture). + // Scheduled executor is shutting down: no cycle will run, so undo the bookkeeping + // this method set. Clearing reobtainScheduled keeps a coalesced re-obtain from + // getting stuck. nextScheduledAtMillis returns to the no-cycle-pending marker + // (stopTokensUpdate() above already nulled tokensUpdateFuture). reobtainScheduled = false; nextScheduledAtMillis = Long.MAX_VALUE; LOG.debug("Tokens update task rejected by scheduled executor", e); @@ -595,7 +595,7 @@ public void reobtainDelegationTokens() { // pending and scheduled to fire sooner than the cooldown-deferred time, fire at that // earlier time instead of pushing it later — otherwise a short-lived token could expire // before it is renewed. The nextScheduledAtMillis > now guard skips an already-fired - // future that has not yet been re-armed, so this never bypasses the cooldown. + // future that has not yet been rescheduled, so this never bypasses the cooldown. if (tokensUpdateFuture != null && nextScheduledAtMillis > now && nextScheduledAtMillis - now < delayMillis) { diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DelegationTokenManager.java b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DelegationTokenManager.java index ccdeb36d0d1ee0..19277db8e1e2e8 100644 --- a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DelegationTokenManager.java +++ b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DelegationTokenManager.java @@ -66,10 +66,10 @@ interface Listener { /** * Requests an immediate, asynchronous token-obtain-and-distribute cycle, bringing the next - * cycle forward instead of waiting for the periodic renewal. May be called from any thread; - * it is a no-op on a manager constructed without executors (the one-shot obtain path). - * Concurrent requests are coalesced and a configurable cooldown may apply, so a call does - * not necessarily map to exactly one obtain. + * cycle forward instead of waiting for the periodic renewal. May be called from any thread. It + * is a no-op on a manager constructed without executors (the one-shot obtain path). Concurrent + * requests are coalesced and a configurable cooldown may apply, so a call does not necessarily + * map to exactly one obtain. * *

Backs {@link * org.apache.flink.core.security.token.DelegationTokenManagerCallback#reobtainDelegationTokens()}. diff --git a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java index 0e46beeb06e3ff..7ed4cff9e54119 100644 --- a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java +++ b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java @@ -550,7 +550,8 @@ public void reobtainShouldRespectCooldown() { @Test public void reobtainShouldBeIgnoredWhenNotStarted() { - // Constructed with null executors -> not started; a re-obtain request must be a safe no-op. + // Constructed with null executors, so never started. A re-obtain request must be a safe + // no-op. DefaultDelegationTokenManager delegationTokenManager = new DefaultDelegationTokenManager(new Configuration(), null, null, null); @@ -624,7 +625,7 @@ protected Optional obtainDelegationTokensAndGetNextRenewal( /** * Configuration for cooldown-scheduling tests: sets the cooldown and disables all providers - * that could fail the obtain cycle (hadoopfs/hbase need a real Hadoop setup; the throw + * that could fail the obtain cycle (hadoopfs/hbase need a real Hadoop setup, and the throw * provider fails on demand). A failed cycle schedules a jittered retry, and the bring-forward * clamp would coalesce the on-demand request into that retry instead of deferring by the * cooldown, making delay assertions nondeterministic. From 9c1c49d9b6b4ad78965d1cb7a94e04a908aa79b2 Mon Sep 17 00:00:00 2001 From: Aleksandr Savonin Date: Tue, 7 Jul 2026 18:41:58 +0200 Subject: [PATCH 03/22] [FLINK-40019][runtime] Fix delegation token manager start/stop lifecycle Rename the stopped flag to running so a never-started manager rejects work like a stopped one, and make start() idempotent. A retry no longer delays a pending re-obtain cycle, a scheduler failure no longer blocks future re-obtains, and a negative renewal delay runs immediately. --- .../token/DefaultDelegationTokenManager.java | 125 ++++++---- .../DefaultDelegationTokenManagerTest.java | 232 +++++++++++++++++- 2 files changed, 308 insertions(+), 49 deletions(-) diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java index cf97db2b015efb..5c82271b26cb27 100644 --- a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java +++ b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java @@ -146,11 +146,11 @@ public class DefaultDelegationTokenManager implements DelegationTokenManager { private long lastReobtainAtMillis = NO_PREVIOUS_REOBTAIN; /** - * Whether {@link #stop()} has been called. Reset by {@link #start(Listener)}. Prevents a late - * provider callback or an in-flight obtain cycle from scheduling new work after shutdown. + * Whether the manager is between {@link #start(Listener)} and {@link #stop()}. Defaults to + * false, so work arriving before the first start() is rejected the same way as after stop(). */ @GuardedBy("tokensUpdateFutureLock") - private boolean stopped; + private boolean running; @Nullable private Listener listener; @@ -358,10 +358,16 @@ protected Optional obtainDelegationTokensAndGetNextRenewal( public void start(Listener listener) throws Exception { checkNotNull(scheduledExecutor, "Scheduled executor must not be null"); checkNotNull(ioExecutor, "IO executor must not be null"); - this.listener = checkNotNull(listener, "Listener must not be null"); + checkNotNull(listener, "Listener must not be null"); synchronized (tokensUpdateFutureLock) { - checkState(tokensUpdateFuture == null, "Manager is already started"); - stopped = false; + if (running) { + LOG.warn("DelegationTokenManager is already started, ignoring redundant start()"); + return; + } + this.listener = listener; + // Set before the inline first cycle below: startTokensUpdate() and + // maybeScheduleRenewal() gate on it. + running = true; } startTokensUpdate(); @@ -370,13 +376,11 @@ public void start(Listener listener) throws Exception { @VisibleForTesting void startTokensUpdate() { synchronized (tokensUpdateFutureLock) { - // The obtain cycle is starting: clear the dedupe flag so later on-demand requests can - // schedule a fresh cycle. + // Clear the dedupe flag so later on-demand requests can schedule a fresh cycle. reobtainScheduled = false; - // If stop() ran before this cycle (already handed to the IO executor) began, skip the - // obtain/broadcast: the providers may already be stopped. Safe via this lock's - // happens-before with stop(). The dedupe flag is cleared above, so it is never stuck. - if (stopped) { + // Stopped or never started: skip the cycle. The providers may already be stopped + // and the listener may not be set yet. + if (!running) { return; } } @@ -403,10 +407,14 @@ void startTokensUpdate() { lastKnownNextRenewal = nextRenewal.get(); currentRetryBackoff = renewalRetryInitialBackoff; long renewalDelay = calculateRenewalDelay(clock, nextRenewal.get()); - maybeScheduleRenewal(renewalDelay); - LOG.info( - "Tokens update task started with {} delay", - TimeUtils.formatWithHighestUnit(Duration.ofMillis(renewalDelay))); + long effectiveDelay = maybeScheduleRenewal(renewalDelay); + if (effectiveDelay >= 0) { + LOG.info( + "Tokens update task started with {} delay", + TimeUtils.formatWithHighestUnit(Duration.ofMillis(effectiveDelay))); + } else { + LOG.info("Tokens update task not rescheduled, the manager is not running"); + } } else { LOG.warn( "Tokens update task not started because either no tokens obtained or none of the tokens specified its renewal date"); @@ -416,11 +424,25 @@ void startTokensUpdate() { LOG.debug("Interrupted", e); } catch (Exception e) { long delay = calculateRetryDelay(clock); - maybeScheduleRenewal(delay); - LOG.warn( - "Failed to update tokens, will try again in {}", - TimeUtils.formatWithHighestUnit(Duration.ofMillis(delay)), - e); + long effectiveDelay; + try { + effectiveDelay = maybeScheduleRenewal(delay); + } catch (Throwable schedulingFailure) { + // The original failure was not logged yet, keep it attached. + schedulingFailure.addSuppressed(e); + throw schedulingFailure; + } + if (effectiveDelay >= 0) { + LOG.warn( + "Failed to update tokens, will try again in {}", + TimeUtils.formatWithHighestUnit(Duration.ofMillis(effectiveDelay)), + e); + } else { + LOG.warn( + "Failed to update tokens, no retry scheduled because the manager is " + + "not running", + e); + } } } } @@ -454,34 +476,54 @@ private void scheduleRenewalLocked(long delayMs) { TimeUnit.MILLISECONDS); } catch (RejectedExecutionException e) { // Scheduled executor is shutting down: no cycle will run, so undo the bookkeeping - // this method set. Clearing reobtainScheduled keeps a coalesced re-obtain from - // getting stuck. nextScheduledAtMillis returns to the no-cycle-pending marker - // (stopTokensUpdate() above already nulled tokensUpdateFuture). + // this method set. reobtainScheduled = false; nextScheduledAtMillis = Long.MAX_VALUE; LOG.debug("Tokens update task rejected by scheduled executor", e); + } catch (Throwable t) { + // Undo the same bookkeeping as the rejection branch, or every later re-obtain would + // be coalesced against a cycle that never got scheduled. Rethrow to keep the failure + // visible. + reobtainScheduled = false; + nextScheduledAtMillis = Long.MAX_VALUE; + throw t; } } /** - * Schedules the next periodic renewal at the end of a completed obtain cycle, unless the - * manager was stopped or an on-demand re-obtain was scheduled while this cycle ran. A pending - * on-demand cycle already re-establishes the renewal schedule, so it is left in place rather - * than cancelled: the periodic renewal is folded into it, never dropped. + * Schedules the next cycle (periodic renewal or failure retry). A pending on-demand cycle is + * brought forward when {@code delayMs} is sooner and left in place otherwise, so a pending + * cycle is never delayed. + * + * @param delayMs requested delay in millis + * @return the delay in millis until the cycle that will actually run next, or -1 when nothing + * is scheduled because the manager is not running. */ @VisibleForTesting - void maybeScheduleRenewal(long delayMs) { + long maybeScheduleRenewal(long delayMs) { + // A negative delay (the token already passed its validUntil) means run now. Clamp it so + // it cannot be mistaken for the -1 not-running sentinel. + delayMs = Math.max(0L, delayMs); synchronized (tokensUpdateFutureLock) { - if (stopped) { - return; + if (!running) { + return -1L; } if (reobtainScheduled) { + long pendingInMillis = Math.max(0L, nextScheduledAtMillis - clock.millis()); + if (delayMs < pendingInMillis) { + // Bring the pending on-demand cycle forward. scheduleRenewalLocked() leaves + // reobtainScheduled set, so coalescing still holds and the earlier cycle + // serves the coalesced on-demand requests too. + scheduleRenewalLocked(delayMs); + return delayMs; + } LOG.debug( - "An on-demand re-obtain is already scheduled; leaving it in place instead " - + "of overwriting it with the periodic renewal."); - return; + "An on-demand re-obtain is already scheduled to fire sooner, leaving it " + + "in place."); + return pendingInMillis; } scheduleRenewalLocked(delayMs); + return delayMs; } } @@ -542,10 +584,10 @@ public void stop() { LOG.info("Stopping credential renewal"); synchronized (tokensUpdateFutureLock) { - // Mark stopped, cancel the pending cycle, and reset on-demand re-obtain bookkeeping - // atomically, so a concurrent reobtainDelegationTokens() cannot leave a live future - // orphaned after stop and a later start() does not inherit stale state. - stopped = true; + // Mark not running, cancel the pending cycle, and reset the re-obtain bookkeeping + // atomically, so a re-obtain racing shutdown cannot schedule a cycle for a manager + // that is shutting down. + running = false; stopTokensUpdate(); reobtainScheduled = false; lastReobtainAtMillis = NO_PREVIOUS_REOBTAIN; @@ -572,10 +614,11 @@ public void reobtainDelegationTokens() { + "request is ignored."); return; } - if (stopped) { + if (!running) { LOG.debug( - "A re-obtain of delegation tokens was requested after the manager was " - + "stopped; the request is ignored."); + "A re-obtain of delegation tokens was requested while the manager is not " + + "running (not started yet, or already stopped), ignoring the " + + "request."); return; } // Dedupe: if an on-demand re-obtain is already scheduled and has not started yet, the diff --git a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java index 7ed4cff9e54119..0e815ba97a2503 100644 --- a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java +++ b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java @@ -24,6 +24,7 @@ import org.apache.flink.core.security.token.DelegationTokenReceiver; import org.apache.flink.core.testutils.ManuallyTriggeredScheduledExecutorService; import org.apache.flink.util.concurrent.ManuallyTriggeredScheduledExecutor; +import org.apache.flink.util.concurrent.ScheduledExecutor; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -40,6 +41,7 @@ import java.util.Optional; import java.util.Set; import java.util.concurrent.BrokenBarrierException; +import java.util.concurrent.Callable; import java.util.concurrent.CountDownLatch; import java.util.concurrent.CyclicBarrier; import java.util.concurrent.ExecutorService; @@ -205,7 +207,7 @@ public void checkSamePrefixedProvidersShouldGiveErrorsWhenSamePrefix() { } @Test - public void startTokensUpdateShouldScheduleRenewal() { + public void startTokensUpdateShouldScheduleRenewal() throws Exception { final ManuallyTriggeredScheduledExecutor scheduledExecutor = new ManuallyTriggeredScheduledExecutor(); final ManuallyTriggeredScheduledExecutorService scheduler = @@ -214,6 +216,8 @@ public void startTokensUpdateShouldScheduleRenewal() { ExceptionThrowingDelegationTokenProvider.addToken.set(true); Configuration configuration = new Configuration(); configuration.set(getBooleanConfigOption(CONFIG_PREFIX + ".throw.enabled"), true); + configuration.set(getBooleanConfigOption(CONFIG_PREFIX + ".hadoopfs.enabled"), false); + configuration.set(getBooleanConfigOption(CONFIG_PREFIX + ".hbase.enabled"), false); AtomicInteger startTokensUpdateCallCount = new AtomicInteger(0); DefaultDelegationTokenManager delegationTokenManager = new DefaultDelegationTokenManager( @@ -225,8 +229,9 @@ void startTokensUpdate() { } }; - delegationTokenManager.startTokensUpdate(); + // The first two cycles fail and schedule a retry each. The third succeeds. ExceptionThrowingDelegationTokenProvider.throwInUsage.set(true); + delegationTokenManager.start(tokens -> {}); scheduledExecutor.triggerScheduledTasks(); scheduler.triggerAll(); ExceptionThrowingDelegationTokenProvider.throwInUsage.set(false); @@ -324,6 +329,8 @@ public void registerJobShouldTriggerImmediateRenewalAndTrackJob() throws Excepti Configuration configuration = new Configuration(); configuration.set(getBooleanConfigOption(CONFIG_PREFIX + ".throw.enabled"), true); + configuration.set(getBooleanConfigOption(CONFIG_PREFIX + ".hadoopfs.enabled"), false); + configuration.set(getBooleanConfigOption(CONFIG_PREFIX + ".hbase.enabled"), false); AtomicInteger startTokensUpdateCallCount = new AtomicInteger(0); DefaultDelegationTokenManager delegationTokenManager = new DefaultDelegationTokenManager( @@ -337,6 +344,10 @@ void startTokensUpdate() { // Ask the provider to request an immediate refresh when the job is registered. ExceptionThrowingDelegationTokenProvider.shouldReobtainOnRegister.set(true); + delegationTokenManager.start(tokens -> {}); + // Only count the cycle triggered by the registration below, not start()'s inline cycle. + startTokensUpdateCallCount.set(0); + JobID jobId = JobID.generate(); delegationTokenManager.registerJob(jobId, new Configuration()); scheduledExecutor.triggerScheduledTasks(); @@ -394,7 +405,7 @@ public void unregisterJobShouldSwallowProviderFailure() throws Exception { } @Test - public void reobtainShouldCoalesceConcurrentRequests() { + public void reobtainShouldCoalesceConcurrentRequests() throws Exception { final ManuallyTriggeredScheduledExecutor scheduledExecutor = new ManuallyTriggeredScheduledExecutor(); final ManuallyTriggeredScheduledExecutorService scheduler = @@ -403,13 +414,19 @@ public void reobtainShouldCoalesceConcurrentRequests() { AtomicInteger startTokensUpdateCallCount = new AtomicInteger(0); DefaultDelegationTokenManager delegationTokenManager = new DefaultDelegationTokenManager( - new Configuration(), null, scheduledExecutor, scheduler) { + hermeticCooldownConfig(Duration.ofMillis(60_000)), + null, + scheduledExecutor, + scheduler) { @Override void startTokensUpdate() { startTokensUpdateCallCount.incrementAndGet(); super.startTokensUpdate(); } }; + delegationTokenManager.start(tokens -> {}); + // Only count the cycle serving the coalesced requests, not start()'s inline cycle. + startTokensUpdateCallCount.set(0); // Two requests before the cycle runs must be coalesced into a single scheduled obtain. delegationTokenManager.reobtainDelegationTokens(); @@ -426,7 +443,7 @@ void startTokensUpdate() { } @Test - public void periodicRenewalMustNotCancelPendingOnDemandReobtain() { + public void periodicRenewalMustNotCancelPendingOnDemandReobtain() throws Exception { final ManuallyTriggeredScheduledExecutor scheduledExecutor = new ManuallyTriggeredScheduledExecutor(); final ManuallyTriggeredScheduledExecutorService scheduler = @@ -434,7 +451,11 @@ public void periodicRenewalMustNotCancelPendingOnDemandReobtain() { DefaultDelegationTokenManager delegationTokenManager = new DefaultDelegationTokenManager( - new Configuration(), null, scheduledExecutor, scheduler); + hermeticCooldownConfig(Duration.ofMillis(60_000)), + null, + scheduledExecutor, + scheduler); + delegationTokenManager.start(tokens -> {}); // An on-demand re-obtain is scheduled (e.g. a freshly registered job). delegationTokenManager.reobtainDelegationTokens(); @@ -460,7 +481,7 @@ public void periodicRenewalMustNotCancelPendingOnDemandReobtain() { } @Test - public void reobtainShouldRunImmediatelyAfterCooldownWindowElapses() { + public void reobtainShouldRunImmediatelyAfterCooldownWindowElapses() throws Exception { final ManuallyTriggeredScheduledExecutor scheduledExecutor = new ManuallyTriggeredScheduledExecutor(); final ManuallyTriggeredScheduledExecutorService scheduler = @@ -473,6 +494,7 @@ public void reobtainShouldRunImmediatelyAfterCooldownWindowElapses() { long t0 = 1_000_000L; delegationTokenManager.setClock(Clock.fixed(ofEpochMilli(t0), ZoneId.systemDefault())); + delegationTokenManager.start(tokens -> {}); delegationTokenManager.reobtainDelegationTokens(); assertEquals(0L, onlyScheduledDelayMillis(scheduledExecutor)); scheduledExecutor.triggerScheduledTasks(); @@ -499,6 +521,7 @@ public void stopShouldResetCooldownForSubsequentStart() throws Exception { long t0 = 1_000_000L; delegationTokenManager.setClock(Clock.fixed(ofEpochMilli(t0), ZoneId.systemDefault())); + delegationTokenManager.start(tokens -> {}); delegationTokenManager.reobtainDelegationTokens(); assertEquals(0L, onlyScheduledDelayMillis(scheduledExecutor)); scheduledExecutor.triggerScheduledTasks(); @@ -521,7 +544,7 @@ public void stopShouldResetCooldownForSubsequentStart() throws Exception { } @Test - public void reobtainShouldRespectCooldown() { + public void reobtainShouldRespectCooldown() throws Exception { final ManuallyTriggeredScheduledExecutor scheduledExecutor = new ManuallyTriggeredScheduledExecutor(); final ManuallyTriggeredScheduledExecutorService scheduler = @@ -535,6 +558,8 @@ public void reobtainShouldRespectCooldown() { long t0 = 1_000_000L; delegationTokenManager.setClock(Clock.fixed(ofEpochMilli(t0), ZoneId.systemDefault())); + delegationTokenManager.start(tokens -> {}); + // First re-obtain after a quiet period runs immediately (no cooldown applies). delegationTokenManager.reobtainDelegationTokens(); assertEquals(0L, onlyScheduledDelayMillis(scheduledExecutor)); @@ -558,6 +583,188 @@ public void reobtainShouldBeIgnoredWhenNotStarted() { assertDoesNotThrow(delegationTokenManager::reobtainDelegationTokens); } + @Test + public void reobtainBeforeStartMustNotScheduleObtainCycle() { + final ManuallyTriggeredScheduledExecutor scheduledExecutor = + new ManuallyTriggeredScheduledExecutor(); + final ManuallyTriggeredScheduledExecutorService scheduler = + new ManuallyTriggeredScheduledExecutorService(); + + DefaultDelegationTokenManager delegationTokenManager = + new DefaultDelegationTokenManager( + hermeticCooldownConfig(Duration.ofMillis(60_000)), + null, + scheduledExecutor, + scheduler); + + // Providers receive the re-obtain callback already in the constructor (init), so a + // provider can invoke it before start(). The manager has no listener yet, so the + // request must be rejected instead of dispatching an obtain cycle that can only fail + // on the null listener and keep rescheduling itself through the retry path. + delegationTokenManager.reobtainDelegationTokens(); + + assertEquals( + 0, + scheduledExecutor.getActiveScheduledTasks().size(), + "A re-obtain before start() must not schedule an obtain cycle"); + } + + @Test + public void schedulerFailureMustNotWedgeSubsequentReobtains() throws Exception { + final ManuallyTriggeredScheduledExecutor delegate = + new ManuallyTriggeredScheduledExecutor(); + final ManuallyTriggeredScheduledExecutorService scheduler = + new ManuallyTriggeredScheduledExecutorService(); + + // Throws a plain RuntimeException (not a RejectedExecutionException) on the next + // schedule() call when the flag is set, then behaves normally again. + final AtomicBoolean throwNext = new AtomicBoolean(false); + ScheduledExecutor throwOnce = + new ScheduledExecutor() { + @Override + public ScheduledFuture schedule( + Runnable command, long delay, TimeUnit unit) { + if (throwNext.compareAndSet(true, false)) { + throw new RuntimeException("simulated scheduler failure"); + } + return delegate.schedule(command, delay, unit); + } + + @Override + public ScheduledFuture schedule( + Callable callable, long delay, TimeUnit unit) { + return delegate.schedule(callable, delay, unit); + } + + @Override + public ScheduledFuture scheduleAtFixedRate( + Runnable command, long initialDelay, long period, TimeUnit unit) { + return delegate.scheduleAtFixedRate(command, initialDelay, period, unit); + } + + @Override + public ScheduledFuture scheduleWithFixedDelay( + Runnable command, long initialDelay, long delay, TimeUnit unit) { + return delegate.scheduleWithFixedDelay(command, initialDelay, delay, unit); + } + + @Override + public void execute(Runnable command) { + delegate.execute(command); + } + }; + + DefaultDelegationTokenManager delegationTokenManager = + new DefaultDelegationTokenManager( + hermeticCooldownConfig(Duration.ZERO), null, throwOnce, scheduler); + delegationTokenManager.start(tokens -> {}); + + // The first re-obtain hits a scheduler that blows up with something other than the + // handled RejectedExecutionException. The failure propagates to the caller. + throwNext.set(true); + assertThrows(RuntimeException.class, delegationTokenManager::reobtainDelegationTokens); + + // The scheduler is healthy again. The next re-obtain must schedule a fresh obtain + // cycle instead of being coalesced against the cycle that never got scheduled. + delegationTokenManager.reobtainDelegationTokens(); + assertEquals( + 1, + delegate.getActiveScheduledTasks().size(), + "A re-obtain after a scheduler failure must schedule a fresh obtain cycle"); + } + + @Test + public void startShouldBeIdempotent() throws Exception { + final ManuallyTriggeredScheduledExecutor scheduledExecutor = + new ManuallyTriggeredScheduledExecutor(); + final ManuallyTriggeredScheduledExecutorService scheduler = + new ManuallyTriggeredScheduledExecutorService(); + + // The throw provider produces tokens so the listener gets notified on every cycle. + ExceptionThrowingDelegationTokenProvider.addToken.set(true); + Configuration configuration = new Configuration(); + configuration.set(getBooleanConfigOption(CONFIG_PREFIX + ".throw.enabled"), true); + configuration.set(getBooleanConfigOption(CONFIG_PREFIX + ".hadoopfs.enabled"), false); + configuration.set(getBooleanConfigOption(CONFIG_PREFIX + ".hbase.enabled"), false); + + AtomicInteger startTokensUpdateCallCount = new AtomicInteger(0); + DefaultDelegationTokenManager delegationTokenManager = + new DefaultDelegationTokenManager( + configuration, null, scheduledExecutor, scheduler) { + @Override + void startTokensUpdate() { + startTokensUpdateCallCount.incrementAndGet(); + super.startTokensUpdate(); + } + }; + + AtomicInteger firstListenerNotifications = new AtomicInteger(0); + AtomicInteger secondListenerNotifications = new AtomicInteger(0); + delegationTokenManager.start(tokens -> firstListenerNotifications.incrementAndGet()); + // A redundant start() (e.g. a buggy caller) must be ignored: no second inline obtain + // cycle, and the listener of the running manager must not be swapped. + delegationTokenManager.start(tokens -> secondListenerNotifications.incrementAndGet()); + + assertEquals( + 1, + startTokensUpdateCallCount.get(), + "A redundant start() must not run another obtain cycle"); + assertEquals( + 1, + firstListenerNotifications.get(), + "The first start()'s inline cycle must notify the listener once"); + + // A later cycle must still notify the original listener, not the ignored one. + delegationTokenManager.reobtainDelegationTokens(); + scheduledExecutor.triggerScheduledTasks(); + scheduler.triggerAll(); + assertEquals( + 2, + firstListenerNotifications.get(), + "The original listener must keep receiving tokens"); + assertEquals( + 0, + secondListenerNotifications.get(), + "The listener from the ignored start() must never receive tokens"); + } + + @Test + public void retryMustBringPendingOnDemandReobtainForward() throws Exception { + final ManuallyTriggeredScheduledExecutor scheduledExecutor = + new ManuallyTriggeredScheduledExecutor(); + final ManuallyTriggeredScheduledExecutorService scheduler = + new ManuallyTriggeredScheduledExecutorService(); + + Configuration configuration = hermeticCooldownConfig(Duration.ofMillis(60_000)); + DefaultDelegationTokenManager delegationTokenManager = + new DefaultDelegationTokenManager( + configuration, null, scheduledExecutor, scheduler); + + long t0 = 1_000_000L; + delegationTokenManager.setClock(Clock.fixed(ofEpochMilli(t0), ZoneId.systemDefault())); + delegationTokenManager.start(tokens -> {}); + + delegationTokenManager.reobtainDelegationTokens(); + scheduledExecutor.triggerScheduledTasks(); + scheduler.triggerAll(); + + // 10s later a second re-obtain is cooldown-deferred by 50s. + delegationTokenManager.setClock( + Clock.fixed(ofEpochMilli(t0 + 10_000L), ZoneId.systemDefault())); + delegationTokenManager.reobtainDelegationTokens(); + assertEquals(50_000L, onlyScheduledDelayMillis(scheduledExecutor)); + + // A failed cycle now wants a retry in 10s. The pending on-demand cycle must be brought + // forward to the sooner time instead of silently swallowing the retry, otherwise the + // effective retry would fire 50s out while the backoff (and its token-TTL cap) asked + // for 10s. + delegationTokenManager.maybeScheduleRenewal(10_000L); + assertEquals( + 10_000L, + onlyScheduledDelayMillis(scheduledExecutor), + "A sooner retry must bring the pending on-demand cycle forward"); + } + @Test public void registerJobShouldBeIdempotent() throws Exception { DefaultDelegationTokenManager delegationTokenManager = @@ -583,6 +790,9 @@ public void obtainLockSerializesConcurrentObtainCycles() throws Exception { final CyclicBarrier barrier = new CyclicBarrier(2); final AtomicBoolean concurrentObtainDetected = new AtomicBoolean(false); final CountDownLatch done = new CountDownLatch(2); + // Enabled only after start(): its inline first cycle must not wait at (and, by + // timing out, break) the barrier meant for the two concurrent cycles below. + final AtomicBoolean barrierEnabled = new AtomicBoolean(false); DefaultDelegationTokenManager delegationTokenManager = new DefaultDelegationTokenManager( @@ -590,6 +800,9 @@ public void obtainLockSerializesConcurrentObtainCycles() throws Exception { @Override protected Optional obtainDelegationTokensAndGetNextRenewal( DelegationTokenContainer container) { + if (!barrierEnabled.get()) { + return Optional.empty(); + } try { barrier.await(200, TimeUnit.MILLISECONDS); // Reached only if both cycles met here concurrently. @@ -603,6 +816,9 @@ protected Optional obtainDelegationTokensAndGetNextRenewal( } }; + delegationTokenManager.start(tokens -> {}); + barrierEnabled.set(true); + ioExecutor.execute( () -> { delegationTokenManager.startTokensUpdate(); From 485c10a47f6c884b15ec34ecfd2999f29cead82a Mon Sep 17 00:00:00 2001 From: Aleksandr Savonin Date: Tue, 7 Jul 2026 18:44:44 +0200 Subject: [PATCH 04/22] [FLINK-40019][core][runtime] Treat provider LinkageError like a failure in job registration and cleanup A LinkageError from provider plugin code (realistically a NoClassDefFoundError, the same failure class loadProviders already special-cases at init) previously slipped past the Exception-only catches on both job hooks. The ResourceManager now also wraps the rethrown registration failure into a FlinkException naming the job before it is sent back in the RegistrationResponse, so the JobMaster log points at the job and the delegation token manager instead of a bare provider exception. --- .../token/DelegationTokenProvider.java | 13 ++++---- .../resourcemanager/ResourceManager.java | 18 +++++++--- .../token/DefaultDelegationTokenManager.java | 13 ++++---- .../ResourceManagerJobMasterTest.java | 6 ++-- .../DefaultDelegationTokenManagerTest.java | 33 +++++++++++++++++++ ...eptionThrowingDelegationTokenProvider.java | 12 +++++++ 6 files changed, 75 insertions(+), 20 deletions(-) diff --git a/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenProvider.java b/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenProvider.java index ea606d1524b5c3..77a333d0407aea 100644 --- a/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenProvider.java +++ b/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenProvider.java @@ -136,10 +136,10 @@ default void init(Configuration configuration, DelegationTokenManagerCallback ca *

Must be idempotent: it may be called more than once for the same {@code jobId} (e.g. on * JobManager or ResourceManager failover, when the JobMaster re-registers). * - *

Should not throw: a thrown (unchecked) exception rejects the job's registration (the job - * does not start) and triggers {@link #unregisterJob(JobID)} on all providers to roll back. - * Prefer deferring the real fetch to the (retrying) obtain cycle over a synchronous fetch, so a - * transient failure does not fail the job. + *

Should not throw: a thrown (unchecked) exception or linkage error rejects the job's + * registration (the job does not start) and triggers {@link #unregisterJob(JobID)} on all + * providers to roll back. Prefer deferring the real fetch to the (retrying) obtain cycle over a + * synchronous fetch, so a transient failure does not fail the job. * * @param jobId The job id of the job. * @param jobConfiguration The job configuration. @@ -149,8 +149,9 @@ default void registerJob(JobID jobId, Configuration jobConfiguration) {} /** * Called when the job is being removed — it reached a globally terminal state, or its * job-leader registration timed out — and its per-job state should be released. Must be - * idempotent. Exceptions are caught and logged by the framework (one provider's failure does - * not abort cleanup of the others), but implementations should still avoid throwing. + * idempotent. Exceptions and linkage errors are caught and logged by the framework (one + * provider's failure does not abort cleanup of the others), but implementations should still + * avoid throwing. * * @param jobId The job id of the job. */ diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/ResourceManager.java b/flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/ResourceManager.java index 257253319c45d8..918f2c971a1a4f 100755 --- a/flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/ResourceManager.java +++ b/flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/ResourceManager.java @@ -431,13 +431,21 @@ public CompletableFuture registerJobMaster( jobMasterIdFuture, (JobMasterGateway jobMasterGateway, JobMasterId leadingJobMasterId) -> { if (Objects.equals(leadingJobMasterId, jobMasterId)) { - // Register with the delegation token manager first. A - // provider failure rejects this registration so the job - // never starts without the tokens it requires. + // Register with the delegation token manager first, so a + // provider failure rejects the registration and the job does + // not start without the tokens it requires. LinkageError is + // caught so a plugin classpath failure is reported the same + // way. try { delegationTokenManager.registerJob(jobId, jobConfiguration); - } catch (Exception e) { - return new RegistrationResponse.Failure(e); + } catch (Exception | LinkageError e) { + return new RegistrationResponse.Failure( + new FlinkException( + "Failed to register job " + + jobId + + " with the delegation token " + + "manager", + e)); } return registerJobMasterInternal( jobMasterGateway, diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java index 5c82271b26cb27..5d5ba233b5ca18 100644 --- a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java +++ b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java @@ -659,14 +659,13 @@ public void registerJob(JobID jobId, Configuration jobConfiguration) throws Exce for (DelegationTokenProvider provider : delegationTokenProviders.values()) { provider.registerJob(jobId, jobConfiguration); } - } catch (Exception e) { - // If any of the providers fail to register, then unregister the job from them all. - // unregisterJob is idempotent, so it is safe to call it for providers that were never - // (or only partially) registered for this job before the failure. The rollback must - // never mask the original failure, so swallow any rollback exception. + } catch (Exception | LinkageError e) { + // Roll the job back from all providers (unregisterJob is idempotent) and rethrow. + // The rollback must never mask the original failure. LinkageError is included + // because provider plugin code can fail class resolution. try { unregisterJob(jobId); - } catch (Exception rollbackException) { + } catch (Exception | LinkageError rollbackException) { LOG.error("Failed to roll back registration of job {}", jobId, rollbackException); } LOG.error("Failed to register job {}", jobId, e); @@ -679,7 +678,7 @@ public void unregisterJob(JobID jobId) throws Exception { for (DelegationTokenProvider provider : delegationTokenProviders.values()) { try { provider.unregisterJob(jobId); - } catch (Exception e) { + } catch (Exception | LinkageError e) { LOG.error("Failed to unregister job for provider {}", provider.serviceName(), e); } } diff --git a/flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/ResourceManagerJobMasterTest.java b/flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/ResourceManagerJobMasterTest.java index 8830bdea6396e6..8fb78372ac51a2 100644 --- a/flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/ResourceManagerJobMasterTest.java +++ b/flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/ResourceManagerJobMasterTest.java @@ -188,8 +188,10 @@ void testRegisterJobMasterRejectedWhenDelegationTokenRegistrationFails() throws final RegistrationResponse response = registrationFuture.get(TIMEOUT.toMillis(), TimeUnit.MILLISECONDS); assertThat(response).isInstanceOf(RegistrationResponse.Failure.class); - assertThat(((RegistrationResponse.Failure) response).getReason().getMessage()) - .contains("registerJob rejected by provider"); + final Throwable reason = ((RegistrationResponse.Failure) response).getReason(); + assertThat(reason.getMessage()).contains(jobId.toString()); + assertThat(reason.getMessage()).contains("delegation token manager"); + assertThat(reason.getCause().getMessage()).contains("registerJob rejected by provider"); } @Test diff --git a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java index 0e815ba97a2503..f4bd99527adbdd 100644 --- a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java +++ b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java @@ -390,6 +390,25 @@ public void registerJobShouldRollBackAndRethrowWhenProviderThrows() throws Excep assertEquals(0, ExceptionThrowingDelegationTokenProvider.registeredJobs.get().size()); } + @Test + public void registerJobFailureWithLinkageErrorMustRollBackProviders() { + DefaultDelegationTokenManager delegationTokenManager = + new DefaultDelegationTokenManager(new Configuration(), null, null, null); + + // A LinkageError from provider plugin code must get the same treatment as an exception: + // roll back on all providers and rethrow. + ExceptionThrowingDelegationTokenProvider.throwErrorInRegister.set(true); + JobID jobId = JobID.generate(); + + assertThrows( + NoClassDefFoundError.class, + () -> delegationTokenManager.registerJob(jobId, new Configuration())); + assertTrue( + ExceptionThrowingDelegationTokenProvider.registeredJobs.get().isEmpty(), + "A registration that failed with a LinkageError must be rolled back on all" + + " providers"); + } + @Test public void unregisterJobShouldSwallowProviderFailure() throws Exception { Configuration configuration = new Configuration(); @@ -404,6 +423,20 @@ public void unregisterJobShouldSwallowProviderFailure() throws Exception { assertDoesNotThrow(() -> delegationTokenManager.unregisterJob(jobId)); } + @Test + public void unregisterJobShouldSwallowProviderLinkageError() throws Exception { + DefaultDelegationTokenManager delegationTokenManager = + new DefaultDelegationTokenManager(new Configuration(), null, null, null); + + JobID jobId = JobID.generate(); + delegationTokenManager.registerJob(jobId, new Configuration()); + + // A LinkageError during unregistration must be swallowed like an exception, so it does + // not abort the cleanup of the remaining providers. + ExceptionThrowingDelegationTokenProvider.throwErrorInUnregister.set(true); + assertDoesNotThrow(() -> delegationTokenManager.unregisterJob(jobId)); + } + @Test public void reobtainShouldCoalesceConcurrentRequests() throws Exception { final ManuallyTriggeredScheduledExecutor scheduledExecutor = diff --git a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/ExceptionThrowingDelegationTokenProvider.java b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/ExceptionThrowingDelegationTokenProvider.java index 292a716bb2ee91..69ecff074cab85 100644 --- a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/ExceptionThrowingDelegationTokenProvider.java +++ b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/ExceptionThrowingDelegationTokenProvider.java @@ -44,8 +44,12 @@ public class ExceptionThrowingDelegationTokenProvider implements DelegationToken ThreadLocal.withInitial(() -> Boolean.FALSE); public static volatile ThreadLocal throwInRegister = ThreadLocal.withInitial(() -> Boolean.FALSE); + public static volatile ThreadLocal throwErrorInRegister = + ThreadLocal.withInitial(() -> Boolean.FALSE); public static volatile ThreadLocal throwInUnregister = ThreadLocal.withInitial(() -> Boolean.FALSE); + public static volatile ThreadLocal throwErrorInUnregister = + ThreadLocal.withInitial(() -> Boolean.FALSE); public static volatile ThreadLocal stopped = ThreadLocal.withInitial(() -> Boolean.FALSE); public static volatile ThreadLocal> registeredJobs = @@ -58,7 +62,9 @@ public static void reset() { constructed.set(false); shouldReobtainOnRegister.set(false); throwInRegister.set(false); + throwErrorInRegister.set(false); throwInUnregister.set(false); + throwErrorInUnregister.set(false); stopped.set(false); registeredJobs.get().clear(); } @@ -113,6 +119,9 @@ public void registerJob(JobID jobId, Configuration jobConfiguration) { throw new IllegalArgumentException(); } registeredJobs.get().add(jobId); + if (throwErrorInRegister.get()) { + throw new NoClassDefFoundError("simulated classpath failure in provider registerJob"); + } if (shouldReobtainOnRegister.get()) { callback.reobtainDelegationTokens(); } @@ -123,6 +132,9 @@ public void unregisterJob(JobID jobId) { if (throwInUnregister.get()) { throw new IllegalArgumentException(); } + if (throwErrorInUnregister.get()) { + throw new NoClassDefFoundError("simulated classpath failure in provider unregisterJob"); + } registeredJobs.get().remove(jobId); } From 3c1dbd76bce6fbd345735c233c40b30d28824d6c Mon Sep 17 00:00:00 2001 From: Aleksandr Savonin Date: Tue, 7 Jul 2026 18:46:03 +0200 Subject: [PATCH 05/22] [FLINK-40019][runtime] Add job and provider in registration failure logs --- .../token/DefaultDelegationTokenManager.java | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java index 5d5ba233b5ca18..a81caf54abb291 100644 --- a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java +++ b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java @@ -655,8 +655,10 @@ public void reobtainDelegationTokens() { @Override public void registerJob(JobID jobId, Configuration jobConfiguration) throws Exception { + DelegationTokenProvider failedProvider = null; try { for (DelegationTokenProvider provider : delegationTokenProviders.values()) { + failedProvider = provider; provider.registerJob(jobId, jobConfiguration); } } catch (Exception | LinkageError e) { @@ -668,7 +670,11 @@ public void registerJob(JobID jobId, Configuration jobConfiguration) throws Exce } catch (Exception | LinkageError rollbackException) { LOG.error("Failed to roll back registration of job {}", jobId, rollbackException); } - LOG.error("Failed to register job {}", jobId, e); + LOG.error( + "Failed to register job {} for provider {}", + jobId, + failedProvider == null ? "" : failedProvider.serviceName(), + e); throw e; } } @@ -679,7 +685,11 @@ public void unregisterJob(JobID jobId) throws Exception { try { provider.unregisterJob(jobId); } catch (Exception | LinkageError e) { - LOG.error("Failed to unregister job for provider {}", provider.serviceName(), e); + LOG.error( + "Failed to unregister job {} for provider {}", + jobId, + provider.serviceName(), + e); } } } From 451af6c2b9a6c3bfe7513fc2bd41da37554023ce Mon Sep 17 00:00:00 2001 From: Aleksandr Savonin Date: Tue, 21 Jul 2026 17:41:11 +0200 Subject: [PATCH 06/22] [FLINK-40019][core][runtime] Keep per-job token state consistent across re-registrations and sessions A failed re-registration of an already-registered job no longer rolls the job back, so a transient provider failure during a JobMaster reconnect cannot wipe a running job's token state. A manager-held registry tracks jobs whose provider state may exist: failed unregistrations stay tracked and are retried by the stop() drain, and stop() releases the listener and all job registrations so nothing leaks across leadership sessions. start() resets the retry backoff, the re-obtain cooldown is anchored to cycle executions (as the config option documents), an in-flight obtain cycle re-checks the running state before broadcasting, and providers receive a defensive copy of the job configuration. --- .../token/DelegationTokenProvider.java | 9 +- .../token/DefaultDelegationTokenManager.java | 165 +++++++-- .../token/DelegationTokenManager.java | 19 +- .../DefaultDelegationTokenManagerTest.java | 334 +++++++++++++++++- ...eptionThrowingDelegationTokenProvider.java | 10 + 5 files changed, 486 insertions(+), 51 deletions(-) diff --git a/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenProvider.java b/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenProvider.java index 77a333d0407aea..e884ed49c5d526 100644 --- a/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenProvider.java +++ b/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenProvider.java @@ -32,9 +32,12 @@ * *

Threading contract. A single instance per provider implementation is created and {@link * #init(Configuration, DelegationTokenManagerCallback) initialized} once and then shared for the - * lifetime of the manager. {@link #obtainDelegationTokens()} runs on the manager's IO executor, - * while {@link #registerJob(JobID, Configuration)} and {@link #unregisterJob(JobID)} are invoked - * from the ResourceManager main thread. These can therefore run concurrently. {@link + * lifetime of the manager. {@link #obtainDelegationTokens()} usually runs on the manager's IO + * executor, but the first cycle runs on the thread that starts the manager (the ResourceManager + * main thread) and one-shot obtains run on the caller's thread, so implementations must not assume + * a particular thread. {@link #registerJob(JobID, Configuration)} and {@link #unregisterJob(JobID)} + * are invoked from the ResourceManager main thread. These can therefore run concurrently with + * {@link #obtainDelegationTokens()}. {@link * DelegationTokenManagerCallback#reobtainDelegationTokens()} may be invoked from any thread. * Implementations must keep any per-job state thread-safe, and {@code registerJob}/{@code * unregisterJob} must be non-blocking so they do not stall the ResourceManager — defer real work to diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java index a81caf54abb291..7d5468adc1e2fa 100644 --- a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java +++ b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java @@ -46,6 +46,7 @@ import java.util.Optional; import java.util.ServiceLoader; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutorService; import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.ScheduledFuture; @@ -138,9 +139,9 @@ public class DefaultDelegationTokenManager implements DelegationTokenManager { private boolean reobtainScheduled; /** - * Clock time (millis) of the last on-demand re-obtain request, used to enforce the cooldown. - * Holds {@link #NO_PREVIOUS_REOBTAIN} until the first request. Only on-demand re-obtains update - * this (not the periodic renewal), so the cooldown spaces requests, not obtain executions. + * Clock time (millis) at which the last on-demand re-obtain cycle was scheduled to execute, or + * {@link #NO_PREVIOUS_REOBTAIN}. Anchored to the execution time rather than the request time, + * so the cooldown spaces cycle executions. Updated only by on-demand re-obtains. */ @GuardedBy("tokensUpdateFutureLock") private long lastReobtainAtMillis = NO_PREVIOUS_REOBTAIN; @@ -152,7 +153,19 @@ public class DefaultDelegationTokenManager implements DelegationTokenManager { @GuardedBy("tokensUpdateFutureLock") private boolean running; - @Nullable private Listener listener; + @GuardedBy("tokensUpdateFutureLock") + @VisibleForTesting + @Nullable + Listener listener; + + /** + * Jobs for which providers may hold per-job state. A job is added on successful registration + * (or when a failed rollback left provider state behind) and removed when every provider + * unregistered it cleanly. Lets a failed re-registration keep the previous state and lets + * {@link #stop()} unregister the jobs of the ending session. All checks and updates run on the + * ResourceManager main thread, and leadership sessions are serialized. + */ + private final Set registeredJobs = ConcurrentHashMap.newKeySet(); public DefaultDelegationTokenManager( Configuration configuration, @@ -370,6 +383,14 @@ public void start(Listener listener) throws Exception { running = true; } + // A new session must not inherit the previous session's retry backoff or renewal + // deadline. obtainLock orders this reset after any still-running previous cycle. Not + // nested in the block above to keep the obtainLock -> tokensUpdateFutureLock order. + synchronized (obtainLock) { + currentRetryBackoff = renewalRetryInitialBackoff; + lastKnownNextRenewal = Long.MAX_VALUE; + } + startTokensUpdate(); } @@ -393,12 +414,26 @@ void startTokensUpdate() { Optional nextRenewal = obtainDelegationTokensAndGetNextRenewal(container); if (container.hasTokens()) { - delegationTokenReceiverRepository.onNewTokensObtained(container); + // stop() does not wait for an in-flight cycle. Re-check so a cycle resuming + // after stop() does not notify the stopped session's listener (the disposed + // ResourceManager). A stop() right after this read still lets one delivery + // through, which is benign. + final Listener currentListener; + synchronized (tokensUpdateFutureLock) { + currentListener = running ? listener : null; + } + if (currentListener != null) { + delegationTokenReceiverRepository.onNewTokensObtained(container); - LOG.info("Notifying listener about new tokens"); - checkNotNull(listener, "Listener must not be null"); - listener.onNewTokensObtained(InstantiationUtil.serializeObject(container)); - LOG.info("Listener notified successfully"); + LOG.info("Notifying listener about new tokens"); + currentListener.onNewTokensObtained( + InstantiationUtil.serializeObject(container)); + LOG.info("Listener notified successfully"); + } else { + LOG.info( + "Manager stopped while the tokens were being obtained, skipping " + + "notifications"); + } } else { LOG.warn("No tokens obtained so skipping notifications"); } @@ -512,8 +547,9 @@ long maybeScheduleRenewal(long delayMs) { long pendingInMillis = Math.max(0L, nextScheduledAtMillis - clock.millis()); if (delayMs < pendingInMillis) { // Bring the pending on-demand cycle forward. scheduleRenewalLocked() leaves - // reobtainScheduled set, so coalescing still holds and the earlier cycle - // serves the coalesced on-demand requests too. + // reobtainScheduled set, so coalescing still holds. Move the cooldown anchor + // to the time the cycle now actually runs. + lastReobtainAtMillis = clock.millis() + delayMs; scheduleRenewalLocked(delayMs); return delayMs; } @@ -578,7 +614,10 @@ void setClock(Clock clock) { this.clock = clock; } - /** Stops re-occurring token obtain task. */ + /** + * Stops the re-occurring token obtain task, releases the listener, and unregisters the jobs of + * the ending session. See the interface javadoc. + */ @Override public void stop() { LOG.info("Stopping credential renewal"); @@ -591,6 +630,21 @@ public void stop() { stopTokensUpdate(); reobtainScheduled = false; lastReobtainAtMillis = NO_PREVIOUS_REOBTAIN; + // Release the listener: keeping it would pin the disposed ResourceManager of a + // revoked leadership session, forever on a standby that never regains leadership. + listener = null; + } + + // Unregister all jobs: running jobs re-register with the next session, ended jobs never + // would and their entries would leak in the providers. + for (JobID jobId : registeredJobs) { + try { + unregisterJobInternal(jobId); + } catch (Exception | LinkageError e) { + // Guards the cleanup against pathological errors from a broken plugin's + // serviceName(). + LOG.error("Failed to unregister job {} while stopping the manager", jobId, e); + } } for (DelegationTokenProvider provider : delegationTokenProviders.values()) { @@ -610,8 +664,8 @@ public void reobtainDelegationTokens() { if (scheduledExecutor == null || ioExecutor == null) { LOG.debug( "A re-obtain of delegation tokens was requested but the manager was " - + "constructed without executors (one-shot obtain path); the " - + "request is ignored."); + + "constructed without executors (one-shot obtain path), " + + "ignoring the request."); return; } if (!running) { @@ -621,10 +675,9 @@ public void reobtainDelegationTokens() { + "request."); return; } - // Dedupe: if an on-demand re-obtain is already scheduled and has not started yet, the - // newly registered job(s) will be covered by it, so coalesce this request into it. + // An already scheduled re-obtain that has not started yet covers this request too. if (reobtainScheduled) { - LOG.debug("A re-obtain of delegation tokens is already scheduled; coalescing."); + LOG.debug("A re-obtain of delegation tokens is already scheduled, coalescing."); return; } // Cooldown: bound how often on-demand re-obtains can run by deferring this cycle until @@ -634,20 +687,20 @@ public void reobtainDelegationTokens() { lastReobtainAtMillis == NO_PREVIOUS_REOBTAIN ? 0L : Math.max(0L, lastReobtainAtMillis + reobtainCooldownMillis - now); - // Only bring the next cycle forward: if a cycle (e.g. the periodic renewal) is still - // pending and scheduled to fire sooner than the cooldown-deferred time, fire at that - // earlier time instead of pushing it later — otherwise a short-lived token could expire - // before it is renewed. The nextScheduledAtMillis > now guard skips an already-fired - // future that has not yet been rescheduled, so this never bypasses the cooldown. + // Only bring the next cycle forward, never push a pending cycle later, or a + // short-lived token could expire before it is renewed. The nextScheduledAtMillis > + // now guard skips an already-fired future, so this never bypasses the cooldown. if (tokensUpdateFuture != null && nextScheduledAtMillis > now && nextScheduledAtMillis - now < delayMillis) { delayMillis = nextScheduledAtMillis - now; } - lastReobtainAtMillis = now; + // Anchor the cooldown to when the cycle will run, not to this request, so a request + // arriving right after a deferred cycle fired cannot run a second cycle back to back. + lastReobtainAtMillis = now + delayMillis; reobtainScheduled = true; LOG.debug( - "Re-obtain of delegation tokens requested; scheduling an obtain cycle in {}", + "Re-obtain of delegation tokens requested, scheduling an obtain cycle in {}", TimeUtils.formatWithHighestUnit(Duration.ofMillis(delayMillis))); scheduleRenewalLocked(delayMillis); } @@ -655,36 +708,70 @@ public void reobtainDelegationTokens() { @Override public void registerJob(JobID jobId, Configuration jobConfiguration) throws Exception { + // Hand providers a copy so plugin code cannot mutate the caller's live job configuration. + // clone() locks the backing map. Like the copy constructor, the copy is shallow. + final Configuration providerJobConfiguration = jobConfiguration.clone(); + final boolean previouslyRegistered = registeredJobs.contains(jobId); DelegationTokenProvider failedProvider = null; try { for (DelegationTokenProvider provider : delegationTokenProviders.values()) { failedProvider = provider; - provider.registerJob(jobId, jobConfiguration); + provider.registerJob(jobId, providerJobConfiguration); } + registeredJobs.add(jobId); } catch (Exception | LinkageError e) { - // Roll the job back from all providers (unregisterJob is idempotent) and rethrow. - // The rollback must never mask the original failure. LinkageError is included - // because provider plugin code can fail class resolution. - try { - unregisterJob(jobId); - } catch (Exception | LinkageError rollbackException) { - LOG.error("Failed to roll back registration of job {}", jobId, rollbackException); + // LinkageError is included because provider plugin code can fail class resolution. + if (previouslyRegistered) { + // A failed re-registration must not roll back: the job registered successfully + // before and its tasks may still be running. + LOG.error( + "Failed to re-register job {} for provider {}, keeping the previous " + + "registration", + jobId, + failedProvider == null ? "" : failedProvider.serviceName(), + e); + } else { + // First registration: roll back from all providers (unregisterJob is idempotent). + // The rollback must never mask the original failure. + try { + if (!unregisterJobInternal(jobId)) { + // Keep the job tracked so stop() or a registration retry can release the + // provider state left behind. + registeredJobs.add(jobId); + } + } catch (Exception | LinkageError rollbackException) { + LOG.error( + "Failed to roll back registration of job {}", jobId, rollbackException); + } + LOG.error( + "Failed to register job {} for provider {}", + jobId, + failedProvider == null ? "" : failedProvider.serviceName(), + e); } - LOG.error( - "Failed to register job {} for provider {}", - jobId, - failedProvider == null ? "" : failedProvider.serviceName(), - e); throw e; } } @Override public void unregisterJob(JobID jobId) throws Exception { + unregisterJobInternal(jobId); + } + + /** + * Unregisters the job from all providers, swallowing per-provider failures. The job leaves + * {@link #registeredJobs} only when every provider unregistered cleanly, so state a failed + * provider may still hold stays tracked for another attempt. + * + * @return whether every provider unregistered the job without failure. + */ + private boolean unregisterJobInternal(JobID jobId) { + boolean fullyUnregistered = true; for (DelegationTokenProvider provider : delegationTokenProviders.values()) { try { provider.unregisterJob(jobId); } catch (Exception | LinkageError e) { + fullyUnregistered = false; LOG.error( "Failed to unregister job {} for provider {}", jobId, @@ -692,5 +779,9 @@ public void unregisterJob(JobID jobId) throws Exception { e); } } + if (fullyUnregistered) { + registeredJobs.remove(jobId); + } + return fullyUnregistered; } } diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DelegationTokenManager.java b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DelegationTokenManager.java index 19277db8e1e2e8..38539f13d5c406 100644 --- a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DelegationTokenManager.java +++ b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DelegationTokenManager.java @@ -61,7 +61,12 @@ interface Listener { */ void start(Listener listener) throws Exception; - /** Stops re-occurring token obtain task. */ + /** + * Stops the re-occurring token obtain task. Implementations also release any per-job provider + * state accumulated through {@link #registerJob(JobID, Configuration)}, so stale registrations + * cannot outlive the stop (a job that is still running re-registers through the normal + * JobMaster registration retry). + */ void stop(); /** @@ -78,10 +83,11 @@ default void reobtainDelegationTokens() {} /** * Called when a job has started. Fans the event out to all loaded {@link - * org.apache.flink.core.security.token.DelegationTokenProvider}s. On failure the job is - * unregistered from all providers and the exception is rethrown so the caller can reject the - * job's registration. A provider that needs the new job's tokens distributed immediately - * requests it via {@link + * org.apache.flink.core.security.token.DelegationTokenProvider}s. On failure of the job's first + * registration, the job is unregistered from all providers and the exception is rethrown so the + * caller can reject the registration. A failed re-registration rethrows but keeps the job + * registered, so a running job's tokens are not dropped. A provider that needs the new job's + * tokens distributed immediately requests it via {@link * org.apache.flink.core.security.token.DelegationTokenManagerCallback#reobtainDelegationTokens()}. * * @param jobId The job id which just started. @@ -91,7 +97,8 @@ default void registerJob(JobID jobId, Configuration jobConfiguration) throws Exc /** * Called when a job is being removed. Fans the event out to all loaded providers. Must be - * idempotent. + * idempotent. Per-provider failures are caught and logged (one provider's failure does not + * abort cleanup of the others), so in practice this does not throw for provider failures. * * @param jobId The job id of the job. */ diff --git a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java index f4bd99527adbdd..a7b0b86d4228e6 100644 --- a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java +++ b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java @@ -62,6 +62,7 @@ import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -372,7 +373,7 @@ public void stopShouldStopProviders() { } @Test - public void registerJobShouldRollBackAndRethrowWhenProviderThrows() throws Exception { + public void failedFirstRegistrationMustRethrowWithoutTouchingOtherJobs() throws Exception { Configuration configuration = new Configuration(); DefaultDelegationTokenManager delegationTokenManager = new DefaultDelegationTokenManager(configuration, null, null, null); @@ -381,13 +382,19 @@ public void registerJobShouldRollBackAndRethrowWhenProviderThrows() throws Excep delegationTokenManager.registerJob(jobId, new Configuration()); assertEquals(1, ExceptionThrowingDelegationTokenProvider.registeredJobs.get().size()); - // A provider that throws during registration must cause the job to be unregistered from - // all providers and the exception to be rethrown. + // A provider that throws during the FIRST registration of a job must cause that job to + // be unregistered from all providers and the exception to be rethrown, without touching + // other jobs' state. (A failed RE-registration keeps the previous registration instead, + // see failedReregistrationMustNotWipePreviousRegistration.) ExceptionThrowingDelegationTokenProvider.throwInRegister.set(true); + JobID otherJobId = JobID.generate(); assertThrows( IllegalArgumentException.class, - () -> delegationTokenManager.registerJob(jobId, new Configuration())); - assertEquals(0, ExceptionThrowingDelegationTokenProvider.registeredJobs.get().size()); + () -> delegationTokenManager.registerJob(otherJobId, new Configuration())); + assertEquals(1, ExceptionThrowingDelegationTokenProvider.registeredJobs.get().size()); + assertTrue( + ExceptionThrowingDelegationTokenProvider.registeredJobs.get().contains(jobId), + "A failed registration of another job must not affect this job's state"); } @Test @@ -872,6 +879,323 @@ protected Optional obtainDelegationTokensAndGetNextRenewal( } } + @Test + public void failedReregistrationMustNotWipePreviousRegistration() throws Exception { + DefaultDelegationTokenManager delegationTokenManager = + new DefaultDelegationTokenManager(new Configuration(), null, null, null); + + // The job registers successfully when it starts... + JobID jobId = JobID.generate(); + delegationTokenManager.registerJob(jobId, new Configuration()); + assertEquals(1, ExceptionThrowingDelegationTokenProvider.registeredJobs.get().size()); + + // ...and later re-registers while still running (e.g. after a JobMaster<->RM heartbeat + // timeout). A transient provider failure during the re-registration must not roll back + // the per-job state the earlier successful registration established, or obtain cycles + // would broadcast token sets missing the running job until a registration retry + // succeeds. + ExceptionThrowingDelegationTokenProvider.throwInRegister.set(true); + assertThrows( + IllegalArgumentException.class, + () -> delegationTokenManager.registerJob(jobId, new Configuration())); + + assertEquals( + 1, + ExceptionThrowingDelegationTokenProvider.registeredJobs.get().size(), + "A failed re-registration must keep the previous registration intact"); + } + + @Test + public void stopShouldUnregisterAllRegisteredJobs() throws Exception { + DefaultDelegationTokenManager delegationTokenManager = + new DefaultDelegationTokenManager(new Configuration(), null, null, null); + + delegationTokenManager.registerJob(JobID.generate(), new Configuration()); + delegationTokenManager.registerJob(JobID.generate(), new Configuration()); + assertEquals(2, ExceptionThrowingDelegationTokenProvider.registeredJobs.get().size()); + + // stop() ends the manager's (leadership) session. Jobs still running re-register with + // the next session (registerJob is idempotent by contract), while jobs that reached a + // terminal state when no session was active never re-register, and their per-job provider + // state must be released here or it leaks for the process lifetime. + delegationTokenManager.stop(); + + assertEquals( + 0, + ExceptionThrowingDelegationTokenProvider.registeredJobs.get().size(), + "stop() must release the per-job provider state of every registered job"); + } + + @Test + public void leftoverRollbackStateMustBeReleasedByStop() throws Exception { + DefaultDelegationTokenManager delegationTokenManager = + new DefaultDelegationTokenManager(new Configuration(), null, null, null); + + // A FIRST registration fails after the provider recorded state (add-then-throw), and + // the rollback's unregister fails too: the state is left behind in the provider. + ExceptionThrowingDelegationTokenProvider.throwErrorInRegister.set(true); + ExceptionThrowingDelegationTokenProvider.throwInUnregister.set(true); + JobID jobId = JobID.generate(); + assertThrows( + NoClassDefFoundError.class, + () -> delegationTokenManager.registerJob(jobId, new Configuration())); + assertEquals(1, ExceptionThrowingDelegationTokenProvider.registeredJobs.get().size()); + + // The job must have stayed tracked despite the failed rollback, so stop() releases the + // leftover state once the provider recovers. + ExceptionThrowingDelegationTokenProvider.throwErrorInRegister.set(false); + ExceptionThrowingDelegationTokenProvider.throwInUnregister.set(false); + delegationTokenManager.stop(); + assertEquals( + 0, + ExceptionThrowingDelegationTokenProvider.registeredJobs.get().size(), + "State left behind by a failed rollback must be released by stop()"); + } + + @Test + public void stopMustRetryFailedUnregistration() throws Exception { + DefaultDelegationTokenManager delegationTokenManager = + new DefaultDelegationTokenManager(new Configuration(), null, null, null); + + JobID jobId = JobID.generate(); + delegationTokenManager.registerJob(jobId, new Configuration()); + assertEquals(1, ExceptionThrowingDelegationTokenProvider.registeredJobs.get().size()); + + // A provider fails its unregistration (swallowed by contract), so its per-job state + // survives. The manager must keep tracking the job instead of forgetting it. + ExceptionThrowingDelegationTokenProvider.throwInUnregister.set(true); + delegationTokenManager.unregisterJob(jobId); + assertEquals(1, ExceptionThrowingDelegationTokenProvider.registeredJobs.get().size()); + + // Once the provider recovers, stop() gets another attempt, without the retry + // the job's state would leak in the process-lifetime provider until process shutdown. + ExceptionThrowingDelegationTokenProvider.throwInUnregister.set(false); + delegationTokenManager.stop(); + assertEquals( + 0, + ExceptionThrowingDelegationTokenProvider.registeredJobs.get().size(), + "A job whose unregistration failed must be released by stop()"); + } + + @Test + public void cooldownMustSpaceObtainCycleExecutionsNotRequests() throws Exception { + final ManuallyTriggeredScheduledExecutor scheduledExecutor = + new ManuallyTriggeredScheduledExecutor(); + final ManuallyTriggeredScheduledExecutorService scheduler = + new ManuallyTriggeredScheduledExecutorService(); + + DefaultDelegationTokenManager delegationTokenManager = + new DefaultDelegationTokenManager( + hermeticCooldownConfig(Duration.ofMillis(60_000)), + null, + scheduledExecutor, + scheduler); + + long t0 = 1_000_000L; + delegationTokenManager.setClock(Clock.fixed(ofEpochMilli(t0), ZoneId.systemDefault())); + delegationTokenManager.start(tokens -> {}); + + // The first request runs immediately. + delegationTokenManager.reobtainDelegationTokens(); + assertEquals(0L, onlyScheduledDelayMillis(scheduledExecutor)); + scheduledExecutor.triggerScheduledTasks(); + scheduler.triggerAll(); + + // A request at t0+1s is deferred by 59s: its obtain cycle runs at t0+60s. + delegationTokenManager.setClock( + Clock.fixed(ofEpochMilli(t0 + 1_000L), ZoneId.systemDefault())); + delegationTokenManager.reobtainDelegationTokens(); + assertEquals(59_000L, onlyScheduledDelayMillis(scheduledExecutor)); + delegationTokenManager.setClock( + Clock.fixed(ofEpochMilli(t0 + 60_000L), ZoneId.systemDefault())); + scheduledExecutor.triggerScheduledTasks(); + scheduler.triggerAll(); + + // The option documents a minimum time between two consecutive on-demand obtain CYCLES, + // so a request arriving just after the deferred cycle ran must be deferred by a full + // cooldown measured from that cycle's execution, not run (almost) immediately because + // the previous REQUEST arrived one cooldown ago. + delegationTokenManager.setClock( + Clock.fixed(ofEpochMilli(t0 + 61_000L), ZoneId.systemDefault())); + delegationTokenManager.reobtainDelegationTokens(); + assertEquals( + 59_000L, + onlyScheduledDelayMillis(scheduledExecutor), + "The cooldown must space obtain cycle executions, not requests"); + } + + @Test + public void broughtForwardReobtainMustMoveCooldownAnchor() throws Exception { + final ManuallyTriggeredScheduledExecutor scheduledExecutor = + new ManuallyTriggeredScheduledExecutor(); + final ManuallyTriggeredScheduledExecutorService scheduler = + new ManuallyTriggeredScheduledExecutorService(); + + DefaultDelegationTokenManager delegationTokenManager = + new DefaultDelegationTokenManager( + hermeticCooldownConfig(Duration.ofMillis(60_000)), + null, + scheduledExecutor, + scheduler); + + long t0 = 1_000_000L; + delegationTokenManager.setClock(Clock.fixed(ofEpochMilli(t0), ZoneId.systemDefault())); + delegationTokenManager.start(tokens -> {}); + + delegationTokenManager.reobtainDelegationTokens(); + scheduledExecutor.triggerScheduledTasks(); + scheduler.triggerAll(); + + // 10s later a second request is cooldown-deferred by 50s (would run at t0+60s). + delegationTokenManager.setClock( + Clock.fixed(ofEpochMilli(t0 + 10_000L), ZoneId.systemDefault())); + delegationTokenManager.reobtainDelegationTokens(); + assertEquals(50_000L, onlyScheduledDelayMillis(scheduledExecutor)); + + // A completed cycle (renewal or failure retry) brings the pending on-demand cycle + // forward to +5s, so the coalesced cycle actually executes at t0+15s. + delegationTokenManager.maybeScheduleRenewal(5_000L); + assertEquals(5_000L, onlyScheduledDelayMillis(scheduledExecutor)); + delegationTokenManager.setClock( + Clock.fixed(ofEpochMilli(t0 + 15_000L), ZoneId.systemDefault())); + scheduledExecutor.triggerScheduledTasks(); + scheduler.triggerAll(); + + // The next request must measure its cooldown from the brought-forward execution + // (t0+15s), not from the originally scheduled t0+60s. Otherwise it would defer + // beyond a full cooldown (104s here instead of 59s). + delegationTokenManager.setClock( + Clock.fixed(ofEpochMilli(t0 + 16_000L), ZoneId.systemDefault())); + delegationTokenManager.reobtainDelegationTokens(); + assertEquals( + 59_000L, + onlyScheduledDelayMillis(scheduledExecutor), + "The cooldown anchor must follow a brought-forward on-demand cycle"); + } + + @Test + public void startAfterStopMustResetRetryState() throws Exception { + final ManuallyTriggeredScheduledExecutor scheduledExecutor = + new ManuallyTriggeredScheduledExecutor(); + final ManuallyTriggeredScheduledExecutorService scheduler = + new ManuallyTriggeredScheduledExecutorService(); + + Configuration configuration = hermeticCooldownConfig(Duration.ofMillis(60_000)); + configuration.set(DELEGATION_TOKENS_RENEWAL_RETRY_INITIAL_BACKOFF, Duration.ofSeconds(1)); + configuration.set(DELEGATION_TOKENS_RENEWAL_RETRY_MAX_BACKOFF, Duration.ofSeconds(64)); + DefaultDelegationTokenManager delegationTokenManager = + new DefaultDelegationTokenManager( + configuration, null, scheduledExecutor, scheduler); + + delegationTokenManager.start(tokens -> {}); + // The session escalates the retry state through repeated obtain failures. + delegationTokenManager.currentRetryBackoff = Duration.ofSeconds(64).toMillis(); + delegationTokenManager.lastKnownNextRenewal = 123L; + + // The manager instance is reused across leadership sessions: the next session must + // start from the configured initial backoff instead of inheriting the previous + // session's increased one (which would delay token recovery by up to the max backoff). + delegationTokenManager.stop(); + delegationTokenManager.start(tokens -> {}); + + assertEquals( + Duration.ofSeconds(1).toMillis(), + delegationTokenManager.currentRetryBackoff, + "A new session must start from the initial retry backoff"); + assertEquals( + Long.MAX_VALUE, + delegationTokenManager.lastKnownNextRenewal, + "A new session must not inherit the previous session's renewal deadline"); + } + + @Test + public void inFlightCycleMustNotNotifyListenerAfterStop() throws Exception { + final ManuallyTriggeredScheduledExecutor scheduledExecutor = + new ManuallyTriggeredScheduledExecutor(); + final ManuallyTriggeredScheduledExecutorService scheduler = + new ManuallyTriggeredScheduledExecutorService(); + + // The "throw" provider stays enabled so its RECEIVER is loaded: the token added below + // must have a live receiver, or a regression that removes the broadcast gate would hide + // behind the missing-receiver IllegalStateException (swallowed by the cycle's catch) and + // this test would pass vacuously. The overridden obtain below bypasses the providers, so + // the throw provider itself never runs. + Configuration configuration = new Configuration(); + configuration.set(getBooleanConfigOption(CONFIG_PREFIX + ".hadoopfs.enabled"), false); + configuration.set(getBooleanConfigOption(CONFIG_PREFIX + ".hbase.enabled"), false); + + final CountDownLatch cycleInObtain = new CountDownLatch(1); + final CountDownLatch resumeObtain = new CountDownLatch(1); + DefaultDelegationTokenManager delegationTokenManager = + new DefaultDelegationTokenManager( + configuration, null, scheduledExecutor, scheduler) { + @Override + protected Optional obtainDelegationTokensAndGetNextRenewal( + DelegationTokenContainer container) { + // Produce a token so the broadcast path is reached, then park until the + // test has run stop(), simulating slow provider I/O overlapping shutdown. + container.addToken("throw", new byte[] {1}); + cycleInObtain.countDown(); + try { + resumeObtain.await(10, TimeUnit.SECONDS); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + return Optional.empty(); + } + }; + + AtomicInteger listenerNotifications = new AtomicInteger(0); + // start() runs the first obtain cycle inline, so it parks in the obtain above. + Thread starter = + new Thread( + () -> { + try { + delegationTokenManager.start( + tokens -> listenerNotifications.incrementAndGet()); + } catch (Exception e) { + throw new RuntimeException(e); + } + }); + starter.start(); + assertTrue(cycleInObtain.await(10, TimeUnit.SECONDS)); + + // stop() does not wait for the in-flight cycle. Once that cycle resumes it must notice + // the manager stopped: the stopped session's listener must not be notified, and the + // manager must not keep referencing it (it is the disposed ResourceManager). + delegationTokenManager.stop(); + resumeObtain.countDown(); + starter.join(10_000L); + assertFalse(starter.isAlive()); + + assertEquals( + 0, + listenerNotifications.get(), + "An obtain cycle finishing after stop() must not notify the stopped session's" + + " listener"); + assertNull(delegationTokenManager.listener, "stop() must release the listener reference"); + } + + @Test + public void registerJobMustNotExposeCallersConfigurationToProviders() throws Exception { + DefaultDelegationTokenManager delegationTokenManager = + new DefaultDelegationTokenManager(new Configuration(), null, null, null); + + // The caller's configuration object is the live job configuration (the ExecutionPlan's), + // which reaches the manager by reference over local RPC. Providers are plugin code and + // must receive a copy: a provider mutating it must not corrupt the runtime's state. + ExceptionThrowingDelegationTokenProvider.mutateJobConfiguration.set(true); + Configuration callerConfiguration = new Configuration(); + JobID jobId = JobID.generate(); + delegationTokenManager.registerJob(jobId, callerConfiguration); + + assertTrue(ExceptionThrowingDelegationTokenProvider.registeredJobs.get().contains(jobId)); + assertFalse( + callerConfiguration.containsKey( + ExceptionThrowingDelegationTokenProvider.MUTATED_KEY), + "A provider-side mutation must not be visible in the caller's configuration"); + } + /** * Configuration for cooldown-scheduling tests: sets the cooldown and disables all providers * that could fail the obtain cycle (hadoopfs/hbase need a real Hadoop setup, and the throw diff --git a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/ExceptionThrowingDelegationTokenProvider.java b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/ExceptionThrowingDelegationTokenProvider.java index 69ecff074cab85..644101c45b23bd 100644 --- a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/ExceptionThrowingDelegationTokenProvider.java +++ b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/ExceptionThrowingDelegationTokenProvider.java @@ -20,6 +20,7 @@ import org.apache.flink.api.common.JobID; import org.apache.flink.configuration.Configuration; +import org.apache.flink.configuration.ConfigurationUtils; import org.apache.flink.core.security.token.DelegationTokenManagerCallback; import org.apache.flink.core.security.token.DelegationTokenProvider; @@ -32,6 +33,9 @@ */ public class ExceptionThrowingDelegationTokenProvider implements DelegationTokenProvider { + /** Key written into the job configuration when {@link #mutateJobConfiguration} is set. */ + public static final String MUTATED_KEY = "test.mutated.by.provider"; + public static volatile ThreadLocal throwInInit = ThreadLocal.withInitial(() -> Boolean.FALSE); public static volatile ThreadLocal throwInUsage = @@ -52,6 +56,8 @@ public class ExceptionThrowingDelegationTokenProvider implements DelegationToken ThreadLocal.withInitial(() -> Boolean.FALSE); public static volatile ThreadLocal stopped = ThreadLocal.withInitial(() -> Boolean.FALSE); + public static volatile ThreadLocal mutateJobConfiguration = + ThreadLocal.withInitial(() -> Boolean.FALSE); public static volatile ThreadLocal> registeredJobs = ThreadLocal.withInitial(HashSet::new); @@ -66,6 +72,7 @@ public static void reset() { throwInUnregister.set(false); throwErrorInUnregister.set(false); stopped.set(false); + mutateJobConfiguration.set(false); registeredJobs.get().clear(); } @@ -118,6 +125,9 @@ public void registerJob(JobID jobId, Configuration jobConfiguration) { if (throwInRegister.get()) { throw new IllegalArgumentException(); } + if (mutateJobConfiguration.get()) { + jobConfiguration.set(ConfigurationUtils.getBooleanConfigOption(MUTATED_KEY), true); + } registeredJobs.get().add(jobId); if (throwErrorInRegister.get()) { throw new NoClassDefFoundError("simulated classpath failure in provider registerJob"); From 35495a68533d5da7f78ea83e2dec2ba077dd8c66 Mon Sep 17 00:00:00 2001 From: Aleksandr Savonin Date: Tue, 21 Jul 2026 17:41:35 +0200 Subject: [PATCH 07/22] [FLINK-40019][core][runtime] Stop delegation token providers once at process shutdown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DelegationTokenManager.close() is the terminal teardown: it ends any active session via stop() and then stops all providers, exactly once, and a closed manager rejects a later start(). It is called by the component that created the manager: ClusterEntrypoint, MiniCluster, and YarnClusterDescriptor's one-shot client-side obtain. stop() stays session-scoped (cancel scheduling, release the listener, drain job registrations) and keeps the providers usable for the next ResourceManager leadership session, so the provider stop() javadoc — called at most once, at process shutdown, never on leadership changes — holds as written. --- .../token/DelegationTokenProvider.java | 10 ++- .../runtime/entrypoint/ClusterEntrypoint.java | 10 +++ .../runtime/minicluster/MiniCluster.java | 11 +++ .../token/DefaultDelegationTokenManager.java | 32 +++++++- .../token/DelegationTokenManager.java | 17 ++++- .../DefaultDelegationTokenManagerTest.java | 74 ++++++++++++++++++- ...eptionThrowingDelegationTokenProvider.java | 3 + .../flink/yarn/YarnClusterDescriptor.java | 7 +- 8 files changed, 148 insertions(+), 16 deletions(-) diff --git a/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenProvider.java b/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenProvider.java index e884ed49c5d526..a5dcd4eba37019 100644 --- a/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenProvider.java +++ b/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenProvider.java @@ -163,10 +163,12 @@ default void unregisterJob(JobID jobId) {} /** * Stops the provider. Any resources should be closed. * - *

Called once during manager shutdown. Note that an obtain-and-broadcast cycle started just - * before shutdown may still be running on another thread when this is invoked, so {@code - * stop()} may overlap an in-flight {@link #obtainDelegationTokens()}; implementations must - * release resources in a way that is safe with respect to that overlap. + *

Called at most once, when the manager is closed at process shutdown. It is not called on + * ResourceManager leadership changes, those only stop and restart the manager's obtain session + * and the provider instance stays in use. An obtain cycle started just before shutdown may + * still be running, so {@code stop()} may overlap an in-flight {@link + * #obtainDelegationTokens()} and implementations must release resources in a way that is safe + * with respect to that overlap. */ default void stop() {} } diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/entrypoint/ClusterEntrypoint.java b/flink-runtime/src/main/java/org/apache/flink/runtime/entrypoint/ClusterEntrypoint.java index dca34f97720dc5..a631e79714527f 100755 --- a/flink-runtime/src/main/java/org/apache/flink/runtime/entrypoint/ClusterEntrypoint.java +++ b/flink-runtime/src/main/java/org/apache/flink/runtime/entrypoint/ClusterEntrypoint.java @@ -498,6 +498,16 @@ protected CompletableFuture stopClusterServices(boolean cleanupHaData) { final Collection> terminationFutures = new ArrayList<>(3); + if (delegationTokenManager != null) { + try { + // Terminal teardown of the delegation token providers. The per-session + // manager stop() already ran when the ResourceManager component closed. + delegationTokenManager.close(); + } catch (Throwable t) { + exception = ExceptionUtils.firstOrSuppressed(t, exception); + } + } + if (blobServer != null) { try { blobServer.close(); diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/minicluster/MiniCluster.java b/flink-runtime/src/main/java/org/apache/flink/runtime/minicluster/MiniCluster.java index 96cd052698acd8..9eee7f30685951 100644 --- a/flink-runtime/src/main/java/org/apache/flink/runtime/minicluster/MiniCluster.java +++ b/flink-runtime/src/main/java/org/apache/flink/runtime/minicluster/MiniCluster.java @@ -1372,6 +1372,17 @@ private void terminateMiniClusterServices(boolean cleanupHaData) throws Exceptio Exception exception = null; synchronized (lock) { + if (delegationTokenManager != null) { + try { + // Terminal teardown of the delegation token providers. The per-session + // manager stop() already ran when the ResourceManager component closed. + delegationTokenManager.close(); + } catch (Exception e) { + exception = ExceptionUtils.firstOrSuppressed(e, exception); + } + delegationTokenManager = null; + } + if (blobCacheService != null) { try { blobCacheService.close(); diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java index 7d5468adc1e2fa..33811219030b7b 100644 --- a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java +++ b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java @@ -52,6 +52,7 @@ import java.util.concurrent.ScheduledFuture; import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Consumer; import java.util.stream.Stream; @@ -167,6 +168,12 @@ public class DefaultDelegationTokenManager implements DelegationTokenManager { */ private final Set registeredJobs = ConcurrentHashMap.newKeySet(); + /** + * Set once by {@link #close()}. Keeps provider stop() at most once and rejects any later {@link + * #start(Listener)}. + */ + private final AtomicBoolean closed = new AtomicBoolean(false); + public DefaultDelegationTokenManager( Configuration configuration, @Nullable PluginManager pluginManager, @@ -369,6 +376,9 @@ protected Optional obtainDelegationTokensAndGetNextRenewal( */ @Override public void start(Listener listener) throws Exception { + checkState( + !closed.get(), + "The delegation token manager is already closed, its providers are stopped"); checkNotNull(scheduledExecutor, "Scheduled executor must not be null"); checkNotNull(ioExecutor, "IO executor must not be null"); checkNotNull(listener, "Listener must not be null"); @@ -616,7 +626,8 @@ void setClock(Clock clock) { /** * Stops the re-occurring token obtain task, releases the listener, and unregisters the jobs of - * the ending session. See the interface javadoc. + * the ending session. Providers stay usable for a later {@link #start(Listener)}. Their + * teardown happens in {@link #close()}. */ @Override public void stop() { @@ -647,6 +658,23 @@ public void stop() { } } + LOG.info("Stopped credential renewal"); + } + + /** + * Terminal teardown: ends any active session via {@link #stop()} and then stops all providers, + * exactly once. Called by the component that created the manager at process shutdown, not on + * ResourceManager leadership changes. + */ + @Override + public void close() { + // Flip the flag before stopping anything: a concurrent start() then fails its + // !closed check instead of slipping in between the session stop and the provider + // teardown and running on providers that get stopped underneath it. + if (!closed.compareAndSet(false, true)) { + return; + } + stop(); for (DelegationTokenProvider provider : delegationTokenProviders.values()) { try { provider.stop(); @@ -654,8 +682,6 @@ public void stop() { LOG.error("Failed to stop delegation token provider {}", provider.serviceName(), t); } } - - LOG.info("Stopped credential renewal"); } @Override diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DelegationTokenManager.java b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DelegationTokenManager.java index 38539f13d5c406..c3130ad829c3c7 100644 --- a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DelegationTokenManager.java +++ b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DelegationTokenManager.java @@ -62,13 +62,22 @@ interface Listener { void start(Listener listener) throws Exception; /** - * Stops the re-occurring token obtain task. Implementations also release any per-job provider - * state accumulated through {@link #registerJob(JobID, Configuration)}, so stale registrations - * cannot outlive the stop (a job that is still running re-registers through the normal - * JobMaster registration retry). + * Stops the re-occurring token obtain task. Implementations also unregister all jobs registered + * through {@link #registerJob(JobID, Configuration)}, so nothing leaks across leadership + * sessions (a job that is still running re-registers through the normal JobMaster registration + * retry). Providers are not stopped here and stay usable for a subsequent {@link + * #start(Listener)}. Their teardown happens in {@link #close()}. */ void stop(); + /** + * Terminal teardown of the manager: ends any active obtain session and releases the providers' + * resources, exactly once. Called by the component that created the manager at process + * shutdown, unlike {@link #stop()}, which may run once per ResourceManager leadership session. + * The manager must not be started after close. + */ + default void close() {} + /** * Requests an immediate, asynchronous token-obtain-and-distribute cycle, bringing the next * cycle forward instead of waiting for the periodic renewal. May be called from any thread. It diff --git a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java index a7b0b86d4228e6..d5c85302331401 100644 --- a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java +++ b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java @@ -362,14 +362,53 @@ void startTokensUpdate() { } @Test - public void stopShouldStopProviders() { - Configuration configuration = new Configuration(); + public void closeShouldStopProvidersExactlyOnce() { DefaultDelegationTokenManager delegationTokenManager = - new DefaultDelegationTokenManager(configuration, null, null, null); + new DefaultDelegationTokenManager(new Configuration(), null, null, null); - delegationTokenManager.stop(); + // close() is the terminal teardown: it stops the providers, and a repeated close() must + // not stop them again (the SPI promises stop() is called at most once). + delegationTokenManager.close(); + delegationTokenManager.close(); assertTrue(ExceptionThrowingDelegationTokenProvider.stopped.get()); + assertEquals(1, (int) ExceptionThrowingDelegationTokenProvider.stopCallCount.get()); + } + + @Test + public void closeShouldEndSessionAndUnregisterJobs() throws Exception { + DefaultDelegationTokenManager delegationTokenManager = + new DefaultDelegationTokenManager(new Configuration(), null, null, null); + + JobID jobId = JobID.generate(); + delegationTokenManager.registerJob(jobId, new Configuration()); + + // close() ends the session first, so the jobs are unregistered while the providers are + // still usable, and only then are the providers stopped. + delegationTokenManager.close(); + + assertEquals(0, ExceptionThrowingDelegationTokenProvider.registeredJobs.get().size()); + assertTrue(ExceptionThrowingDelegationTokenProvider.stopped.get()); + } + + @Test + public void startAfterCloseMustFail() { + final ManuallyTriggeredScheduledExecutor scheduledExecutor = + new ManuallyTriggeredScheduledExecutor(); + final ManuallyTriggeredScheduledExecutorService scheduler = + new ManuallyTriggeredScheduledExecutorService(); + + DefaultDelegationTokenManager delegationTokenManager = + new DefaultDelegationTokenManager( + hermeticCooldownConfig(Duration.ofMillis(60_000)), + null, + scheduledExecutor, + scheduler); + delegationTokenManager.close(); + + // A closed manager's providers are stopped for good: starting it would run obtain + // cycles against dead providers, so it must fail fast. + assertThrows(IllegalStateException.class, () -> delegationTokenManager.start(tokens -> {})); } @Test @@ -713,6 +752,33 @@ public void execute(Runnable command) { "A re-obtain after a scheduler failure must schedule a fresh obtain cycle"); } + @Test + public void stopShouldKeepProvidersUsableForSubsequentStart() throws Exception { + final ManuallyTriggeredScheduledExecutor scheduledExecutor = + new ManuallyTriggeredScheduledExecutor(); + final ManuallyTriggeredScheduledExecutorService scheduler = + new ManuallyTriggeredScheduledExecutorService(); + + DefaultDelegationTokenManager delegationTokenManager = + new DefaultDelegationTokenManager( + new Configuration(), null, scheduledExecutor, scheduler); + + // The manager is a process-lifetime singleton reused across ResourceManager leadership + // sessions: stop() runs on every leadership revoke and start() on the next grant, with + // the same provider instances. Providers are init()-ed exactly once, in the manager + // constructor, and the SPI has no re-init hook, so a provider closed by stop() stays + // broken for every following term. Providers must therefore only be closed at genuine + // process shutdown, not by the per-session stop(). + delegationTokenManager.start(tokens -> {}); + delegationTokenManager.stop(); + delegationTokenManager.start(tokens -> {}); + + assertFalse( + ExceptionThrowingDelegationTokenProvider.stopped.get(), + "A leadership-session stop() must not close the providers, the next start()" + + " re-uses them"); + } + @Test public void startShouldBeIdempotent() throws Exception { final ManuallyTriggeredScheduledExecutor scheduledExecutor = diff --git a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/ExceptionThrowingDelegationTokenProvider.java b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/ExceptionThrowingDelegationTokenProvider.java index 644101c45b23bd..d6e91ffaf272e6 100644 --- a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/ExceptionThrowingDelegationTokenProvider.java +++ b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/ExceptionThrowingDelegationTokenProvider.java @@ -56,6 +56,7 @@ public class ExceptionThrowingDelegationTokenProvider implements DelegationToken ThreadLocal.withInitial(() -> Boolean.FALSE); public static volatile ThreadLocal stopped = ThreadLocal.withInitial(() -> Boolean.FALSE); + public static volatile ThreadLocal stopCallCount = ThreadLocal.withInitial(() -> 0); public static volatile ThreadLocal mutateJobConfiguration = ThreadLocal.withInitial(() -> Boolean.FALSE); public static volatile ThreadLocal> registeredJobs = @@ -72,6 +73,7 @@ public static void reset() { throwInUnregister.set(false); throwErrorInUnregister.set(false); stopped.set(false); + stopCallCount.set(0); mutateJobConfiguration.set(false); registeredJobs.get().clear(); } @@ -151,5 +153,6 @@ public void unregisterJob(JobID jobId) { @Override public void stop() { stopped.set(true); + stopCallCount.set(stopCallCount.get() + 1); } } diff --git a/flink-yarn/src/main/java/org/apache/flink/yarn/YarnClusterDescriptor.java b/flink-yarn/src/main/java/org/apache/flink/yarn/YarnClusterDescriptor.java index ad169a6f52c39b..6d63256d170bf1 100644 --- a/flink-yarn/src/main/java/org/apache/flink/yarn/YarnClusterDescriptor.java +++ b/flink-yarn/src/main/java/org/apache/flink/yarn/YarnClusterDescriptor.java @@ -1348,7 +1348,12 @@ void setTokensFor(ContainerLaunchContext containerLaunchContext, boolean fetchTo DelegationTokenManager delegationTokenManager = new DefaultDelegationTokenManager(flinkConfiguration, null, null, null); DelegationTokenContainer container = new DelegationTokenContainer(); - delegationTokenManager.obtainDelegationTokens(container); + try { + delegationTokenManager.obtainDelegationTokens(container); + } finally { + // One-shot, client-side use: release the providers' resources right away. + delegationTokenManager.close(); + } // This is here for backward compatibility to make log aggregation work for (Map.Entry e : container.getTokens().entrySet()) { From 9ce19501ff8308313a3ba5c0bbdb83243f936187 Mon Sep 17 00:00:00 2001 From: Aleksandr Savonin Date: Wed, 22 Jul 2026 16:02:41 +0200 Subject: [PATCH 08/22] [FLINK-40019][runtime] Inject Clock into DefaultDelegationTokenManager instead of a mutable test setter --- .../token/DefaultDelegationTokenManager.java | 41 +++++---- .../DefaultDelegationTokenManagerTest.java | 84 ++++++++----------- 2 files changed, 63 insertions(+), 62 deletions(-) diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java index 33811219030b7b..dbfa82410596db 100644 --- a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java +++ b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java @@ -30,6 +30,8 @@ import org.apache.flink.util.FlinkRuntimeException; import org.apache.flink.util.InstantiationUtil; import org.apache.flink.util.TimeUtils; +import org.apache.flink.util.clock.Clock; +import org.apache.flink.util.clock.SystemClock; import org.apache.flink.util.concurrent.ScheduledExecutor; import org.slf4j.Logger; @@ -38,7 +40,6 @@ import javax.annotation.Nullable; import javax.annotation.concurrent.GuardedBy; -import java.time.Clock; import java.time.Duration; import java.util.HashMap; import java.util.HashSet; @@ -102,8 +103,8 @@ public class DefaultDelegationTokenManager implements DelegationTokenManager { private final long reobtainCooldownMillis; - /** Clock used for cooldown bookkeeping; overridable in tests. */ - private volatile Clock clock = Clock.systemDefaultZone(); + /** Clock used for renewal and cooldown timing. */ + private final Clock clock; /** * Serializes the obtain-and-broadcast cycle so that, even though {@code cancel(true)} does not @@ -179,7 +180,23 @@ public DefaultDelegationTokenManager( @Nullable PluginManager pluginManager, @Nullable ScheduledExecutor scheduledExecutor, @Nullable ExecutorService ioExecutor) { + this( + configuration, + pluginManager, + scheduledExecutor, + ioExecutor, + SystemClock.getInstance()); + } + + @VisibleForTesting + DefaultDelegationTokenManager( + Configuration configuration, + @Nullable PluginManager pluginManager, + @Nullable ScheduledExecutor scheduledExecutor, + @Nullable ExecutorService ioExecutor, + Clock clock) { this.configuration = checkNotNull(configuration, "Flink configuration must not be null"); + this.clock = checkNotNull(clock, "Clock must not be null"); this.pluginManager = pluginManager; this.tokensRenewalTimeRatio = configuration.get(DELEGATION_TOKENS_RENEWAL_TIME_RATIO); this.renewalRetryInitialBackoff = @@ -501,7 +518,7 @@ void startTokensUpdate() { @GuardedBy("tokensUpdateFutureLock") private void scheduleRenewalLocked(long delayMs) { stopTokensUpdate(); - nextScheduledAtMillis = clock.millis() + delayMs; + nextScheduledAtMillis = clock.absoluteTimeMillis() + delayMs; try { tokensUpdateFuture = scheduledExecutor.schedule( @@ -554,12 +571,13 @@ long maybeScheduleRenewal(long delayMs) { return -1L; } if (reobtainScheduled) { - long pendingInMillis = Math.max(0L, nextScheduledAtMillis - clock.millis()); + long pendingInMillis = + Math.max(0L, nextScheduledAtMillis - clock.absoluteTimeMillis()); if (delayMs < pendingInMillis) { // Bring the pending on-demand cycle forward. scheduleRenewalLocked() leaves // reobtainScheduled set, so coalescing still holds. Move the cooldown anchor // to the time the cycle now actually runs. - lastReobtainAtMillis = clock.millis() + delayMs; + lastReobtainAtMillis = clock.absoluteTimeMillis() + delayMs; scheduleRenewalLocked(delayMs); return delayMs; } @@ -586,7 +604,7 @@ void stopTokensUpdate() { @VisibleForTesting long calculateRetryDelay(Clock clock) { - long nowMillis = clock.millis(); + long nowMillis = clock.absoluteTimeMillis(); long effectiveMax; if (lastKnownNextRenewal != Long.MAX_VALUE) { long remaining = lastKnownNextRenewal - nowMillis; @@ -608,7 +626,7 @@ long calculateRetryDelay(Clock clock) { @VisibleForTesting long calculateRenewalDelay(Clock clock, long nextRenewal) { - long now = clock.millis(); + long now = clock.absoluteTimeMillis(); long renewalDelay = Math.round(tokensRenewalTimeRatio * (nextRenewal - now)); LOG.debug( "Calculated delay on renewal is {}, based on next renewal {} and the ratio {}, and current time {}", @@ -619,11 +637,6 @@ long calculateRenewalDelay(Clock clock, long nextRenewal) { return renewalDelay; } - @VisibleForTesting - void setClock(Clock clock) { - this.clock = clock; - } - /** * Stops the re-occurring token obtain task, releases the listener, and unregisters the jobs of * the ending session. Providers stay usable for a later {@link #start(Listener)}. Their @@ -708,7 +721,7 @@ public void reobtainDelegationTokens() { } // Cooldown: bound how often on-demand re-obtains can run by deferring this cycle until // at least reobtainCooldownMillis have passed since the previous on-demand re-obtain. - long now = clock.millis(); + long now = clock.absoluteTimeMillis(); long delayMillis = lastReobtainAtMillis == NO_PREVIOUS_REOBTAIN ? 0L diff --git a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java index d5c85302331401..a6d627482ac3fe 100644 --- a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java +++ b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java @@ -23,6 +23,7 @@ import org.apache.flink.core.security.token.DelegationTokenProvider; import org.apache.flink.core.security.token.DelegationTokenReceiver; import org.apache.flink.core.testutils.ManuallyTriggeredScheduledExecutorService; +import org.apache.flink.util.clock.ManualClock; import org.apache.flink.util.concurrent.ManuallyTriggeredScheduledExecutor; import org.apache.flink.util.concurrent.ScheduledExecutor; @@ -30,9 +31,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import java.time.Clock; import java.time.Duration; -import java.time.ZoneId; import java.util.Collection; import java.util.Collections; import java.util.HashMap; @@ -52,7 +51,6 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; -import static java.time.Instant.ofEpochMilli; import static org.apache.flink.configuration.ConfigurationUtils.getBooleanConfigOption; import static org.apache.flink.configuration.SecurityOptions.DELEGATION_TOKENS_RENEWAL_RETRY_INITIAL_BACKOFF; import static org.apache.flink.configuration.SecurityOptions.DELEGATION_TOKENS_RENEWAL_RETRY_MAX_BACKOFF; @@ -251,7 +249,7 @@ public void calculateRenewalDelayShouldConsiderRenewalRatio() { DefaultDelegationTokenManager delegationTokenManager = new DefaultDelegationTokenManager(configuration, null, null, null); - Clock constantClock = Clock.fixed(ofEpochMilli(100), ZoneId.systemDefault()); + ManualClock constantClock = new ManualClock(100 * 1_000_000L); assertEquals(50, delegationTokenManager.calculateRenewalDelay(constantClock, 200)); } @@ -265,7 +263,7 @@ public void calculateRetryDelayShouldDoubleOnConsecutiveFailures() { DefaultDelegationTokenManager manager = new DefaultDelegationTokenManager(configuration, null, null, null); - Clock clock = Clock.fixed(ofEpochMilli(0), ZoneId.systemDefault()); + ManualClock clock = new ManualClock(0); long delay1 = manager.calculateRetryDelay(clock); long delay2 = manager.calculateRetryDelay(clock); long delay3 = manager.calculateRetryDelay(clock); @@ -289,7 +287,7 @@ public void calculateRetryDelayShouldResetAfterSuccess() { new DefaultDelegationTokenManager(configuration, null, null, null); // Ramp up the backoff via two failures. - Clock clock = Clock.fixed(ofEpochMilli(0), ZoneId.systemDefault()); + ManualClock clock = new ManualClock(0); manager.calculateRetryDelay(clock); manager.calculateRetryDelay(clock); // Simulate success: reset currentRetryBackoff (as startTokensUpdate() would). @@ -311,7 +309,7 @@ public void calculateRetryDelayShouldCapToTtlBound() { // Simulate a failure close to token expiry (30 s remaining). The delay must be capped // so that the retry happens while the token is still valid (at most 30 s / 3 = 10 s). - Clock clock = Clock.fixed(ofEpochMilli(0), ZoneId.systemDefault()); + ManualClock clock = new ManualClock(0); manager.lastKnownNextRenewal = Duration.ofSeconds(30).toMillis(); long delay = manager.calculateRetryDelay(clock); @@ -567,12 +565,12 @@ public void reobtainShouldRunImmediatelyAfterCooldownWindowElapses() throws Exce new ManuallyTriggeredScheduledExecutorService(); Configuration configuration = hermeticCooldownConfig(Duration.ofMillis(60_000)); + long t0 = 1_000_000L; + ManualClock clock = new ManualClock(t0 * 1_000_000L); DefaultDelegationTokenManager delegationTokenManager = new DefaultDelegationTokenManager( - configuration, null, scheduledExecutor, scheduler); + configuration, null, scheduledExecutor, scheduler, clock); - long t0 = 1_000_000L; - delegationTokenManager.setClock(Clock.fixed(ofEpochMilli(t0), ZoneId.systemDefault())); delegationTokenManager.start(tokens -> {}); delegationTokenManager.reobtainDelegationTokens(); assertEquals(0L, onlyScheduledDelayMillis(scheduledExecutor)); @@ -580,8 +578,7 @@ public void reobtainShouldRunImmediatelyAfterCooldownWindowElapses() throws Exce scheduler.triggerAll(); // A request arriving after the full cooldown window has elapsed runs immediately again. - delegationTokenManager.setClock( - Clock.fixed(ofEpochMilli(t0 + 70_000L), ZoneId.systemDefault())); + clock.advanceTime(Duration.ofMillis(70_000L)); delegationTokenManager.reobtainDelegationTokens(); assertEquals(0L, onlyScheduledDelayMillis(scheduledExecutor)); } @@ -594,12 +591,12 @@ public void stopShouldResetCooldownForSubsequentStart() throws Exception { new ManuallyTriggeredScheduledExecutorService(); Configuration configuration = hermeticCooldownConfig(Duration.ofMillis(60_000)); + long t0 = 1_000_000L; + ManualClock clock = new ManualClock(t0 * 1_000_000L); DefaultDelegationTokenManager delegationTokenManager = new DefaultDelegationTokenManager( - configuration, null, scheduledExecutor, scheduler); + configuration, null, scheduledExecutor, scheduler, clock); - long t0 = 1_000_000L; - delegationTokenManager.setClock(Clock.fixed(ofEpochMilli(t0), ZoneId.systemDefault())); delegationTokenManager.start(tokens -> {}); delegationTokenManager.reobtainDelegationTokens(); assertEquals(0L, onlyScheduledDelayMillis(scheduledExecutor)); @@ -607,8 +604,7 @@ public void stopShouldResetCooldownForSubsequentStart() throws Exception { scheduler.triggerAll(); // 10s later a re-obtain is deferred by the cooldown. - delegationTokenManager.setClock( - Clock.fixed(ofEpochMilli(t0 + 10_000L), ZoneId.systemDefault())); + clock.advanceTime(Duration.ofMillis(10_000L)); delegationTokenManager.reobtainDelegationTokens(); assertEquals(50_000L, onlyScheduledDelayMillis(scheduledExecutor)); @@ -616,8 +612,7 @@ public void stopShouldResetCooldownForSubsequentStart() throws Exception { // next re-obtain runs immediately instead of inheriting the stale cooldown. delegationTokenManager.stop(); delegationTokenManager.start(tokens -> {}); - delegationTokenManager.setClock( - Clock.fixed(ofEpochMilli(t0 + 15_000L), ZoneId.systemDefault())); + clock.advanceTime(Duration.ofMillis(5_000L)); delegationTokenManager.reobtainDelegationTokens(); assertEquals(0L, onlyScheduledDelayMillis(scheduledExecutor)); } @@ -630,12 +625,11 @@ public void reobtainShouldRespectCooldown() throws Exception { new ManuallyTriggeredScheduledExecutorService(); Configuration configuration = hermeticCooldownConfig(Duration.ofMillis(60_000)); + long t0 = 1_000_000L; + ManualClock clock = new ManualClock(t0 * 1_000_000L); DefaultDelegationTokenManager delegationTokenManager = new DefaultDelegationTokenManager( - configuration, null, scheduledExecutor, scheduler); - - long t0 = 1_000_000L; - delegationTokenManager.setClock(Clock.fixed(ofEpochMilli(t0), ZoneId.systemDefault())); + configuration, null, scheduledExecutor, scheduler, clock); delegationTokenManager.start(tokens -> {}); @@ -646,8 +640,7 @@ public void reobtainShouldRespectCooldown() throws Exception { scheduler.triggerAll(); // A second re-obtain 10s later must be deferred until the 60s cooldown elapses. - delegationTokenManager.setClock( - Clock.fixed(ofEpochMilli(t0 + 10_000L), ZoneId.systemDefault())); + clock.advanceTime(Duration.ofMillis(10_000L)); delegationTokenManager.reobtainDelegationTokens(); assertEquals(50_000L, onlyScheduledDelayMillis(scheduledExecutor)); } @@ -842,12 +835,12 @@ public void retryMustBringPendingOnDemandReobtainForward() throws Exception { new ManuallyTriggeredScheduledExecutorService(); Configuration configuration = hermeticCooldownConfig(Duration.ofMillis(60_000)); + long t0 = 1_000_000L; + ManualClock clock = new ManualClock(t0 * 1_000_000L); DefaultDelegationTokenManager delegationTokenManager = new DefaultDelegationTokenManager( - configuration, null, scheduledExecutor, scheduler); + configuration, null, scheduledExecutor, scheduler, clock); - long t0 = 1_000_000L; - delegationTokenManager.setClock(Clock.fixed(ofEpochMilli(t0), ZoneId.systemDefault())); delegationTokenManager.start(tokens -> {}); delegationTokenManager.reobtainDelegationTokens(); @@ -855,8 +848,7 @@ public void retryMustBringPendingOnDemandReobtainForward() throws Exception { scheduler.triggerAll(); // 10s later a second re-obtain is cooldown-deferred by 50s. - delegationTokenManager.setClock( - Clock.fixed(ofEpochMilli(t0 + 10_000L), ZoneId.systemDefault())); + clock.advanceTime(Duration.ofMillis(10_000L)); delegationTokenManager.reobtainDelegationTokens(); assertEquals(50_000L, onlyScheduledDelayMillis(scheduledExecutor)); @@ -1050,15 +1042,16 @@ public void cooldownMustSpaceObtainCycleExecutionsNotRequests() throws Exception final ManuallyTriggeredScheduledExecutorService scheduler = new ManuallyTriggeredScheduledExecutorService(); + long t0 = 1_000_000L; + ManualClock clock = new ManualClock(t0 * 1_000_000L); DefaultDelegationTokenManager delegationTokenManager = new DefaultDelegationTokenManager( hermeticCooldownConfig(Duration.ofMillis(60_000)), null, scheduledExecutor, - scheduler); + scheduler, + clock); - long t0 = 1_000_000L; - delegationTokenManager.setClock(Clock.fixed(ofEpochMilli(t0), ZoneId.systemDefault())); delegationTokenManager.start(tokens -> {}); // The first request runs immediately. @@ -1068,12 +1061,10 @@ public void cooldownMustSpaceObtainCycleExecutionsNotRequests() throws Exception scheduler.triggerAll(); // A request at t0+1s is deferred by 59s: its obtain cycle runs at t0+60s. - delegationTokenManager.setClock( - Clock.fixed(ofEpochMilli(t0 + 1_000L), ZoneId.systemDefault())); + clock.advanceTime(Duration.ofMillis(1_000L)); delegationTokenManager.reobtainDelegationTokens(); assertEquals(59_000L, onlyScheduledDelayMillis(scheduledExecutor)); - delegationTokenManager.setClock( - Clock.fixed(ofEpochMilli(t0 + 60_000L), ZoneId.systemDefault())); + clock.advanceTime(Duration.ofMillis(59_000L)); scheduledExecutor.triggerScheduledTasks(); scheduler.triggerAll(); @@ -1081,8 +1072,7 @@ public void cooldownMustSpaceObtainCycleExecutionsNotRequests() throws Exception // so a request arriving just after the deferred cycle ran must be deferred by a full // cooldown measured from that cycle's execution, not run (almost) immediately because // the previous REQUEST arrived one cooldown ago. - delegationTokenManager.setClock( - Clock.fixed(ofEpochMilli(t0 + 61_000L), ZoneId.systemDefault())); + clock.advanceTime(Duration.ofMillis(1_000L)); delegationTokenManager.reobtainDelegationTokens(); assertEquals( 59_000L, @@ -1097,15 +1087,16 @@ public void broughtForwardReobtainMustMoveCooldownAnchor() throws Exception { final ManuallyTriggeredScheduledExecutorService scheduler = new ManuallyTriggeredScheduledExecutorService(); + long t0 = 1_000_000L; + ManualClock clock = new ManualClock(t0 * 1_000_000L); DefaultDelegationTokenManager delegationTokenManager = new DefaultDelegationTokenManager( hermeticCooldownConfig(Duration.ofMillis(60_000)), null, scheduledExecutor, - scheduler); + scheduler, + clock); - long t0 = 1_000_000L; - delegationTokenManager.setClock(Clock.fixed(ofEpochMilli(t0), ZoneId.systemDefault())); delegationTokenManager.start(tokens -> {}); delegationTokenManager.reobtainDelegationTokens(); @@ -1113,8 +1104,7 @@ public void broughtForwardReobtainMustMoveCooldownAnchor() throws Exception { scheduler.triggerAll(); // 10s later a second request is cooldown-deferred by 50s (would run at t0+60s). - delegationTokenManager.setClock( - Clock.fixed(ofEpochMilli(t0 + 10_000L), ZoneId.systemDefault())); + clock.advanceTime(Duration.ofMillis(10_000L)); delegationTokenManager.reobtainDelegationTokens(); assertEquals(50_000L, onlyScheduledDelayMillis(scheduledExecutor)); @@ -1122,16 +1112,14 @@ public void broughtForwardReobtainMustMoveCooldownAnchor() throws Exception { // forward to +5s, so the coalesced cycle actually executes at t0+15s. delegationTokenManager.maybeScheduleRenewal(5_000L); assertEquals(5_000L, onlyScheduledDelayMillis(scheduledExecutor)); - delegationTokenManager.setClock( - Clock.fixed(ofEpochMilli(t0 + 15_000L), ZoneId.systemDefault())); + clock.advanceTime(Duration.ofMillis(5_000L)); scheduledExecutor.triggerScheduledTasks(); scheduler.triggerAll(); // The next request must measure its cooldown from the brought-forward execution // (t0+15s), not from the originally scheduled t0+60s. Otherwise it would defer // beyond a full cooldown (104s here instead of 59s). - delegationTokenManager.setClock( - Clock.fixed(ofEpochMilli(t0 + 16_000L), ZoneId.systemDefault())); + clock.advanceTime(Duration.ofMillis(1_000L)); delegationTokenManager.reobtainDelegationTokens(); assertEquals( 59_000L, From c2ac7c361d59bda2e1644156f096454930d5eba7 Mon Sep 17 00:00:00 2001 From: Aleksandr Savonin Date: Wed, 22 Jul 2026 18:04:43 +0200 Subject: [PATCH 09/22] [FLINK-40019][runtime] Fence cross-session token delivery and fix the start/close race and cooldown clock --- .../token/DefaultDelegationTokenManager.java | 73 ++++--- .../DefaultDelegationTokenManagerTest.java | 195 ++++++++++++++++++ 2 files changed, 245 insertions(+), 23 deletions(-) diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java index dbfa82410596db..0a598e14333a3c 100644 --- a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java +++ b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java @@ -103,7 +103,11 @@ public class DefaultDelegationTokenManager implements DelegationTokenManager { private final long reobtainCooldownMillis; - /** Clock used for renewal and cooldown timing. */ + /** + * Clock used for renewal and cooldown timing. Renewal math reads absolute time (a token's + * validUntil is an absolute epoch), while scheduling and the cooldown read relative time, which + * wall-clock adjustments cannot distort. Never mix the two in one expression. + */ private final Clock clock; /** @@ -128,10 +132,11 @@ public class DefaultDelegationTokenManager implements DelegationTokenManager { private ScheduledFuture tokensUpdateFuture; /** - * Clock time (millis) at which {@link #tokensUpdateFuture} is scheduled to fire, or {@link - * Long#MAX_VALUE} when no cycle is pending. Lets an on-demand re-obtain only ever bring the - * next obtain cycle forward and never push an already-scheduled (e.g. periodic) - * renewal later, which could otherwise let a short-lived token expire before it is renewed. + * Relative (monotonic) clock time (millis) at which {@link #tokensUpdateFuture} is scheduled to + * fire, or {@link Long#MAX_VALUE} when no cycle is pending. Lets an on-demand re-obtain only + * ever bring the next obtain cycle forward and never push an already-scheduled (e.g. + * periodic) renewal later, which could otherwise let a short-lived token expire before it is + * renewed. */ @GuardedBy("tokensUpdateFutureLock") private long nextScheduledAtMillis = Long.MAX_VALUE; @@ -141,9 +146,10 @@ public class DefaultDelegationTokenManager implements DelegationTokenManager { private boolean reobtainScheduled; /** - * Clock time (millis) at which the last on-demand re-obtain cycle was scheduled to execute, or - * {@link #NO_PREVIOUS_REOBTAIN}. Anchored to the execution time rather than the request time, - * so the cooldown spaces cycle executions. Updated only by on-demand re-obtains. + * Relative (monotonic) clock time (millis) at which the last on-demand re-obtain cycle was + * scheduled to execute, or {@link #NO_PREVIOUS_REOBTAIN}. Anchored to the execution time rather + * than the request time, so the cooldown spaces cycle executions. Updated only by on-demand + * re-obtains. */ @GuardedBy("tokensUpdateFutureLock") private long lastReobtainAtMillis = NO_PREVIOUS_REOBTAIN; @@ -155,6 +161,16 @@ public class DefaultDelegationTokenManager implements DelegationTokenManager { @GuardedBy("tokensUpdateFutureLock") private boolean running; + /** + * Incremented by every {@link #start(Listener)}. An obtain cycle captures it when it begins and + * re-checks it before notifying, so a cycle that began under an earlier leadership session + * cannot deliver into a later session. The fence gates delivery only: a stale cycle's renewal + * state is cleared by start()'s reset, and a timer it scheduled just runs a fresh, + * fence-checked cycle later. + */ + @GuardedBy("tokensUpdateFutureLock") + private long sessionEpoch; + @GuardedBy("tokensUpdateFutureLock") @VisibleForTesting @Nullable @@ -393,18 +409,25 @@ protected Optional obtainDelegationTokensAndGetNextRenewal( */ @Override public void start(Listener listener) throws Exception { - checkState( - !closed.get(), - "The delegation token manager is already closed, its providers are stopped"); checkNotNull(scheduledExecutor, "Scheduled executor must not be null"); checkNotNull(ioExecutor, "IO executor must not be null"); checkNotNull(listener, "Listener must not be null"); synchronized (tokensUpdateFutureLock) { + // Checked under the lock so a start() arriving after close() fails instead of + // resurrecting a session against stopped providers. A start() racing close() can + // still slip past the check. close()'s stop() then ends its session under this + // lock, so its inline first cycle either skips on running == false or runs at most + // one obtain that cannot deliver or reschedule and may overlap the provider stop() + // (see close()). + checkState( + !closed.get(), + "The delegation token manager is already closed, its providers are stopped"); if (running) { LOG.warn("DelegationTokenManager is already started, ignoring redundant start()"); return; } this.listener = listener; + sessionEpoch++; // Set before the inline first cycle below: startTokensUpdate() and // maybeScheduleRenewal() gate on it. running = true; @@ -423,6 +446,7 @@ public void start(Listener listener) throws Exception { @VisibleForTesting void startTokensUpdate() { + final long cycleEpoch; synchronized (tokensUpdateFutureLock) { // Clear the dedupe flag so later on-demand requests can schedule a fresh cycle. reobtainScheduled = false; @@ -431,6 +455,7 @@ void startTokensUpdate() { if (!running) { return; } + cycleEpoch = sessionEpoch; } // Serialize the obtain-and-broadcast so a re-obtain racing the periodic renewal cannot run // two cycles concurrently on the (multi-threaded) IO executor and broadcast out of order. @@ -441,13 +466,14 @@ void startTokensUpdate() { Optional nextRenewal = obtainDelegationTokensAndGetNextRenewal(container); if (container.hasTokens()) { - // stop() does not wait for an in-flight cycle. Re-check so a cycle resuming - // after stop() does not notify the stopped session's listener (the disposed - // ResourceManager). A stop() right after this read still lets one delivery - // through, which is benign. + // stop() does not wait for an in-flight cycle: re-check running so a resumed + // cycle does not notify the stopped session's listener, and compare epochs + // so a cycle begun under an earlier session cannot deliver into the next + // one (see sessionEpoch). A stop() right after this read still lets one + // delivery through, which is benign. final Listener currentListener; synchronized (tokensUpdateFutureLock) { - currentListener = running ? listener : null; + currentListener = running && cycleEpoch == sessionEpoch ? listener : null; } if (currentListener != null) { delegationTokenReceiverRepository.onNewTokensObtained(container); @@ -518,7 +544,7 @@ void startTokensUpdate() { @GuardedBy("tokensUpdateFutureLock") private void scheduleRenewalLocked(long delayMs) { stopTokensUpdate(); - nextScheduledAtMillis = clock.absoluteTimeMillis() + delayMs; + nextScheduledAtMillis = clock.relativeTimeMillis() + delayMs; try { tokensUpdateFuture = scheduledExecutor.schedule( @@ -572,12 +598,12 @@ long maybeScheduleRenewal(long delayMs) { } if (reobtainScheduled) { long pendingInMillis = - Math.max(0L, nextScheduledAtMillis - clock.absoluteTimeMillis()); + Math.max(0L, nextScheduledAtMillis - clock.relativeTimeMillis()); if (delayMs < pendingInMillis) { // Bring the pending on-demand cycle forward. scheduleRenewalLocked() leaves // reobtainScheduled set, so coalescing still holds. Move the cooldown anchor // to the time the cycle now actually runs. - lastReobtainAtMillis = clock.absoluteTimeMillis() + delayMs; + lastReobtainAtMillis = clock.relativeTimeMillis() + delayMs; scheduleRenewalLocked(delayMs); return delayMs; } @@ -681,9 +707,10 @@ public void stop() { */ @Override public void close() { - // Flip the flag before stopping anything: a concurrent start() then fails its - // !closed check instead of slipping in between the session stop and the provider - // teardown and running on providers that get stopped underneath it. + // Flip the flag before stopping anything. start() checks it under + // tokensUpdateFutureLock, so a racing start() either fails the check or has its + // session ended by the stop() below (see start()). At most one obtain may still + // overlap the provider stop() below, which the provider threading contract covers. if (!closed.compareAndSet(false, true)) { return; } @@ -721,7 +748,7 @@ public void reobtainDelegationTokens() { } // Cooldown: bound how often on-demand re-obtains can run by deferring this cycle until // at least reobtainCooldownMillis have passed since the previous on-demand re-obtain. - long now = clock.absoluteTimeMillis(); + long now = clock.relativeTimeMillis(); long delayMillis = lastReobtainAtMillis == NO_PREVIOUS_REOBTAIN ? 0L diff --git a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java index a6d627482ac3fe..efcda1526638cb 100644 --- a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java +++ b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java @@ -23,6 +23,7 @@ import org.apache.flink.core.security.token.DelegationTokenProvider; import org.apache.flink.core.security.token.DelegationTokenReceiver; import org.apache.flink.core.testutils.ManuallyTriggeredScheduledExecutorService; +import org.apache.flink.util.clock.Clock; import org.apache.flink.util.clock.ManualClock; import org.apache.flink.util.concurrent.ManuallyTriggeredScheduledExecutor; import org.apache.flink.util.concurrent.ScheduledExecutor; @@ -50,6 +51,8 @@ import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.AtomicReference; import static org.apache.flink.configuration.ConfigurationUtils.getBooleanConfigOption; import static org.apache.flink.configuration.SecurityOptions.DELEGATION_TOKENS_RENEWAL_RETRY_INITIAL_BACKOFF; @@ -1230,6 +1233,159 @@ protected Optional obtainDelegationTokensAndGetNextRenewal( assertNull(delegationTokenManager.listener, "stop() must release the listener reference"); } + @Test + public void inFlightCycleMustNotDeliverIntoTheNextSession() throws Exception { + final ManuallyTriggeredScheduledExecutor scheduledExecutor = + new ManuallyTriggeredScheduledExecutor(); + final ManuallyTriggeredScheduledExecutorService scheduler = + new ManuallyTriggeredScheduledExecutorService(); + + // Same setup rationale as inFlightCycleMustNotNotifyListenerAfterStop: keep the "throw" + // receiver loaded so the broadcast path is real. Unlike there, only the first obtain + // parks (the old session's inline cycle). The new session's first cycle must run + // through. + Configuration configuration = new Configuration(); + configuration.set(getBooleanConfigOption(CONFIG_PREFIX + ".hadoopfs.enabled"), false); + configuration.set(getBooleanConfigOption(CONFIG_PREFIX + ".hbase.enabled"), false); + + final CountDownLatch cycleInObtain = new CountDownLatch(1); + final CountDownLatch resumeObtain = new CountDownLatch(1); + final AtomicBoolean parkNextObtain = new AtomicBoolean(true); + DefaultDelegationTokenManager delegationTokenManager = + new DefaultDelegationTokenManager( + configuration, null, scheduledExecutor, scheduler) { + @Override + protected Optional obtainDelegationTokensAndGetNextRenewal( + DelegationTokenContainer container) { + container.addToken("throw", new byte[] {1}); + if (parkNextObtain.compareAndSet(true, false)) { + cycleInObtain.countDown(); + try { + // Longer than the readiness-poll deadline below, so A cannot + // resume on its own while the test is still waiting for B. + resumeObtain.await(30, TimeUnit.SECONDS); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + return Optional.empty(); + } + }; + + AtomicReference starterFailure = new AtomicReference<>(); + AtomicInteger sessionANotifications = new AtomicInteger(0); + Thread starterA = + new Thread( + () -> { + try { + delegationTokenManager.start( + tokens -> sessionANotifications.incrementAndGet()); + } catch (Throwable t) { + starterFailure.compareAndSet(null, t); + } + }); + starterA.start(); + assertTrue(cycleInObtain.await(10, TimeUnit.SECONDS)); + + // Leadership changes while A's cycle is parked in the obtain: stop() ends session A and + // the next session starts before the cycle resumes. start(B) publishes B's listener + // under tokensUpdateFutureLock and then blocks on obtainLock behind A's cycle, so the + // resuming cycle observes running == true and B's listener. + delegationTokenManager.stop(); + AtomicInteger sessionBNotifications = new AtomicInteger(0); + Thread starterB = + new Thread( + () -> { + try { + delegationTokenManager.start( + tokens -> sessionBNotifications.incrementAndGet()); + } catch (Throwable t) { + starterFailure.compareAndSet(null, t); + } + }); + starterB.start(); + // Wait until start(B) is parked on obtainLock behind A's cycle. B publishes its + // listener under the manager's lock strictly before it can block there, so a durable + // BLOCKED state implies the listener is in place without reading the lock-guarded + // field unsynchronized. This gates when to resume A's parked cycle, so A's re-check + // exercises the epoch fence, not the running flag. + long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(10); + while (starterB.getState() != Thread.State.BLOCKED && System.nanoTime() < deadline) { + Thread.sleep(1); + } + assertEquals(Thread.State.BLOCKED, starterB.getState()); + + resumeObtain.countDown(); + starterA.join(10_000L); + starterB.join(10_000L); + assertFalse(starterA.isAlive()); + assertFalse(starterB.isAlive()); + if (starterFailure.get() != null) { + throw new AssertionError("start() failed in a session thread", starterFailure.get()); + } + + assertEquals( + 0, + sessionANotifications.get(), + "Session A's listener was released by stop() and must not be notified"); + assertEquals( + 1, + sessionBNotifications.get(), + "A cycle that began under an earlier session must not deliver into the next" + + " session: only the next session's own first cycle may notify it"); + } + + @Test + public void cooldownMustBeImmuneToWallClockJumps() throws Exception { + final ManuallyTriggeredScheduledExecutor scheduledExecutor = + new ManuallyTriggeredScheduledExecutor(); + final ManuallyTriggeredScheduledExecutorService scheduler = + new ManuallyTriggeredScheduledExecutorService(); + + JumpableClock clock = new JumpableClock(1_000_000L); + DefaultDelegationTokenManager delegationTokenManager = + new DefaultDelegationTokenManager( + hermeticCooldownConfig(Duration.ofMillis(60_000)), + null, + scheduledExecutor, + scheduler, + clock); + + delegationTokenManager.start(tokens -> {}); + delegationTokenManager.reobtainDelegationTokens(); + assertEquals(0L, onlyScheduledDelayMillis(scheduledExecutor)); + scheduledExecutor.triggerScheduledTasks(); + scheduler.triggerAll(); + + // 10s of real time pass, then NTP steps the wall clock back by an hour. The cooldown is + // a process-local interval, so the next request must still be deferred by the remaining + // 50s, not by the wall-clock difference. + clock.advance(Duration.ofSeconds(10)); + clock.jumpWallClock(Duration.ofHours(-1)); + delegationTokenManager.reobtainDelegationTokens(); + assertEquals( + 50_000L, + onlyScheduledDelayMillis(scheduledExecutor), + "The cooldown must be computed on monotonic time: a wall-clock rollback must" + + " not extend it"); + + // Let the deferred cycle run at its due time (the anchor sits at its execution time). + clock.advance(Duration.ofSeconds(50)); + scheduledExecutor.triggerScheduledTasks(); + scheduler.triggerAll(); + + // 10s later the wall clock jumps two hours forward. Under wall-clock math that would + // zero the remaining cooldown. The monotonic cooldown must still defer by 50s. + clock.advance(Duration.ofSeconds(10)); + clock.jumpWallClock(Duration.ofHours(2)); + delegationTokenManager.reobtainDelegationTokens(); + assertEquals( + 50_000L, + onlyScheduledDelayMillis(scheduledExecutor), + "The cooldown must be computed on monotonic time: a wall-clock jump forward" + + " must not bypass it"); + } + @Test public void registerJobMustNotExposeCallersConfigurationToProviders() throws Exception { DefaultDelegationTokenManager delegationTokenManager = @@ -1272,4 +1428,43 @@ private static long onlyScheduledDelayMillis( assertEquals(1, tasks.size()); return tasks.iterator().next().getDelay(TimeUnit.MILLISECONDS); } + + /** + * A clock whose absolute (wall) time can jump independently of its relative (monotonic) time, + * simulating an NTP step or a manual clock adjustment. {@link ManualClock} cannot express this: + * it drives both flavors from one counter. + */ + private static final class JumpableClock extends Clock { + private final AtomicLong absoluteMillis; + private final AtomicLong relativeNanos = new AtomicLong(); + + JumpableClock(long absoluteMillis) { + this.absoluteMillis = new AtomicLong(absoluteMillis); + } + + @Override + public long absoluteTimeMillis() { + return absoluteMillis.get(); + } + + @Override + public long relativeTimeMillis() { + return relativeNanos.get() / 1_000_000L; + } + + @Override + public long relativeTimeNanos() { + return relativeNanos.get(); + } + + void advance(Duration duration) { + absoluteMillis.addAndGet(duration.toMillis()); + relativeNanos.addAndGet(duration.toNanos()); + } + + /** Steps the wall clock only; relative time is unaffected, like a real NTP step. */ + void jumpWallClock(Duration duration) { + absoluteMillis.addAndGet(duration.toMillis()); + } + } } From 4df526e2daaa3badbf2222fbd22343e02e4a151b Mon Sep 17 00:00:00 2001 From: Aleksandr Savonin Date: Mon, 31 Aug 2026 13:24:28 +0200 Subject: [PATCH 10/22] [FLINK-40019][core][runtime] Rename DelegationTokenProvider.stop() to close() --- .../token/DelegationTokenProvider.java | 6 +++--- .../token/DefaultDelegationTokenManager.java | 21 ++++++++++--------- .../DefaultDelegationTokenManagerTest.java | 18 ++++++++-------- ...eptionThrowingDelegationTokenProvider.java | 14 ++++++------- 4 files changed, 30 insertions(+), 29 deletions(-) diff --git a/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenProvider.java b/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenProvider.java index a5dcd4eba37019..78c6d378451d7d 100644 --- a/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenProvider.java +++ b/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenProvider.java @@ -161,14 +161,14 @@ default void registerJob(JobID jobId, Configuration jobConfiguration) {} default void unregisterJob(JobID jobId) {} /** - * Stops the provider. Any resources should be closed. + * Closes the provider. Any resources should be released. * *

Called at most once, when the manager is closed at process shutdown. It is not called on * ResourceManager leadership changes, those only stop and restart the manager's obtain session * and the provider instance stays in use. An obtain cycle started just before shutdown may - * still be running, so {@code stop()} may overlap an in-flight {@link + * still be running, so {@code close()} may overlap an in-flight {@link * #obtainDelegationTokens()} and implementations must release resources in a way that is safe * with respect to that overlap. */ - default void stop() {} + default void close() {} } diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java index 0a598e14333a3c..1607e96e422529 100644 --- a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java +++ b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java @@ -186,8 +186,8 @@ public class DefaultDelegationTokenManager implements DelegationTokenManager { private final Set registeredJobs = ConcurrentHashMap.newKeySet(); /** - * Set once by {@link #close()}. Keeps provider stop() at most once and rejects any later {@link - * #start(Listener)}. + * Set once by {@link #close()}. Keeps provider close() at most once and rejects any later + * {@link #start(Listener)}. */ private final AtomicBoolean closed = new AtomicBoolean(false); @@ -414,14 +414,14 @@ public void start(Listener listener) throws Exception { checkNotNull(listener, "Listener must not be null"); synchronized (tokensUpdateFutureLock) { // Checked under the lock so a start() arriving after close() fails instead of - // resurrecting a session against stopped providers. A start() racing close() can + // resurrecting a session against closed providers. A start() racing close() can // still slip past the check. close()'s stop() then ends its session under this // lock, so its inline first cycle either skips on running == false or runs at most - // one obtain that cannot deliver or reschedule and may overlap the provider stop() + // one obtain that cannot deliver or reschedule and may overlap the provider close() // (see close()). checkState( !closed.get(), - "The delegation token manager is already closed, its providers are stopped"); + "The delegation token manager is already closed, its providers are closed"); if (running) { LOG.warn("DelegationTokenManager is already started, ignoring redundant start()"); return; @@ -450,7 +450,7 @@ void startTokensUpdate() { synchronized (tokensUpdateFutureLock) { // Clear the dedupe flag so later on-demand requests can schedule a fresh cycle. reobtainScheduled = false; - // Stopped or never started: skip the cycle. The providers may already be stopped + // Stopped or never started: skip the cycle. The providers may already be closed // and the listener may not be set yet. if (!running) { return; @@ -701,7 +701,7 @@ public void stop() { } /** - * Terminal teardown: ends any active session via {@link #stop()} and then stops all providers, + * Terminal teardown: ends any active session via {@link #stop()} and then closes all providers, * exactly once. Called by the component that created the manager at process shutdown, not on * ResourceManager leadership changes. */ @@ -710,16 +710,17 @@ public void close() { // Flip the flag before stopping anything. start() checks it under // tokensUpdateFutureLock, so a racing start() either fails the check or has its // session ended by the stop() below (see start()). At most one obtain may still - // overlap the provider stop() below, which the provider threading contract covers. + // overlap the provider close() below, which the provider threading contract covers. if (!closed.compareAndSet(false, true)) { return; } stop(); for (DelegationTokenProvider provider : delegationTokenProviders.values()) { try { - provider.stop(); + provider.close(); } catch (Throwable t) { - LOG.error("Failed to stop delegation token provider {}", provider.serviceName(), t); + LOG.error( + "Failed to close delegation token provider {}", provider.serviceName(), t); } } } diff --git a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java index efcda1526638cb..4111bbb734dff5 100644 --- a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java +++ b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java @@ -363,17 +363,17 @@ void startTokensUpdate() { } @Test - public void closeShouldStopProvidersExactlyOnce() { + public void closeShouldCloseProvidersExactlyOnce() { DefaultDelegationTokenManager delegationTokenManager = new DefaultDelegationTokenManager(new Configuration(), null, null, null); - // close() is the terminal teardown: it stops the providers, and a repeated close() must - // not stop them again (the SPI promises stop() is called at most once). + // close() is the terminal teardown: it closes the providers, and a repeated close() must + // not close them again (the SPI promises close() is called at most once). delegationTokenManager.close(); delegationTokenManager.close(); - assertTrue(ExceptionThrowingDelegationTokenProvider.stopped.get()); - assertEquals(1, (int) ExceptionThrowingDelegationTokenProvider.stopCallCount.get()); + assertTrue(ExceptionThrowingDelegationTokenProvider.closed.get()); + assertEquals(1, (int) ExceptionThrowingDelegationTokenProvider.closeCallCount.get()); } @Test @@ -385,11 +385,11 @@ public void closeShouldEndSessionAndUnregisterJobs() throws Exception { delegationTokenManager.registerJob(jobId, new Configuration()); // close() ends the session first, so the jobs are unregistered while the providers are - // still usable, and only then are the providers stopped. + // still usable, and only then are the providers closed. delegationTokenManager.close(); assertEquals(0, ExceptionThrowingDelegationTokenProvider.registeredJobs.get().size()); - assertTrue(ExceptionThrowingDelegationTokenProvider.stopped.get()); + assertTrue(ExceptionThrowingDelegationTokenProvider.closed.get()); } @Test @@ -407,7 +407,7 @@ public void startAfterCloseMustFail() { scheduler); delegationTokenManager.close(); - // A closed manager's providers are stopped for good: starting it would run obtain + // A closed manager's providers are closed for good: starting it would run obtain // cycles against dead providers, so it must fail fast. assertThrows(IllegalStateException.class, () -> delegationTokenManager.start(tokens -> {})); } @@ -770,7 +770,7 @@ public void stopShouldKeepProvidersUsableForSubsequentStart() throws Exception { delegationTokenManager.start(tokens -> {}); assertFalse( - ExceptionThrowingDelegationTokenProvider.stopped.get(), + ExceptionThrowingDelegationTokenProvider.closed.get(), "A leadership-session stop() must not close the providers, the next start()" + " re-uses them"); } diff --git a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/ExceptionThrowingDelegationTokenProvider.java b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/ExceptionThrowingDelegationTokenProvider.java index d6e91ffaf272e6..04e73c4af7214d 100644 --- a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/ExceptionThrowingDelegationTokenProvider.java +++ b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/ExceptionThrowingDelegationTokenProvider.java @@ -54,9 +54,9 @@ public class ExceptionThrowingDelegationTokenProvider implements DelegationToken ThreadLocal.withInitial(() -> Boolean.FALSE); public static volatile ThreadLocal throwErrorInUnregister = ThreadLocal.withInitial(() -> Boolean.FALSE); - public static volatile ThreadLocal stopped = + public static volatile ThreadLocal closed = ThreadLocal.withInitial(() -> Boolean.FALSE); - public static volatile ThreadLocal stopCallCount = ThreadLocal.withInitial(() -> 0); + public static volatile ThreadLocal closeCallCount = ThreadLocal.withInitial(() -> 0); public static volatile ThreadLocal mutateJobConfiguration = ThreadLocal.withInitial(() -> Boolean.FALSE); public static volatile ThreadLocal> registeredJobs = @@ -72,8 +72,8 @@ public static void reset() { throwErrorInRegister.set(false); throwInUnregister.set(false); throwErrorInUnregister.set(false); - stopped.set(false); - stopCallCount.set(0); + closed.set(false); + closeCallCount.set(0); mutateJobConfiguration.set(false); registeredJobs.get().clear(); } @@ -151,8 +151,8 @@ public void unregisterJob(JobID jobId) { } @Override - public void stop() { - stopped.set(true); - stopCallCount.set(stopCallCount.get() + 1); + public void close() { + closed.set(true); + closeCallCount.set(closeCallCount.get() + 1); } } From b02b2245eacce04d4ced16b524eaee8fe9e16d82 Mon Sep 17 00:00:00 2001 From: Aleksandr Savonin Date: Mon, 31 Aug 2026 20:46:48 +0200 Subject: [PATCH 11/22] [FLINK-40019][runtime] Rename lock fields obtainLock to renewalCycleLock and tokensUpdateFutureLock to schedulingLock --- .../token/DefaultDelegationTokenManager.java | 48 +++++++++---------- .../DefaultDelegationTokenManagerTest.java | 11 +++-- 2 files changed, 30 insertions(+), 29 deletions(-) diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java index 1607e96e422529..114fa6416bfee1 100644 --- a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java +++ b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java @@ -115,7 +115,7 @@ public class DefaultDelegationTokenManager implements DelegationTokenManager { * wait for an in-flight cycle and the IO executor is multi-threaded, two cycles can never run * concurrently and broadcast tokens out of order. */ - private final Object obtainLock = new Object(); + private final Object renewalCycleLock = new Object(); @VisibleForTesting final Map delegationTokenProviders; @@ -125,9 +125,9 @@ public class DefaultDelegationTokenManager implements DelegationTokenManager { @Nullable private final ExecutorService ioExecutor; - private final Object tokensUpdateFutureLock = new Object(); + private final Object schedulingLock = new Object(); - @GuardedBy("tokensUpdateFutureLock") + @GuardedBy("schedulingLock") @Nullable private ScheduledFuture tokensUpdateFuture; @@ -138,11 +138,11 @@ public class DefaultDelegationTokenManager implements DelegationTokenManager { * periodic) renewal later, which could otherwise let a short-lived token expire before it is * renewed. */ - @GuardedBy("tokensUpdateFutureLock") + @GuardedBy("schedulingLock") private long nextScheduledAtMillis = Long.MAX_VALUE; /** Whether an on-demand re-obtain is scheduled but has not started executing yet (dedupe). */ - @GuardedBy("tokensUpdateFutureLock") + @GuardedBy("schedulingLock") private boolean reobtainScheduled; /** @@ -151,14 +151,14 @@ public class DefaultDelegationTokenManager implements DelegationTokenManager { * than the request time, so the cooldown spaces cycle executions. Updated only by on-demand * re-obtains. */ - @GuardedBy("tokensUpdateFutureLock") + @GuardedBy("schedulingLock") private long lastReobtainAtMillis = NO_PREVIOUS_REOBTAIN; /** * Whether the manager is between {@link #start(Listener)} and {@link #stop()}. Defaults to * false, so work arriving before the first start() is rejected the same way as after stop(). */ - @GuardedBy("tokensUpdateFutureLock") + @GuardedBy("schedulingLock") private boolean running; /** @@ -168,10 +168,10 @@ public class DefaultDelegationTokenManager implements DelegationTokenManager { * state is cleared by start()'s reset, and a timer it scheduled just runs a fresh, * fence-checked cycle later. */ - @GuardedBy("tokensUpdateFutureLock") + @GuardedBy("schedulingLock") private long sessionEpoch; - @GuardedBy("tokensUpdateFutureLock") + @GuardedBy("schedulingLock") @VisibleForTesting @Nullable Listener listener; @@ -412,7 +412,7 @@ public void start(Listener listener) throws Exception { checkNotNull(scheduledExecutor, "Scheduled executor must not be null"); checkNotNull(ioExecutor, "IO executor must not be null"); checkNotNull(listener, "Listener must not be null"); - synchronized (tokensUpdateFutureLock) { + synchronized (schedulingLock) { // Checked under the lock so a start() arriving after close() fails instead of // resurrecting a session against closed providers. A start() racing close() can // still slip past the check. close()'s stop() then ends its session under this @@ -434,9 +434,9 @@ public void start(Listener listener) throws Exception { } // A new session must not inherit the previous session's retry backoff or renewal - // deadline. obtainLock orders this reset after any still-running previous cycle. Not - // nested in the block above to keep the obtainLock -> tokensUpdateFutureLock order. - synchronized (obtainLock) { + // deadline. renewalCycleLock orders this reset after any still-running previous cycle. Not + // nested in the block above to keep the renewalCycleLock -> schedulingLock order. + synchronized (renewalCycleLock) { currentRetryBackoff = renewalRetryInitialBackoff; lastKnownNextRenewal = Long.MAX_VALUE; } @@ -447,7 +447,7 @@ public void start(Listener listener) throws Exception { @VisibleForTesting void startTokensUpdate() { final long cycleEpoch; - synchronized (tokensUpdateFutureLock) { + synchronized (schedulingLock) { // Clear the dedupe flag so later on-demand requests can schedule a fresh cycle. reobtainScheduled = false; // Stopped or never started: skip the cycle. The providers may already be closed @@ -459,7 +459,7 @@ void startTokensUpdate() { } // Serialize the obtain-and-broadcast so a re-obtain racing the periodic renewal cannot run // two cycles concurrently on the (multi-threaded) IO executor and broadcast out of order. - synchronized (obtainLock) { + synchronized (renewalCycleLock) { try { LOG.info("Starting tokens update task"); DelegationTokenContainer container = new DelegationTokenContainer(); @@ -472,7 +472,7 @@ void startTokensUpdate() { // one (see sessionEpoch). A stop() right after this read still lets one // delivery through, which is benign. final Listener currentListener; - synchronized (tokensUpdateFutureLock) { + synchronized (schedulingLock) { currentListener = running && cycleEpoch == sessionEpoch ? listener : null; } if (currentListener != null) { @@ -539,9 +539,9 @@ void startTokensUpdate() { * Schedules a one-shot token-obtain-and-broadcast cycle after {@code delayMs}, replacing any * pending renewal. A delay of {@code 0} brings the next cycle forward to now. Must only be * called after {@link #start(Listener)} (the scheduled and IO executors are non-null then) and - * while holding {@link #tokensUpdateFutureLock}. + * while holding {@link #schedulingLock}. */ - @GuardedBy("tokensUpdateFutureLock") + @GuardedBy("schedulingLock") private void scheduleRenewalLocked(long delayMs) { stopTokensUpdate(); nextScheduledAtMillis = clock.relativeTimeMillis() + delayMs; @@ -554,7 +554,7 @@ private void scheduleRenewalLocked(long delayMs) { } catch (RejectedExecutionException e) { // IO executor is shutting down: drop the cycle but release the // dedupe flag so it cannot get stuck if the manager is reused. - synchronized (tokensUpdateFutureLock) { + synchronized (schedulingLock) { reobtainScheduled = false; } LOG.debug("Tokens update task rejected by IO executor", e); @@ -592,7 +592,7 @@ long maybeScheduleRenewal(long delayMs) { // A negative delay (the token already passed its validUntil) means run now. Clamp it so // it cannot be mistaken for the -1 not-running sentinel. delayMs = Math.max(0L, delayMs); - synchronized (tokensUpdateFutureLock) { + synchronized (schedulingLock) { if (!running) { return -1L; } @@ -619,7 +619,7 @@ long maybeScheduleRenewal(long delayMs) { @VisibleForTesting void stopTokensUpdate() { - synchronized (tokensUpdateFutureLock) { + synchronized (schedulingLock) { if (tokensUpdateFuture != null) { tokensUpdateFuture.cancel(true); tokensUpdateFuture = null; @@ -672,7 +672,7 @@ long calculateRenewalDelay(Clock clock, long nextRenewal) { public void stop() { LOG.info("Stopping credential renewal"); - synchronized (tokensUpdateFutureLock) { + synchronized (schedulingLock) { // Mark not running, cancel the pending cycle, and reset the re-obtain bookkeeping // atomically, so a re-obtain racing shutdown cannot schedule a cycle for a manager // that is shutting down. @@ -708,7 +708,7 @@ public void stop() { @Override public void close() { // Flip the flag before stopping anything. start() checks it under - // tokensUpdateFutureLock, so a racing start() either fails the check or has its + // schedulingLock, so a racing start() either fails the check or has its // session ended by the stop() below (see start()). At most one obtain may still // overlap the provider close() below, which the provider threading contract covers. if (!closed.compareAndSet(false, true)) { @@ -727,7 +727,7 @@ public void close() { @Override public void reobtainDelegationTokens() { - synchronized (tokensUpdateFutureLock) { + synchronized (schedulingLock) { if (scheduledExecutor == null || ioExecutor == null) { LOG.debug( "A re-obtain of delegation tokens was requested but the manager was " diff --git a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java index 4111bbb734dff5..de12d0beec9b13 100644 --- a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java +++ b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java @@ -881,13 +881,14 @@ public void registerJobShouldBeIdempotent() throws Exception { } @Test - public void obtainLockSerializesConcurrentObtainCycles() throws Exception { + public void renewalCycleLockSerializesConcurrentObtainCycles() throws Exception { final ManuallyTriggeredScheduledExecutor scheduledExecutor = new ManuallyTriggeredScheduledExecutor(); final ExecutorService ioExecutor = Executors.newFixedThreadPool(2); try { // The barrier trips only if two obtain cycles are inside the obtain/broadcast - // section at the same time. obtainLock must serialize them, so each should time out. + // section at the same time. renewalCycleLock must serialize them, so each should + // time out. final CyclicBarrier barrier = new CyclicBarrier(2); final AtomicBoolean concurrentObtainDetected = new AtomicBoolean(false); final CountDownLatch done = new CountDownLatch(2); @@ -934,7 +935,7 @@ protected Optional obtainDelegationTokensAndGetNextRenewal( assertTrue(done.await(10, TimeUnit.SECONDS)); assertFalse( concurrentObtainDetected.get(), - "obtainLock must prevent two obtain cycles from running concurrently"); + "renewalCycleLock must prevent two obtain cycles from running concurrently"); } finally { ioExecutor.shutdownNow(); } @@ -1289,7 +1290,7 @@ protected Optional obtainDelegationTokensAndGetNextRenewal( // Leadership changes while A's cycle is parked in the obtain: stop() ends session A and // the next session starts before the cycle resumes. start(B) publishes B's listener - // under tokensUpdateFutureLock and then blocks on obtainLock behind A's cycle, so the + // under schedulingLock and then blocks on renewalCycleLock behind A's cycle, so the // resuming cycle observes running == true and B's listener. delegationTokenManager.stop(); AtomicInteger sessionBNotifications = new AtomicInteger(0); @@ -1304,7 +1305,7 @@ protected Optional obtainDelegationTokensAndGetNextRenewal( } }); starterB.start(); - // Wait until start(B) is parked on obtainLock behind A's cycle. B publishes its + // Wait until start(B) is parked on renewalCycleLock behind A's cycle. B publishes its // listener under the manager's lock strictly before it can block there, so a durable // BLOCKED state implies the listener is in place without reading the lock-guarded // field unsynchronized. This gates when to resume A's parked cycle, so A's re-check From ddf100114cb5f209edd5930b5d99e9ce5313c198 Mon Sep 17 00:00:00 2001 From: Aleksandr Savonin Date: Mon, 31 Aug 2026 20:52:48 +0200 Subject: [PATCH 12/22] [FLINK-40019][runtime] Annotate renewal backoff and next-renewal fields as guarded by renewalCycleLock --- .../security/token/DefaultDelegationTokenManager.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java index 114fa6416bfee1..fb63221f88d3fa 100644 --- a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java +++ b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java @@ -97,9 +97,13 @@ public class DefaultDelegationTokenManager implements DelegationTokenManager { private final long renewalRetryMaxBackoff; - @VisibleForTesting long currentRetryBackoff; + @GuardedBy("renewalCycleLock") + @VisibleForTesting + long currentRetryBackoff; - @VisibleForTesting long lastKnownNextRenewal = Long.MAX_VALUE; + @GuardedBy("renewalCycleLock") + @VisibleForTesting + long lastKnownNextRenewal = Long.MAX_VALUE; private final long reobtainCooldownMillis; From 8d5d41ad5c0de802898f742d94186d0d37475d4f Mon Sep 17 00:00:00 2001 From: Aleksandr Savonin Date: Mon, 31 Aug 2026 20:58:03 +0200 Subject: [PATCH 13/22] [FLINK-40019][runtime] Inline the re-obtain callback at the provider init call --- .../security/token/DefaultDelegationTokenManager.java | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java index fb63221f88d3fa..8d64eb0287648b 100644 --- a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java +++ b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java @@ -24,7 +24,6 @@ import org.apache.flink.configuration.Configuration; import org.apache.flink.configuration.SecurityOptions; import org.apache.flink.core.plugin.PluginManager; -import org.apache.flink.core.security.token.DelegationTokenManagerCallback; import org.apache.flink.core.security.token.DelegationTokenProvider; import org.apache.flink.core.security.token.DelegationTokenReceiver; import org.apache.flink.util.FlinkRuntimeException; @@ -244,15 +243,12 @@ public DefaultDelegationTokenManager( private Map loadProviders() { LOG.info("Loading delegation token providers"); - // Handed to every provider so it can request an immediate re-obtain later, from any - // thread, decoupled from the registerJob call stack. - final DelegationTokenManagerCallback callback = this::reobtainDelegationTokens; Map providers = new HashMap<>(); Consumer loadProvider = (provider) -> { try { if (isProviderEnabled(configuration, provider.serviceName())) { - provider.init(configuration, callback); + provider.init(configuration, this::reobtainDelegationTokens); LOG.info( "Delegation token provider {} loaded and initialized", provider.serviceName()); From ce9efa9af8ab1307ec5e4a27eff7a6f2bc5d0910 Mon Sep 17 00:00:00 2001 From: Aleksandr Savonin Date: Mon, 31 Aug 2026 21:03:06 +0200 Subject: [PATCH 14/22] [FLINK-40019][runtime] Tear down startTokensUpdateShouldScheduleRenewal with stop() to match its start() call --- .../security/token/DefaultDelegationTokenManagerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java index de12d0beec9b13..b20aba87c901d9 100644 --- a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java +++ b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java @@ -239,7 +239,7 @@ void startTokensUpdate() { ExceptionThrowingDelegationTokenProvider.throwInUsage.set(false); scheduledExecutor.triggerScheduledTasks(); scheduler.triggerAll(); - delegationTokenManager.stopTokensUpdate(); + delegationTokenManager.stop(); assertEquals(3, startTokensUpdateCallCount.get()); } From 8c56630692d133b2b079b2aa603b03b386d6c038 Mon Sep 17 00:00:00 2001 From: Aleksandr Savonin Date: Mon, 21 Sep 2026 16:37:24 +0200 Subject: [PATCH 15/22] [FLINK-40019][core][runtime] Track only successful delegation token registrations Track jobs only after all providers accept registration, and drop them after unregistration even if cleanup fails. Document the cleanup policy and cover both failure paths in tests. Generated-by: Fable 5.1 Generated-by: Codex (GPT-6) --- .../token/DelegationTokenProvider.java | 30 +++++--- .../token/DefaultDelegationTokenManager.java | 43 ++++++------ .../token/DelegationTokenManager.java | 27 ++++--- .../DefaultDelegationTokenManagerTest.java | 70 +++++++++++-------- 4 files changed, 97 insertions(+), 73 deletions(-) diff --git a/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenProvider.java b/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenProvider.java index 78c6d378451d7d..9effbfd15c690a 100644 --- a/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenProvider.java +++ b/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenProvider.java @@ -123,7 +123,8 @@ default void init(Configuration configuration, DelegationTokenManagerCallback ca ObtainedDelegationTokens obtainDelegationTokens() throws Exception; /** - * Called when a job has started, before its tasks are scheduled, with its configuration. + * Called with the job's configuration when its JobMaster registers with the ResourceManager. + * Re-registration may occur while the job's tasks are running. * *

To get the job's tokens distributed without waiting for the periodic renewal, call {@link * DelegationTokenManagerCallback#reobtainDelegationTokens()} on the callback handed to {@link @@ -139,10 +140,15 @@ default void init(Configuration configuration, DelegationTokenManagerCallback ca *

Must be idempotent: it may be called more than once for the same {@code jobId} (e.g. on * JobManager or ResourceManager failover, when the JobMaster re-registers). * - *

Should not throw: a thrown (unchecked) exception or linkage error rejects the job's - * registration (the job does not start) and triggers {@link #unregisterJob(JobID)} on all - * providers to roll back. Prefer deferring the real fetch to the (retrying) obtain cycle over a - * synchronous fetch, so a transient failure does not fail the job. + *

Should not throw: an unchecked exception or linkage error rejects the current registration + * attempt. If the manager does not currently track a successful registration for this job, it + * calls {@link #unregisterJob(JobID)} on all providers to attempt rollback. Otherwise, it keeps + * the existing registration and does not invoke {@code unregisterJob} for that failure, because + * the job's tasks may still be running. Keeping the registration does not undo changes + * providers made during the failed attempt. + * + *

Prefer deferring token retrieval to the retrying {@link #obtainDelegationTokens()} cycle + * so transient fetch failures do not prevent registration. * * @param jobId The job id of the job. * @param jobConfiguration The job configuration. @@ -150,11 +156,15 @@ default void init(Configuration configuration, DelegationTokenManagerCallback ca default void registerJob(JobID jobId, Configuration jobConfiguration) {} /** - * Called when the job is being removed — it reached a globally terminal state, or its - * job-leader registration timed out — and its per-job state should be released. Must be - * idempotent. Exceptions and linkage errors are caught and logged by the framework (one - * provider's failure does not abort cleanup of the others), but implementations should still - * avoid throwing. + * Called to release per-job state when a job is removed, a registration attempt is rolled back, + * or the manager stops its current session. A job is removed when it reaches a globally + * terminal state or its job-leader registration times out. Must be idempotent and should not + * throw. + * + *

Exceptions and linkage errors are caught and logged, so cleanup continues for the other + * providers. The manager removes the job from its tracking even if cleanup fails and does not + * retain it for a later cleanup attempt. Providers are responsible for releasing any remaining + * state in {@link #close()}. * * @param jobId The job id of the job. */ diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java index 8d64eb0287648b..6a318b086a9080 100644 --- a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java +++ b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java @@ -180,11 +180,13 @@ public class DefaultDelegationTokenManager implements DelegationTokenManager { Listener listener; /** - * Jobs for which providers may hold per-job state. A job is added on successful registration - * (or when a failed rollback left provider state behind) and removed when every provider - * unregistered it cleanly. Lets a failed re-registration keep the previous state and lets - * {@link #stop()} unregister the jobs of the ending session. All checks and updates run on the - * ResourceManager main thread, and leadership sessions are serialized. + * Jobs successfully registered with all providers. A job is removed when it is unregistered, + * even if a provider fails to release its state. + * + *

Tracking successful registrations prevents a failed re-registration from rolling back an + * existing registration. {@link #stop()} uses this set to unregister jobs from the ending + * session. All checks and updates run on the ResourceManager main thread, and leadership + * sessions are serialized. */ private final Set registeredJobs = ConcurrentHashMap.newKeySet(); @@ -798,14 +800,12 @@ public void registerJob(JobID jobId, Configuration jobConfiguration) throws Exce failedProvider == null ? "" : failedProvider.serviceName(), e); } else { - // First registration: roll back from all providers (unregisterJob is idempotent). - // The rollback must never mask the original failure. + // No successful registration is currently tracked for this job. Roll back + // all providers (unregisterJob is idempotent). Leave the job untracked, + // even if rollback fails, so a later failed registration attempt also + // triggers rollback. The rollback must never mask the original failure. try { - if (!unregisterJobInternal(jobId)) { - // Keep the job tracked so stop() or a registration retry can release the - // provider state left behind. - registeredJobs.add(jobId); - } + unregisterJobInternal(jobId); } catch (Exception | LinkageError rollbackException) { LOG.error( "Failed to roll back registration of job {}", jobId, rollbackException); @@ -826,19 +826,19 @@ public void unregisterJob(JobID jobId) throws Exception { } /** - * Unregisters the job from all providers, swallowing per-provider failures. The job leaves - * {@link #registeredJobs} only when every provider unregistered cleanly, so state a failed - * provider may still hold stays tracked for another attempt. + * Attempts to unregister the job from all providers and removes it from {@link + * #registeredJobs}. Provider failures are logged and swallowed, so cleanup continues for the + * other providers. * - * @return whether every provider unregistered the job without failure. + *

The job is removed even if cleanup fails. The manager does not retain it for a later + * cleanup attempt. Providers are responsible for releasing any remaining state in {@link + * DelegationTokenProvider#close()}. */ - private boolean unregisterJobInternal(JobID jobId) { - boolean fullyUnregistered = true; + private void unregisterJobInternal(JobID jobId) { for (DelegationTokenProvider provider : delegationTokenProviders.values()) { try { provider.unregisterJob(jobId); } catch (Exception | LinkageError e) { - fullyUnregistered = false; LOG.error( "Failed to unregister job {} for provider {}", jobId, @@ -846,9 +846,6 @@ private boolean unregisterJobInternal(JobID jobId) { e); } } - if (fullyUnregistered) { - registeredJobs.remove(jobId); - } - return fullyUnregistered; + registeredJobs.remove(jobId); } } diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DelegationTokenManager.java b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DelegationTokenManager.java index c3130ad829c3c7..18db63e3788764 100644 --- a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DelegationTokenManager.java +++ b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DelegationTokenManager.java @@ -91,25 +91,30 @@ default void close() {} default void reobtainDelegationTokens() {} /** - * Called when a job has started. Fans the event out to all loaded {@link - * org.apache.flink.core.security.token.DelegationTokenProvider}s. On failure of the job's first - * registration, the job is unregistered from all providers and the exception is rethrown so the - * caller can reject the registration. A failed re-registration rethrows but keeps the job - * registered, so a running job's tokens are not dropped. A provider that needs the new job's - * tokens distributed immediately requests it via {@link + * Called when a JobMaster registers with the ResourceManager. Fans the event out to all loaded + * {@link org.apache.flink.core.security.token.DelegationTokenProvider}s. On failure, the + * exception is rethrown so the caller can reject the registration attempt. If no successful + * registration is currently tracked for the job, the manager calls {@link + * org.apache.flink.core.security.token.DelegationTokenProvider#unregisterJob(JobID)} on all + * providers to attempt rollback. Otherwise, it keeps the existing registration and does not + * attempt rollback, because the job's tasks may still be running. A provider that needs the + * job's tokens distributed immediately requests it via {@link * org.apache.flink.core.security.token.DelegationTokenManagerCallback#reobtainDelegationTokens()}. * - * @param jobId The job id which just started. + * @param jobId The ID of the job being registered. * @param jobConfiguration The job's configuration. */ default void registerJob(JobID jobId, Configuration jobConfiguration) throws Exception {} /** - * Called when a job is being removed. Fans the event out to all loaded providers. Must be - * idempotent. Per-provider failures are caught and logged (one provider's failure does not - * abort cleanup of the others), so in practice this does not throw for provider failures. + * Called when a job is being removed. Attempts to unregister it from all loaded providers. Must + * be idempotent. Provider failures are caught and logged, so cleanup continues for the other + * providers. * - * @param jobId The job id of the job. + *

The job is removed from the manager even if cleanup fails. The manager does not retain it + * for a later cleanup attempt. + * + * @param jobId The ID of the job being removed. */ default void unregisterJob(JobID jobId) throws Exception {} } diff --git a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java index b20aba87c901d9..f3e9c8f404b79a 100644 --- a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java +++ b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java @@ -60,6 +60,8 @@ import static org.apache.flink.configuration.SecurityOptions.DELEGATION_TOKENS_RENEWAL_TIME_RATIO; import static org.apache.flink.configuration.SecurityOptions.DELEGATION_TOKENS_REOBTAIN_COOLDOWN; import static org.apache.flink.core.security.token.DelegationTokenProvider.CONFIG_PREFIX; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -989,54 +991,64 @@ public void stopShouldUnregisterAllRegisteredJobs() throws Exception { } @Test - public void leftoverRollbackStateMustBeReleasedByStop() throws Exception { - DefaultDelegationTokenManager delegationTokenManager = + public void failedRegistrationIsNotTrackedUntilARetrySucceeds() throws Exception { + final DefaultDelegationTokenManager delegationTokenManager = new DefaultDelegationTokenManager(new Configuration(), null, null, null); + final JobID jobId = JobID.generate(); - // A FIRST registration fails after the provider recorded state (add-then-throw), and - // the rollback's unregister fails too: the state is left behind in the provider. + // Registration stores job state and then fails. Rollback also fails, + // leaving the state in the provider. ExceptionThrowingDelegationTokenProvider.throwErrorInRegister.set(true); ExceptionThrowingDelegationTokenProvider.throwInUnregister.set(true); - JobID jobId = JobID.generate(); - assertThrows( - NoClassDefFoundError.class, - () -> delegationTokenManager.registerJob(jobId, new Configuration())); - assertEquals(1, ExceptionThrowingDelegationTokenProvider.registeredJobs.get().size()); + assertThatThrownBy(() -> delegationTokenManager.registerJob(jobId, new Configuration())) + .isInstanceOf(NoClassDefFoundError.class); + assertThat(ExceptionThrowingDelegationTokenProvider.registeredJobs.get()) + .containsExactly(jobId); - // The job must have stayed tracked despite the failed rollback, so stop() releases the - // leftover state once the provider recovers. - ExceptionThrowingDelegationTokenProvider.throwErrorInRegister.set(false); + // Allow cleanup to succeed, but keep registration failing. No successful registration + // should be tracked, so the next failed attempt must roll back the leftover state. ExceptionThrowingDelegationTokenProvider.throwInUnregister.set(false); + assertThatThrownBy(() -> delegationTokenManager.registerJob(jobId, new Configuration())) + .isInstanceOf(NoClassDefFoundError.class); + assertThat(ExceptionThrowingDelegationTokenProvider.registeredJobs.get()) + .as("a failed registration retry removes leftover provider state") + .isEmpty(); + + // Recovery must track the successful retry so session shutdown cleans it up. + ExceptionThrowingDelegationTokenProvider.throwErrorInRegister.set(false); + delegationTokenManager.registerJob(jobId, new Configuration()); + assertThat(ExceptionThrowingDelegationTokenProvider.registeredJobs.get()) + .containsExactly(jobId); + delegationTokenManager.stop(); - assertEquals( - 0, - ExceptionThrowingDelegationTokenProvider.registeredJobs.get().size(), - "State left behind by a failed rollback must be released by stop()"); + assertThat(ExceptionThrowingDelegationTokenProvider.registeredJobs.get()) + .as("stop() releases the successfully registered job") + .isEmpty(); } @Test - public void stopMustRetryFailedUnregistration() throws Exception { - DefaultDelegationTokenManager delegationTokenManager = + public void failedUnregistrationMustDropTheJob() throws Exception { + final DefaultDelegationTokenManager delegationTokenManager = new DefaultDelegationTokenManager(new Configuration(), null, null, null); - JobID jobId = JobID.generate(); + final JobID jobId = JobID.generate(); delegationTokenManager.registerJob(jobId, new Configuration()); - assertEquals(1, ExceptionThrowingDelegationTokenProvider.registeredJobs.get().size()); + assertThat(ExceptionThrowingDelegationTokenProvider.registeredJobs.get()) + .containsExactly(jobId); - // A provider fails its unregistration (swallowed by contract), so its per-job state - // survives. The manager must keep tracking the job instead of forgetting it. + // Cleanup fails and leaves provider state behind, but the manager must + // stop tracking the job. ExceptionThrowingDelegationTokenProvider.throwInUnregister.set(true); delegationTokenManager.unregisterJob(jobId); - assertEquals(1, ExceptionThrowingDelegationTokenProvider.registeredJobs.get().size()); + assertThat(ExceptionThrowingDelegationTokenProvider.registeredJobs.get()) + .containsExactly(jobId); - // Once the provider recovers, stop() gets another attempt, without the retry - // the job's state would leak in the process-lifetime provider until process shutdown. + // Allow cleanup to succeed. stop() must not retry the failed unregistration. ExceptionThrowingDelegationTokenProvider.throwInUnregister.set(false); delegationTokenManager.stop(); - assertEquals( - 0, - ExceptionThrowingDelegationTokenProvider.registeredJobs.get().size(), - "A job whose unregistration failed must be released by stop()"); + assertThat(ExceptionThrowingDelegationTokenProvider.registeredJobs.get()) + .as("stop() must not retry a failed unregistration") + .containsExactly(jobId); } @Test From c2e1b981eb6a17618ecce2444618b31151185a70 Mon Sep 17 00:00:00 2001 From: Aleksandr Savonin Date: Mon, 21 Sep 2026 18:39:35 +0200 Subject: [PATCH 16/22] [FLINK-40019][runtime] Remove redundant JobMaster registration overload Require the job configuration when registering a JobMaster with the ResourceManager. Update the existing tests to pass an empty configuration. --- .../ResourceManagerGateway.java | 29 ------------------- .../ResourceManagerJobMasterTest.java | 12 +++++--- .../resourcemanager/ResourceManagerTest.java | 7 +++++ 3 files changed, 15 insertions(+), 33 deletions(-) diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/ResourceManagerGateway.java b/flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/ResourceManagerGateway.java index 73f6563192eaca..30897747d25784 100644 --- a/flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/ResourceManagerGateway.java +++ b/flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/ResourceManagerGateway.java @@ -62,35 +62,6 @@ public interface ResourceManagerGateway extends FencedRpcGateway, ClusterPartitionManager, BlocklistListener { - /** - * Register a {@link JobMaster} at the resource manager. - * - *

Backward-compatible overload that registers without a job configuration. Equivalent to - * calling {@link #registerJobMaster(JobMasterId, ResourceID, String, JobID, Configuration, - * Duration)} with an empty configuration. - * - * @param jobMasterId The fencing token for the JobMaster leader - * @param jobMasterResourceId The resource ID of the JobMaster that registers - * @param jobMasterAddress The address of the JobMaster that registers - * @param jobId The Job ID of the JobMaster that registers - * @param timeout Timeout for the future to complete - * @return Future registration response - */ - default CompletableFuture registerJobMaster( - JobMasterId jobMasterId, - ResourceID jobMasterResourceId, - String jobMasterAddress, - JobID jobId, - @RpcTimeout Duration timeout) { - return registerJobMaster( - jobMasterId, - jobMasterResourceId, - jobMasterAddress, - jobId, - new Configuration(), - timeout); - } - /** * Register a {@link JobMaster} at the resource manager, supplying the job's {@link * Configuration} so implementations can perform per-job initialization (e.g. obtaining diff --git a/flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/ResourceManagerJobMasterTest.java b/flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/ResourceManagerJobMasterTest.java index 8fb78372ac51a2..e5db80ad463d8a 100644 --- a/flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/ResourceManagerJobMasterTest.java +++ b/flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/ResourceManagerJobMasterTest.java @@ -155,6 +155,7 @@ void testRegisterJobMaster() { jobMasterResourceId, jobMasterGateway.getAddress(), jobId, + new Configuration(), TIMEOUT); assertThatFuture(successfulFuture) .succeedsWithin(TIMEOUT.toMillis(), TimeUnit.MILLISECONDS) @@ -162,10 +163,8 @@ void testRegisterJobMaster() { } /** - * FLIP-588: if the delegation token manager rejects the job (its {@code registerJob} throws), - * the ResourceManager must reject the JobMaster registration so the job does not start without - * the tokens it requires. This also exercises the widened (6-arg) {@code registerJobMaster} RPC - * that carries the job {@link Configuration}. + * Verifies that the ResourceManager rejects JobMaster registration when the delegation token + * manager fails to register the job. */ @Test void testRegisterJobMasterRejectedWhenDelegationTokenRegistrationFails() throws Exception { @@ -226,6 +225,7 @@ void testDisconnectTaskManagerInResourceManager() jobMasterResourceId, jobMasterGateway.getAddress(), jobId, + new Configuration(), TIMEOUT); assertThatFuture(successfulFuture) .succeedsWithin(TIMEOUT.toMillis(), TimeUnit.MILLISECONDS) @@ -267,6 +267,7 @@ void testRegisterJobMasterWithUnmatchedLeaderSessionId1() throws Exception { jobMasterResourceId, jobMasterGateway.getAddress(), jobId, + new Configuration(), TIMEOUT); assertThatFuture(unMatchedLeaderFuture) .withFailMessage("Should fail because we are using the wrong fencing token.") @@ -287,6 +288,7 @@ void testRegisterJobMasterWithUnmatchedLeaderSessionId2() { jobMasterResourceId, jobMasterGateway.getAddress(), jobId, + new Configuration(), TIMEOUT); assertThatFuture(unMatchedLeaderFuture) .eventuallySucceeds() @@ -305,6 +307,7 @@ void testRegisterJobMasterFromInvalidAddress() { jobMasterResourceId, invalidAddress, jobId, + new Configuration(), TIMEOUT); assertThatFuture(invalidAddressFuture) .succeedsWithin(5, TimeUnit.SECONDS) @@ -326,6 +329,7 @@ void testRegisterJobMasterWithFailureLeaderListener() { jobMasterResourceId, jobMasterGateway.getAddress(), unknownJobIDToHAServices, + new Configuration(), TIMEOUT); assertThatFuture(registrationFuture) diff --git a/flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/ResourceManagerTest.java b/flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/ResourceManagerTest.java index 2096c95fb97714..b22b363567d743 100644 --- a/flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/ResourceManagerTest.java +++ b/flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/ResourceManagerTest.java @@ -20,6 +20,7 @@ import org.apache.flink.api.common.JobID; import org.apache.flink.api.common.JobStatus; +import org.apache.flink.configuration.Configuration; import org.apache.flink.core.testutils.OneShotLatch; import org.apache.flink.runtime.blocklist.BlockedNode; import org.apache.flink.runtime.blocklist.BlocklistHandler; @@ -303,6 +304,7 @@ void testDisconnectJobManagerClearsRequirements() throws Exception { ResourceID.generate(), jobMasterGateway.getAddress(), jobId, + new Configuration(), TIMEOUT) .get(); @@ -363,6 +365,7 @@ void testProcessResourceRequirementsWhenRecoveryFinished() throws Exception { ResourceID.generate(), jobMasterGateway.getAddress(), jobId, + new Configuration(), TIMEOUT) .get(); @@ -421,6 +424,7 @@ void testHeartbeatTimeoutWithJobMaster() throws Exception { jobMasterResourceId, jobMasterGateway.getAddress(), jobId, + new Configuration(), TIMEOUT); assertThatFuture(registrationFuture) @@ -483,6 +487,7 @@ void testJobMasterBecomesUnreachableTriggersDisconnect() throws Exception { jobMasterResourceId, jobMasterGateway.getAddress(), jobId, + new Configuration(), TIMEOUT); assertThatFuture(registrationFuture) @@ -813,6 +818,7 @@ private static void registerJobMasterToResourceManager( ResourceID.generate(), jobMasterGateway.getAddress(), jobId, + new Configuration(), TIMEOUT) .get(); } @@ -852,6 +858,7 @@ private void testDisconnectJobManager(JobStatus jobStatus) throws Exception { ResourceID.generate(), jobMasterGateway.getAddress(), jobId, + new Configuration(), TIMEOUT); jobAdded.await(); From 39bf3667d73e4aeebaa6c29de675cf33d9059a79 Mon Sep 17 00:00:00 2001 From: Aleksandr Savonin Date: Wed, 23 Sep 2026 17:47:29 +0200 Subject: [PATCH 17/22] [FLINK-40019][runtime] Test ResourceManager accepts registration after token failure A failed delegation token registration must return an RPC failure and leave no usable JobMaster registration behind. Verify that retrying the same registration request succeeds and resource declarations are then accepted. Generated-by: Codex (GPT-6) --- .../ResourceManagerJobMasterTest.java | 82 +++++++++++++++---- 1 file changed, 64 insertions(+), 18 deletions(-) diff --git a/flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/ResourceManagerJobMasterTest.java b/flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/ResourceManagerJobMasterTest.java index e5db80ad463d8a..dca25dc78f772d 100644 --- a/flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/ResourceManagerJobMasterTest.java +++ b/flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/ResourceManagerJobMasterTest.java @@ -37,6 +37,7 @@ import org.apache.flink.runtime.rpc.exceptions.FencingTokenException; import org.apache.flink.runtime.security.token.DelegationTokenManager; import org.apache.flink.runtime.security.token.NoOpDelegationTokenManager; +import org.apache.flink.runtime.slots.ResourceRequirements; import org.apache.flink.runtime.taskexecutor.TaskExecutorGateway; import org.apache.flink.runtime.taskexecutor.TestingTaskExecutorGatewayBuilder; import org.apache.flink.util.FlinkRuntimeException; @@ -46,11 +47,13 @@ import org.junit.jupiter.api.Test; import java.time.Duration; +import java.util.Collections; import java.util.UUID; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicInteger; import static org.apache.flink.core.testutils.FlinkAssertions.assertThatFuture; import static org.apache.flink.runtime.resourcemanager.ResourceManagerPartitionLifecycleTest.registerTaskExecutor; @@ -130,12 +133,16 @@ private void createAndStartResourceManagerService(DelegationTokenManager delegat "RM not available after confirming leadership.")); } - @AfterEach - void teardown() throws Exception { + private void stopResourceManagerService() throws Exception { if (resourceManagerService != null) { resourceManagerService.rethrowFatalErrorIfAny(); - resourceManagerService.cleanUp(); + resourceManagerService.closeAsync().get(); } + } + + @AfterEach + void teardown() throws Exception { + stopResourceManagerService(); if (rpcService != null) { RpcUtils.terminateRpcService(rpcService); @@ -163,17 +170,19 @@ void testRegisterJobMaster() { } /** - * Verifies that the ResourceManager rejects JobMaster registration when the delegation token - * manager fails to register the job. + * Verifies that failed delegation token registration leaves no JobMaster registration behind + * and that a subsequent registration attempt succeeds. */ @Test - void testRegisterJobMasterRejectedWhenDelegationTokenRegistrationFails() throws Exception { - // Rebuild the RM service with a delegation token manager that rejects registerJob. - resourceManagerService.rethrowFatalErrorIfAny(); - resourceManagerService.cleanUp(); + void testRegisterJobMasterSucceedsAfterDelegationTokenRegistrationFailure() throws Exception { + // Rebuild the RM service with a delegation token manager that rejects the first attempt. + stopResourceManagerService(); final FlinkRuntimeException failure = new FlinkRuntimeException("registerJob rejected by provider"); - createAndStartResourceManagerService(new RejectingDelegationTokenManager(failure)); + final FailingOnceDelegationTokenManager delegationTokenManager = + new FailingOnceDelegationTokenManager(failure); + createAndStartResourceManagerService(delegationTokenManager); + final Configuration jobConfiguration = new Configuration(); final CompletableFuture registrationFuture = resourceManagerGateway.registerJobMaster( @@ -181,16 +190,48 @@ void testRegisterJobMasterRejectedWhenDelegationTokenRegistrationFails() throws jobMasterResourceId, jobMasterGateway.getAddress(), jobId, - new Configuration(), - TIMEOUT); + jobConfiguration, + RpcUtils.INF_TIMEOUT); - final RegistrationResponse response = - registrationFuture.get(TIMEOUT.toMillis(), TimeUnit.MILLISECONDS); + final RegistrationResponse response = registrationFuture.get(); assertThat(response).isInstanceOf(RegistrationResponse.Failure.class); final Throwable reason = ((RegistrationResponse.Failure) response).getReason(); assertThat(reason.getMessage()).contains(jobId.toString()); assertThat(reason.getMessage()).contains("delegation token manager"); assertThat(reason.getCause().getMessage()).contains("registerJob rejected by provider"); + assertThat(delegationTokenManager.registrationAttempts.get()).isEqualTo(1); + + final ResourceRequirements resourceRequirements = + ResourceRequirements.create( + jobId, jobMasterGateway.getAddress(), Collections.emptyList()); + assertThatFuture( + resourceManagerGateway.declareRequiredResources( + jobMasterGateway.getFencingToken(), + resourceRequirements, + RpcUtils.INF_TIMEOUT)) + .eventuallyFails() + .withThrowableOfType(ExecutionException.class) + .withCauseInstanceOf(ResourceManagerException.class) + .withMessageContaining("Could not find registered job manager"); + + final CompletableFuture retryFuture = + resourceManagerGateway.registerJobMaster( + jobMasterGateway.getFencingToken(), + jobMasterResourceId, + jobMasterGateway.getAddress(), + jobId, + jobConfiguration, + RpcUtils.INF_TIMEOUT); + assertThatFuture(retryFuture) + .eventuallySucceeds() + .isInstanceOf(JobMasterRegistrationSuccess.class); + assertThat(delegationTokenManager.registrationAttempts.get()).isEqualTo(2); + assertThatFuture( + resourceManagerGateway.declareRequiredResources( + jobMasterGateway.getFencingToken(), + resourceRequirements, + RpcUtils.INF_TIMEOUT)) + .eventuallySucceeds(); } @Test @@ -342,18 +383,23 @@ void testRegisterJobMasterWithFailureLeaderListener() { resourceManagerService.ignoreFatalErrors(); } - /** A {@link DelegationTokenManager} whose {@code registerJob} always throws. */ - private static final class RejectingDelegationTokenManager extends NoOpDelegationTokenManager { + /** A {@link DelegationTokenManager} whose first {@code registerJob} call throws. */ + private static final class FailingOnceDelegationTokenManager + extends NoOpDelegationTokenManager { private final Exception failure; - private RejectingDelegationTokenManager(Exception failure) { + private final AtomicInteger registrationAttempts = new AtomicInteger(); + + private FailingOnceDelegationTokenManager(Exception failure) { this.failure = failure; } @Override public void registerJob(JobID jobId, Configuration jobConfiguration) throws Exception { - throw failure; + if (registrationAttempts.incrementAndGet() == 1) { + throw failure; + } } } } From 8b25efa085ee1adce0f92819821c123a934eb105 Mon Sep 17 00:00:00 2001 From: Aleksandr Savonin Date: Thu, 24 Sep 2026 15:20:04 +0200 Subject: [PATCH 18/22] [FLINK-40019][runtime] Test automatic JobMaster registration retry after token failure Run a real JobMaster against a ResourceManager whose token manager fails the first registration. Verify automatic recovery preserves the job ID and configuration, keeps job leader monitoring active without restarting it, and allows resource declarations after registration succeeds. Generated-by: Codex (GPT-6) --- ...ourceManagerJobMasterRegistrationTest.java | 223 ++++++++++++++++++ .../TestingResourceManagerService.java | 8 + 2 files changed, 231 insertions(+) create mode 100644 flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/ResourceManagerJobMasterRegistrationTest.java diff --git a/flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/ResourceManagerJobMasterRegistrationTest.java b/flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/ResourceManagerJobMasterRegistrationTest.java new file mode 100644 index 00000000000000..14857f448f714c --- /dev/null +++ b/flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/ResourceManagerJobMasterRegistrationTest.java @@ -0,0 +1,223 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.runtime.resourcemanager; + +import org.apache.flink.api.common.JobID; +import org.apache.flink.api.java.tuple.Tuple2; +import org.apache.flink.configuration.ClusterOptions; +import org.apache.flink.configuration.Configuration; +import org.apache.flink.core.fs.AutoCloseableRegistry; +import org.apache.flink.runtime.checkpoint.StandaloneCheckpointRecoveryFactory; +import org.apache.flink.runtime.highavailability.TestingHighAvailabilityServices; +import org.apache.flink.runtime.jobgraph.JobGraph; +import org.apache.flink.runtime.jobgraph.JobGraphTestUtils; +import org.apache.flink.runtime.jobmaster.DefaultSlotPoolServiceSchedulerFactory; +import org.apache.flink.runtime.jobmaster.JobManagerSharedServices; +import org.apache.flink.runtime.jobmaster.JobMaster; +import org.apache.flink.runtime.jobmaster.TestingJobManagerSharedServicesBuilder; +import org.apache.flink.runtime.jobmaster.slotpool.TestingSlotPoolServiceBuilder; +import org.apache.flink.runtime.jobmaster.utils.JobMasterBuilder; +import org.apache.flink.runtime.leaderelection.LeaderInformation; +import org.apache.flink.runtime.leaderretrieval.LeaderRetrievalListener; +import org.apache.flink.runtime.leaderretrieval.SettableLeaderRetrievalService; +import org.apache.flink.runtime.rpc.RpcUtils; +import org.apache.flink.runtime.rpc.TestingRpcService; +import org.apache.flink.runtime.scheduler.TestingSchedulerNG; +import org.apache.flink.runtime.scheduler.TestingSchedulerNGFactory; +import org.apache.flink.runtime.security.token.NoOpDelegationTokenManager; +import org.apache.flink.runtime.slots.ResourceRequirements; +import org.apache.flink.runtime.util.TestingFatalErrorHandler; +import org.apache.flink.util.FlinkException; + +import org.junit.jupiter.api.Test; + +import java.time.Duration; +import java.util.Collections; +import java.util.Map; +import java.util.Queue; +import java.util.UUID; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.concurrent.atomic.AtomicInteger; + +import static org.apache.flink.core.testutils.FlinkAssertions.assertThatFuture; +import static org.assertj.core.api.Assertions.assertThat; + +/** Tests for automatic JobMaster registration retries with the ResourceManager. */ +class ResourceManagerJobMasterRegistrationTest { + + /** + * Verifies that the JobMaster retries automatically while preserving the job configuration and + * existing job leader monitoring. + * + *

{@link ResourceManagerJobMasterTest} covers the failure response and single token-manager + * invocation per registration request on the ResourceManager side. + */ + @Test + void testJobMasterAutomaticallyRetriesAfterDelegationTokenRegistrationFailure() + throws Exception { + try (AutoCloseableRegistry closeableRegistry = new AutoCloseableRegistry()) { + final TestingRpcService rpcService = new TestingRpcService(); + closeableRegistry.registerCloseable(() -> rpcService.closeAsync().get()); + + final JobManagerSharedServices sharedServices = + new TestingJobManagerSharedServicesBuilder().build(); + closeableRegistry.registerCloseable(sharedServices::shutdown); + + final JobGraph jobGraph = JobGraphTestUtils.singleNoOpJobGraph(); + jobGraph.getJobConfiguration() + .setString("test.job-configuration", "preserved-on-retry"); + final Map jobConfiguration = jobGraph.getJobConfiguration().toMap(); + final FailingOnceDelegationTokenManager delegationTokenManager = + new FailingOnceDelegationTokenManager(); + final AtomicInteger jobLeaderRetrieverStarts = new AtomicInteger(); + final AtomicInteger jobLeaderRetrieverStops = new AtomicInteger(); + final SettableLeaderRetrievalService jobMasterLeaderRetriever = + new SettableLeaderRetrievalService() { + @Override + public synchronized void start(LeaderRetrievalListener listener) + throws Exception { + jobLeaderRetrieverStarts.incrementAndGet(); + super.start(listener); + } + + @Override + public void stop() throws Exception { + jobLeaderRetrieverStops.incrementAndGet(); + super.stop(); + } + }; + final TestingResourceManagerService resourceManagerService = + TestingResourceManagerService.newBuilder() + .setRpcService(rpcService) + .setDelegationTokenManager(delegationTokenManager) + .setJmLeaderRetrieverFunction(ignored -> jobMasterLeaderRetriever) + .build(); + closeableRegistry.registerCloseable(resourceManagerService::rethrowFatalErrorIfAny); + closeableRegistry.registerCloseable(() -> resourceManagerService.closeAsync().get()); + + final SettableLeaderRetrievalService resourceManagerLeaderRetriever = + new SettableLeaderRetrievalService(); + final TestingHighAvailabilityServices highAvailabilityServices = + new TestingHighAvailabilityServices(); + highAvailabilityServices.setResourceManagerLeaderRetriever( + resourceManagerLeaderRetriever); + highAvailabilityServices.setCheckpointRecoveryFactory( + new StandaloneCheckpointRecoveryFactory()); + + final Configuration configuration = new Configuration(); + // Prevent timeout-driven retries from changing the expected attempt count. + configuration.set(ClusterOptions.INITIAL_REGISTRATION_TIMEOUT, RpcUtils.INF_TIMEOUT); + configuration.set(ClusterOptions.MAX_REGISTRATION_TIMEOUT, RpcUtils.INF_TIMEOUT); + configuration.set(ClusterOptions.REFUSED_REGISTRATION_DELAY, Duration.ZERO); + final CompletableFuture connectedResourceManagerFuture = + new CompletableFuture<>(); + resourceManagerService + .getFatalErrorFuture() + .thenAccept(connectedResourceManagerFuture::completeExceptionally); + final JobMasterBuilder.TestingOnCompletionActions completionActions = + new JobMasterBuilder.TestingOnCompletionActions(); + completionActions + .getJobMasterFailedFuture() + .thenAccept(connectedResourceManagerFuture::completeExceptionally); + final TestingFatalErrorHandler fatalErrorHandler = new TestingFatalErrorHandler(); + fatalErrorHandler + .getErrorFuture() + .thenAccept(connectedResourceManagerFuture::completeExceptionally); + closeableRegistry.registerCloseable(fatalErrorHandler::rethrowError); + + final JobMaster jobMaster = + new JobMasterBuilder(jobGraph, rpcService) + .withConfiguration(configuration) + .withHighAvailabilityServices(highAvailabilityServices) + .withJobManagerSharedServices(sharedServices) + .withOnCompletionActions(completionActions) + .withFatalErrorHandler(fatalErrorHandler) + .withSlotPoolServiceSchedulerFactory( + DefaultSlotPoolServiceSchedulerFactory.create( + TestingSlotPoolServiceBuilder.newBuilder() + .setConnectToResourceManagerConsumer( + connectedResourceManagerFuture + ::complete), + new TestingSchedulerNGFactory( + TestingSchedulerNG.newBuilder().build()))) + .createJobMaster(); + closeableRegistry.registerCloseable(() -> jobMaster.closeAsync().get()); + + resourceManagerService.start(); + final CompletableFuture resourceManagerLeadershipFuture = + resourceManagerService.isLeader(UUID.randomUUID()); + resourceManagerService + .getFatalErrorFuture() + .thenAccept(resourceManagerLeadershipFuture::completeExceptionally); + resourceManagerLeadershipFuture.get(); + final ResourceManagerGateway resourceManagerGateway = + resourceManagerService + .getResourceManagerGateway() + .orElseThrow( + () -> new AssertionError("ResourceManager is not available")); + + jobMasterLeaderRetriever.notifyListener( + jobMaster.getAddress(), jobMaster.getFencingToken().toUUID()); + jobMaster.start(); + resourceManagerLeaderRetriever.notifyListener( + resourceManagerGateway.getAddress(), + resourceManagerGateway.getFencingToken().toUUID()); + + final ResourceManagerGateway connectedResourceManager = + connectedResourceManagerFuture.get(); + assertThat(delegationTokenManager.registrations) + .containsExactly( + Tuple2.of(jobGraph.getJobID(), jobConfiguration), + Tuple2.of(jobGraph.getJobID(), jobConfiguration)); + assertThat(jobLeaderRetrieverStarts) + .as("the retry reuses the existing job leader monitoring") + .hasValue(1); + assertThat(jobLeaderRetrieverStops) + .as("job leader monitoring remains active after the failed registration") + .hasValue(0); + assertThatFuture( + connectedResourceManager.declareRequiredResources( + jobMaster.getFencingToken(), + ResourceRequirements.create( + jobGraph.getJobID(), + jobMaster.getAddress(), + Collections.emptyList()), + RpcUtils.INF_TIMEOUT)) + .eventuallySucceeds(); + assertThat(completionActions.getJobMasterFailedFuture()).isNotDone(); + } + } + + private static final class FailingOnceDelegationTokenManager + extends NoOpDelegationTokenManager { + + private final AtomicInteger registrationAttempts = new AtomicInteger(); + private final Queue>> registrations = + new ConcurrentLinkedQueue<>(); + + @Override + public void registerJob(JobID jobId, Configuration jobConfiguration) throws Exception { + registrations.add(Tuple2.of(jobId, jobConfiguration.toMap())); + if (registrationAttempts.incrementAndGet() == 1) { + throw new FlinkException("First delegation token registration attempt failed"); + } + } + } +} diff --git a/flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/TestingResourceManagerService.java b/flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/TestingResourceManagerService.java index 7510f0122e44f6..675423635aef21 100644 --- a/flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/TestingResourceManagerService.java +++ b/flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/TestingResourceManagerService.java @@ -119,6 +119,14 @@ public void notLeader() { leaderElection.notLeader(); } + /** + * Returns the current fatal-error future. Call again after {@link #ignoreFatalErrors()}, which + * replaces the future. + */ + public CompletableFuture getFatalErrorFuture() { + return fatalErrorHandler.getErrorFuture(); + } + public void rethrowFatalErrorIfAny() throws Exception { if (fatalErrorHandler.hasExceptionOccurred()) { fatalErrorHandler.rethrowError(); From 56bf595d678c39743fd6aaa8dc2d003197203ce0 Mon Sep 17 00:00:00 2001 From: Aleksandr Savonin Date: Fri, 25 Sep 2026 16:19:03 +0200 Subject: [PATCH 19/22] [FLINK-40019][runtime] Coalesce token requests while renewal waits Keep on-demand requests coalesced until a current-session cycle acquires the renewal lock, so a slow provider cannot fill the shared IO pool with additional waiting workers. Skip cycles whose session ended while waiting. Verify bounded dispatch, unrelated IO progress, renewed acquisition after coalescing, and rejection of waiting cycles after stop or session replacement. Clarify pending-cycle documentation and logs, including skipped stale cycles. Generated-by: Codex (GPT-6) --- .../token/DefaultDelegationTokenManager.java | 35 ++-- .../DefaultDelegationTokenManagerTest.java | 184 ++++++++++++++++++ 2 files changed, 208 insertions(+), 11 deletions(-) diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java index 6a318b086a9080..6ab2dd7d17818e 100644 --- a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java +++ b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java @@ -144,7 +144,10 @@ public class DefaultDelegationTokenManager implements DelegationTokenManager { @GuardedBy("schedulingLock") private long nextScheduledAtMillis = Long.MAX_VALUE; - /** Whether an on-demand re-obtain is scheduled but has not started executing yet (dedupe). */ + /** + * Whether an on-demand re-obtain is pending. Requests remain coalesced until a cycle of the + * current session acquires {@link #renewalCycleLock}. + */ @GuardedBy("schedulingLock") private boolean reobtainScheduled; @@ -165,11 +168,12 @@ public class DefaultDelegationTokenManager implements DelegationTokenManager { private boolean running; /** - * Incremented by every {@link #start(Listener)}. An obtain cycle captures it when it begins and - * re-checks it before notifying, so a cycle that began under an earlier leadership session - * cannot deliver into a later session. The fence gates delivery only: a stale cycle's renewal - * state is cleared by start()'s reset, and a timer it scheduled just runs a fresh, - * fence-checked cycle later. + * Incremented when {@link #start(Listener)} starts a new session. An obtain cycle captures it + * before waiting for {@link #renewalCycleLock} and re-checks it before obtaining and before + * notifying. A waiting cycle from an earlier session therefore skips the obtain, while an + * already-running cycle cannot deliver into a later session. An in-flight stale cycle's renewal + * state is cleared by start()'s reset, and a timer it scheduled runs a fresh, fence-checked + * cycle later. */ @GuardedBy("schedulingLock") private long sessionEpoch; @@ -450,8 +454,6 @@ public void start(Listener listener) throws Exception { void startTokensUpdate() { final long cycleEpoch; synchronized (schedulingLock) { - // Clear the dedupe flag so later on-demand requests can schedule a fresh cycle. - reobtainScheduled = false; // Stopped or never started: skip the cycle. The providers may already be closed // and the listener may not be set yet. if (!running) { @@ -462,6 +464,17 @@ void startTokensUpdate() { // Serialize the obtain-and-broadcast so a re-obtain racing the periodic renewal cannot run // two cycles concurrently on the (multi-threaded) IO executor and broadcast out of order. synchronized (renewalCycleLock) { + synchronized (schedulingLock) { + if (!running || cycleEpoch != sessionEpoch) { + LOG.debug( + "Skipping tokens update cycle: the manager was stopped or the session " + + "changed while waiting."); + return; + } + // Keep requests coalesced while waiting for the previous obtain, so they + // cannot fill the IO pool with workers blocked on renewalCycleLock. + reobtainScheduled = false; + } try { LOG.info("Starting tokens update task"); DelegationTokenContainer container = new DelegationTokenContainer(); @@ -500,7 +513,7 @@ void startTokensUpdate() { long effectiveDelay = maybeScheduleRenewal(renewalDelay); if (effectiveDelay >= 0) { LOG.info( - "Tokens update task started with {} delay", + "Next tokens update cycle is pending with {} delay", TimeUtils.formatWithHighestUnit(Duration.ofMillis(effectiveDelay))); } else { LOG.info("Tokens update task not rescheduled, the manager is not running"); @@ -610,8 +623,8 @@ long maybeScheduleRenewal(long delayMs) { return delayMs; } LOG.debug( - "An on-demand re-obtain is already scheduled to fire sooner, leaving it " - + "in place."); + "An on-demand re-obtain is already pending with no greater delay, " + + "leaving it in place."); return pendingInMillis; } scheduleRenewalLocked(delayMs); diff --git a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java index f3e9c8f404b79a..eaef157c162ca3 100644 --- a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java +++ b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java @@ -22,7 +22,10 @@ import org.apache.flink.configuration.Configuration; import org.apache.flink.core.security.token.DelegationTokenProvider; import org.apache.flink.core.security.token.DelegationTokenReceiver; +import org.apache.flink.core.testutils.CheckedThread; import org.apache.flink.core.testutils.ManuallyTriggeredScheduledExecutorService; +import org.apache.flink.core.testutils.OneShotLatch; +import org.apache.flink.runtime.testutils.CommonTestUtils; import org.apache.flink.util.clock.Clock; import org.apache.flink.util.clock.ManualClock; import org.apache.flink.util.concurrent.ManuallyTriggeredScheduledExecutor; @@ -31,22 +34,27 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import java.time.Duration; import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.concurrent.BrokenBarrierException; import java.util.concurrent.Callable; +import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.CountDownLatch; import java.util.concurrent.CyclicBarrier; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; @@ -882,6 +890,182 @@ public void registerJobShouldBeIdempotent() throws Exception { assertEquals(1, ExceptionThrowingDelegationTokenProvider.registeredJobs.get().size()); } + @Test + public void waitingReobtainMustKeepFurtherRequestsCoalesced() throws Exception { + final ManuallyTriggeredScheduledExecutor scheduledExecutor = + new ManuallyTriggeredScheduledExecutor(); + final List ioThreads = new CopyOnWriteArrayList<>(); + final ThreadPoolExecutor ioExecutor = + (ThreadPoolExecutor) + Executors.newFixedThreadPool( + 3, + runnable -> { + final Thread thread = new Thread(runnable); + ioThreads.add(thread); + return thread; + }); + final ManualClock clock = new ManualClock(); + final OneShotLatch blockedObtain = new OneShotLatch(); + final OneShotLatch releaseObtain = new OneShotLatch(); + final OneShotLatch pendingObtain = new OneShotLatch(); + final OneShotLatch subsequentObtain = new OneShotLatch(); + final AtomicInteger obtainCalls = new AtomicInteger(); + final DefaultDelegationTokenManager delegationTokenManager = + new DefaultDelegationTokenManager( + hermeticCooldownConfig(Duration.ofMinutes(1)), + null, + scheduledExecutor, + ioExecutor, + clock) { + @Override + protected Optional obtainDelegationTokensAndGetNextRenewal( + DelegationTokenContainer container) { + final int obtainCall = obtainCalls.incrementAndGet(); + if (obtainCall == 2) { + blockedObtain.trigger(); + releaseObtain.awaitQuietly(); + } else if (obtainCall == 3) { + pendingObtain.trigger(); + } else if (obtainCall == 4) { + subsequentObtain.trigger(); + } + return Optional.empty(); + } + }; + + try { + delegationTokenManager.start(tokens -> {}); + delegationTokenManager.reobtainDelegationTokens(); + scheduledExecutor.triggerScheduledTasks(); + blockedObtain.await(); + + clock.advanceTime(Duration.ofMinutes(1)); + delegationTokenManager.reobtainDelegationTokens(); + scheduledExecutor.triggerScheduledTasks(); + // With the first obtain parked, this state identifies the second worker waiting + // for renewalCycleLock. Requests arriving now must remain covered by that worker. + CommonTestUtils.waitUntilCondition( + () -> ioThreads.get(1).getState() == Thread.State.BLOCKED); + + for (int request = 0; request < 5; request++) { + clock.advanceTime(Duration.ofMinutes(1)); + delegationTokenManager.reobtainDelegationTokens(); + scheduledExecutor.triggerScheduledTasks(); + } + + assertThat(ioExecutor.getTaskCount()).isEqualTo(2); + assertThat(ioExecutor.getQueue()).isEmpty(); + assertThat(ioExecutor.submit(() -> "unrelated IO completed").get()) + .isEqualTo("unrelated IO completed"); + assertThat(obtainCalls).hasValue(2); + + releaseObtain.trigger(); + pendingObtain.await(); + assertThat(obtainCalls).hasValue(3); + + clock.advanceTime(Duration.ofMinutes(1)); + delegationTokenManager.reobtainDelegationTokens(); + assertThat(scheduledExecutor.getActiveScheduledTasks()).hasSize(1); + scheduledExecutor.triggerScheduledTasks(); + subsequentObtain.await(); + assertThat(obtainCalls).hasValue(4); + } finally { + releaseObtain.trigger(); + delegationTokenManager.close(); + ioExecutor.shutdownNow(); + for (Thread thread : ioThreads) { + thread.join(); + } + } + } + + @ParameterizedTest(name = "restart={0}") + @ValueSource(booleans = {false, true}) + public void waitingCycleMustNotObtainAfterSessionEnds(boolean restart) throws Exception { + final ManuallyTriggeredScheduledExecutor scheduledExecutor = + new ManuallyTriggeredScheduledExecutor(); + final ManuallyTriggeredScheduledExecutorService ioExecutor = + new ManuallyTriggeredScheduledExecutorService(); + final OneShotLatch initialObtain = new OneShotLatch(); + final OneShotLatch releaseObtain = new OneShotLatch(); + final AtomicInteger obtainCalls = new AtomicInteger(); + final DefaultDelegationTokenManager manager = + new DefaultDelegationTokenManager( + hermeticCooldownConfig(Duration.ZERO), + null, + scheduledExecutor, + ioExecutor) { + @Override + protected Optional obtainDelegationTokensAndGetNextRenewal( + DelegationTokenContainer container) { + if (obtainCalls.incrementAndGet() == 1) { + initialObtain.trigger(); + releaseObtain.awaitQuietly(); + } + return Optional.empty(); + } + }; + final CheckedThread initialStart = + new CheckedThread() { + @Override + public void go() throws Exception { + manager.start(tokens -> {}); + } + }; + final CheckedThread waitingCycle = + new CheckedThread() { + @Override + public void go() { + ioExecutor.trigger(); + } + }; + final CheckedThread nextStart = + new CheckedThread() { + @Override + public void go() throws Exception { + manager.start(tokens -> {}); + } + }; + + try { + initialStart.start(); + initialObtain.await(); + manager.reobtainDelegationTokens(); + scheduledExecutor.triggerScheduledTasks(); + assertThat(ioExecutor.numQueuedRunnables()).isEqualTo(1); + waitingCycle.start(); + CommonTestUtils.waitUntilCondition( + () -> waitingCycle.getState() == Thread.State.BLOCKED); + + manager.stop(); + if (restart) { + nextStart.start(); + // start() publishes the new epoch before waiting for the previous obtain. + CommonTestUtils.waitUntilCondition( + () -> nextStart.getState() == Thread.State.BLOCKED); + } + + releaseObtain.trigger(); + initialStart.sync(); + waitingCycle.sync(); + if (restart) { + nextStart.sync(); + } + assertThat(obtainCalls) + .as("only each session's initial cycle obtains; the old waiting cycle skips") + .hasValue(restart ? 2 : 1); + } finally { + releaseObtain.trigger(); + manager.close(); + for (CheckedThread thread : + new CheckedThread[] {initialStart, waitingCycle, nextStart}) { + if (thread.getState() != Thread.State.NEW) { + thread.sync(); + } + } + } + } + @Test public void renewalCycleLockSerializesConcurrentObtainCycles() throws Exception { final ManuallyTriggeredScheduledExecutor scheduledExecutor = From f01b9597fd518b935af81188148681809e600403 Mon Sep 17 00:00:00 2001 From: Aleksandr Savonin Date: Fri, 25 Sep 2026 16:19:13 +0200 Subject: [PATCH 20/22] [FLINK-40019][core][runtime] Clarify asynchronous token registration semantics Document that provider registration neither gates coordinator initialization nor guarantees token acquisition or distribution. On-demand acquisition is asynchronous and subject to cooldown. Remove the ResourceManager comment claiming registration rejection prevents tokenless startup. Establishing startup readiness requires a separate readiness protocol; this change clarifies the existing behavior only. Generated-by: Codex (GPT-6) --- .../core/security/token/DelegationTokenProvider.java | 7 ++++++- .../flink/runtime/resourcemanager/ResourceManager.java | 8 +++----- .../runtime/security/token/DelegationTokenManager.java | 6 ++++-- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenProvider.java b/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenProvider.java index 9effbfd15c690a..02c4e5742c6653 100644 --- a/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenProvider.java +++ b/flink-core/src/main/java/org/apache/flink/core/security/token/DelegationTokenProvider.java @@ -126,9 +126,14 @@ default void init(Configuration configuration, DelegationTokenManagerCallback ca * Called with the job's configuration when its JobMaster registers with the ResourceManager. * Re-registration may occur while the job's tasks are running. * + *

This notification does not gate job initialization: operator coordinators may already have + * started. Successful registration does not imply that tokens have been obtained or + * distributed. + * *

To get the job's tokens distributed without waiting for the periodic renewal, call {@link * DelegationTokenManagerCallback#reobtainDelegationTokens()} on the callback handed to {@link - * #init(Configuration, DelegationTokenManagerCallback)} to request an immediate obtain cycle. + * #init(Configuration, DelegationTokenManagerCallback)} to request an asynchronous obtain + * cycle, subject to the configured cooldown. * *

A provider that requests a re-obtain must record this job's per-job state before * invoking {@link DelegationTokenManagerCallback#reobtainDelegationTokens()}. That call merely diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/ResourceManager.java b/flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/ResourceManager.java index 918f2c971a1a4f..9b78d603106440 100755 --- a/flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/ResourceManager.java +++ b/flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/ResourceManager.java @@ -431,11 +431,9 @@ public CompletableFuture registerJobMaster( jobMasterIdFuture, (JobMasterGateway jobMasterGateway, JobMasterId leadingJobMasterId) -> { if (Objects.equals(leadingJobMasterId, jobMasterId)) { - // Register with the delegation token manager first, so a - // provider failure rejects the registration and the job does - // not start without the tokens it requires. LinkageError is - // caught so a plugin classpath failure is reported the same - // way. + // Reject a failed provider registration before installing the + // JobMaster registration. Report plugin linkage errors as + // registration failures too. try { delegationTokenManager.registerJob(jobId, jobConfiguration); } catch (Exception | LinkageError e) { diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DelegationTokenManager.java b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DelegationTokenManager.java index 18db63e3788764..6441a7bca31326 100644 --- a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DelegationTokenManager.java +++ b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DelegationTokenManager.java @@ -97,9 +97,11 @@ default void reobtainDelegationTokens() {} * registration is currently tracked for the job, the manager calls {@link * org.apache.flink.core.security.token.DelegationTokenProvider#unregisterJob(JobID)} on all * providers to attempt rollback. Otherwise, it keeps the existing registration and does not - * attempt rollback, because the job's tasks may still be running. A provider that needs the - * job's tokens distributed immediately requests it via {@link + * attempt rollback, because the job's tasks may still be running. A provider can request an + * asynchronous obtain cycle, subject to the configured cooldown, via {@link * org.apache.flink.core.security.token.DelegationTokenManagerCallback#reobtainDelegationTokens()}. + * Successful registration does not imply that tokens have been obtained or distributed, and + * does not gate job initialization. * * @param jobId The ID of the job being registered. * @param jobConfiguration The job's configuration. From 0f60c5c619d2a927c12008c846c226829d41fb87 Mon Sep 17 00:00:00 2001 From: Aleksandr Savonin Date: Fri, 25 Sep 2026 16:56:29 +0200 Subject: [PATCH 21/22] [FLINK-40019][runtime] Retry rejected delegation token renewal submissions Executor rejection can indicate saturation rather than shutdown. Retry live IO dispatch failures and use an independent timer when scheduling rejects work, preserving coalescing and fencing stale rejection handlers. Use the configured initial submission backoff with a one-second minimum. Log the first rejection with its stack trace, then summarize repeated rejections at most once per minute. Reset the warning episode when a cycle starts or the manager stops. Add deterministic tests for recovery, rejection races, retry delays, warning rate limits and lifecycle resets. Verify regressions against the old behavior and mutation-test the stale-rejection guard. Generated-by: Codex (GPT-6) --- .../token/DefaultDelegationTokenManager.java | 199 ++++++-- .../DefaultDelegationTokenManagerTest.java | 430 ++++++++++++++++++ 2 files changed, 588 insertions(+), 41 deletions(-) diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java index 6ab2dd7d17818e..ae4d8314a484e6 100644 --- a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java +++ b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java @@ -31,6 +31,7 @@ import org.apache.flink.util.TimeUtils; import org.apache.flink.util.clock.Clock; import org.apache.flink.util.clock.SystemClock; +import org.apache.flink.util.concurrent.FutureUtils; import org.apache.flink.util.concurrent.ScheduledExecutor; import org.slf4j.Logger; @@ -46,10 +47,11 @@ import java.util.Optional; import java.util.ServiceLoader; import java.util.Set; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutorService; +import java.util.concurrent.Future; import java.util.concurrent.RejectedExecutionException; -import java.util.concurrent.ScheduledFuture; import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; @@ -86,6 +88,10 @@ public class DefaultDelegationTokenManager implements DelegationTokenManager { private static final long NO_PREVIOUS_REOBTAIN = Long.MIN_VALUE; + private static final long MIN_SUBMISSION_RETRY_DELAY_MILLIS = 1_000L; + + private static final long SUBMISSION_REJECTION_WARN_INTERVAL_MILLIS = 60_000L; + private final Configuration configuration; @Nullable private final PluginManager pluginManager; @@ -114,9 +120,9 @@ public class DefaultDelegationTokenManager implements DelegationTokenManager { private final Clock clock; /** - * Serializes the obtain-and-broadcast cycle so that, even though {@code cancel(true)} does not - * wait for an in-flight cycle and the IO executor is multi-threaded, two cycles can never run - * concurrently and broadcast tokens out of order. + * Serializes the obtain-and-broadcast cycle so that, even though cancelling a scheduled + * dispatch does not wait for an in-flight cycle and the IO executor is multi-threaded, two + * cycles can never run concurrently and broadcast tokens out of order. */ private final Object renewalCycleLock = new Object(); @@ -132,7 +138,17 @@ public class DefaultDelegationTokenManager implements DelegationTokenManager { @GuardedBy("schedulingLock") @Nullable - private ScheduledFuture tokensUpdateFuture; + private Future tokensUpdateFuture; + + /** Fences dispatch and rejection handling after a pending cycle is replaced or stopped. */ + @GuardedBy("schedulingLock") + private long renewalTaskGeneration; + + @GuardedBy("schedulingLock") + private long consecutiveSubmissionRejections; + + @GuardedBy("schedulingLock") + private long lastSubmissionRejectionWarnAtMillis; /** * Relative (monotonic) clock time (millis) at which {@link #tokensUpdateFuture} is scheduled to @@ -474,6 +490,8 @@ void startTokensUpdate() { // Keep requests coalesced while waiting for the previous obtain, so they // cannot fill the IO pool with workers blocked on renewalCycleLock. reobtainScheduled = false; + consecutiveSubmissionRejections = 0; + lastSubmissionRejectionWarnAtMillis = 0; } try { LOG.info("Starting tokens update task"); @@ -516,7 +534,9 @@ void startTokensUpdate() { "Next tokens update cycle is pending with {} delay", TimeUtils.formatWithHighestUnit(Duration.ofMillis(effectiveDelay))); } else { - LOG.info("Tokens update task not rescheduled, the manager is not running"); + LOG.info( + "Tokens update task not rescheduled, the manager or IO executor " + + "is shutting down"); } } else { LOG.warn( @@ -542,8 +562,8 @@ void startTokensUpdate() { e); } else { LOG.warn( - "Failed to update tokens, no retry scheduled because the manager is " - + "not running", + "Failed to update tokens, no retry scheduled because the manager or " + + "IO executor is shutting down", e); } } @@ -555,44 +575,134 @@ void startTokensUpdate() { * pending renewal. A delay of {@code 0} brings the next cycle forward to now. Must only be * called after {@link #start(Listener)} (the scheduled and IO executors are non-null then) and * while holding {@link #schedulingLock}. + * + * @return the effective delay, including a submission retry, or -1 if the IO executor is shut + * down. */ @GuardedBy("schedulingLock") - private void scheduleRenewalLocked(long delayMs) { + private long scheduleRenewalLocked(long delayMs) { stopTokensUpdate(); + final long cycleEpoch = sessionEpoch; + final long generation = renewalTaskGeneration; nextScheduledAtMillis = clock.relativeTimeMillis() + delayMs; try { - tokensUpdateFuture = - scheduledExecutor.schedule( - () -> { - try { - ioExecutor.execute(this::startTokensUpdate); - } catch (RejectedExecutionException e) { - // IO executor is shutting down: drop the cycle but release the - // dedupe flag so it cannot get stuck if the manager is reused. - synchronized (schedulingLock) { - reobtainScheduled = false; - } - LOG.debug("Tokens update task rejected by IO executor", e); + try { + tokensUpdateFuture = + scheduledExecutor.schedule( + () -> dispatchTokensUpdate(cycleEpoch, generation), + delayMs, + TimeUnit.MILLISECONDS); + return delayMs; + } catch (RejectedExecutionException e) { + // The scheduler exposes no shutdown state. IO shutdown makes retrying futile + // because token acquisition can no longer be dispatched. + if (ioExecutor.isShutdown()) { + reobtainScheduled = false; + nextScheduledAtMillis = Long.MAX_VALUE; + LOG.debug("Tokens update scheduling rejected during IO executor shutdown", e); + return -1L; + } + // The scheduler may be saturated rather than shut down. Use an independent + // timer to retry submission, without obtaining tokens on its shared thread. + final long retryDelay = + Math.max( + delayMs, + Math.max( + MIN_SUBMISSION_RETRY_DELAY_MILLIS, + renewalRetryInitialBackoff)); + final CompletableFuture retryFuture = new CompletableFuture<>(); + tokensUpdateFuture = retryFuture; + nextScheduledAtMillis = clock.relativeTimeMillis() + retryDelay; + retryFuture.thenRun( + () -> { + synchronized (schedulingLock) { + if (!running + || cycleEpoch != sessionEpoch + || generation != renewalTaskGeneration + || tokensUpdateFuture != retryFuture) { + return; } - }, - delayMs, - TimeUnit.MILLISECONDS); - } catch (RejectedExecutionException e) { - // Scheduled executor is shutting down: no cycle will run, so undo the bookkeeping - // this method set. - reobtainScheduled = false; - nextScheduledAtMillis = Long.MAX_VALUE; - LOG.debug("Tokens update task rejected by scheduled executor", e); + scheduleRenewalLocked(0L); + } + }); + completeSchedulingRetry(retryFuture, retryDelay); + logSubmissionRejection("scheduled executor", retryDelay, e); + return retryDelay; + } } catch (Throwable t) { - // Undo the same bookkeeping as the rejection branch, or every later re-obtain would - // be coalesced against a cycle that never got scheduled. Rethrow to keep the failure - // visible. + // A failed submission must not leave later requests coalescing against missing work. + stopTokensUpdate(); reobtainScheduled = false; - nextScheduledAtMillis = Long.MAX_VALUE; + // Submission may run in a future continuation that would otherwise hide the failure. + LOG.error("Failed to schedule tokens update task", t); throw t; } } + private void dispatchTokensUpdate(long cycleEpoch, long generation) { + synchronized (schedulingLock) { + if (!running || cycleEpoch != sessionEpoch || generation != renewalTaskGeneration) { + return; + } + } + try { + ioExecutor.execute(this::startTokensUpdate); + } catch (RejectedExecutionException e) { + synchronized (schedulingLock) { + if (!running || cycleEpoch != sessionEpoch || generation != renewalTaskGeneration) { + return; + } + if (ioExecutor.isShutdown()) { + stopTokensUpdate(); + reobtainScheduled = false; + LOG.debug("Tokens update task rejected during IO executor shutdown", e); + return; + } + final long retryDelay = + scheduleRenewalLocked( + Math.max( + MIN_SUBMISSION_RETRY_DELAY_MILLIS, + renewalRetryInitialBackoff)); + if (retryDelay >= 0) { + logSubmissionRejection("IO executor", retryDelay, e); + } else { + LOG.debug("Tokens update retry rejected during IO executor shutdown", e); + } + } + } + } + + @GuardedBy("schedulingLock") + private void logSubmissionRejection( + String executor, long retryDelay, RejectedExecutionException rejection) { + consecutiveSubmissionRejections++; + final long now = clock.relativeTimeMillis(); + final String formattedDelay = + TimeUtils.formatWithHighestUnit(Duration.ofMillis(retryDelay)); + if (consecutiveSubmissionRejections == 1) { + lastSubmissionRejectionWarnAtMillis = now; + LOG.warn( + "Token update submission rejected by {}, will retry in {}", + executor, + formattedDelay, + rejection); + } else if (now - lastSubmissionRejectionWarnAtMillis + >= SUBMISSION_REJECTION_WARN_INTERVAL_MILLIS) { + lastSubmissionRejectionWarnAtMillis = now; + LOG.warn( + "Token update submissions rejected {} times without starting a cycle; " + + "latest rejection from {}, will retry in {}", + consecutiveSubmissionRejections, + executor, + formattedDelay); + } else { + LOG.debug( + "Token update submission rejected again by {}, will retry in {}", + executor, + formattedDelay); + } + } + /** * Schedules the next cycle (periodic renewal or failure retry). A pending on-demand cycle is * brought forward when {@code delayMs} is sooner and left in place otherwise, so a pending @@ -600,7 +710,7 @@ private void scheduleRenewalLocked(long delayMs) { * * @param delayMs requested delay in millis * @return the delay in millis until the cycle that will actually run next, or -1 when nothing - * is scheduled because the manager is not running. + * is scheduled because the manager is not running or the IO executor is shut down. */ @VisibleForTesting long maybeScheduleRenewal(long delayMs) { @@ -619,30 +729,35 @@ long maybeScheduleRenewal(long delayMs) { // reobtainScheduled set, so coalescing still holds. Move the cooldown anchor // to the time the cycle now actually runs. lastReobtainAtMillis = clock.relativeTimeMillis() + delayMs; - scheduleRenewalLocked(delayMs); - return delayMs; + return scheduleRenewalLocked(delayMs); } LOG.debug( "An on-demand re-obtain is already pending with no greater delay, " + "leaving it in place."); return pendingInMillis; } - scheduleRenewalLocked(delayMs); - return delayMs; + return scheduleRenewalLocked(delayMs); } } @VisibleForTesting void stopTokensUpdate() { synchronized (schedulingLock) { + renewalTaskGeneration++; if (tokensUpdateFuture != null) { - tokensUpdateFuture.cancel(true); + // A dispatch can reschedule itself after rejection; do not interrupt its thread. + tokensUpdateFuture.cancel(false); tokensUpdateFuture = null; - nextScheduledAtMillis = Long.MAX_VALUE; } + nextScheduledAtMillis = Long.MAX_VALUE; } } + @VisibleForTesting + void completeSchedulingRetry(CompletableFuture retryFuture, long delayMillis) { + FutureUtils.completeDelayed(retryFuture, null, Duration.ofMillis(delayMillis)); + } + @VisibleForTesting long calculateRetryDelay(Clock clock) { long nowMillis = clock.absoluteTimeMillis(); @@ -694,6 +809,8 @@ public void stop() { running = false; stopTokensUpdate(); reobtainScheduled = false; + consecutiveSubmissionRejections = 0; + lastSubmissionRejectionWarnAtMillis = 0; lastReobtainAtMillis = NO_PREVIOUS_REOBTAIN; // Release the listener: keeping it would pin the disposed ResourceManager of a // revoked leadership session, forever on a standby that never regains leadership. diff --git a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java index eaef157c162ca3..c8d67798731f9e 100644 --- a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java +++ b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java @@ -26,18 +26,24 @@ import org.apache.flink.core.testutils.ManuallyTriggeredScheduledExecutorService; import org.apache.flink.core.testutils.OneShotLatch; import org.apache.flink.runtime.testutils.CommonTestUtils; +import org.apache.flink.testutils.logging.LoggerAuditingExtension; import org.apache.flink.util.clock.Clock; import org.apache.flink.util.clock.ManualClock; import org.apache.flink.util.concurrent.ManuallyTriggeredScheduledExecutor; import org.apache.flink.util.concurrent.ScheduledExecutor; +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.core.LogEvent; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import org.junit.jupiter.params.provider.ValueSource; import java.time.Duration; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.HashMap; @@ -48,11 +54,13 @@ import java.util.Set; import java.util.concurrent.BrokenBarrierException; import java.util.concurrent.Callable; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.CountDownLatch; import java.util.concurrent.CyclicBarrier; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; @@ -61,6 +69,7 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; +import java.util.stream.Collectors; import static org.apache.flink.configuration.ConfigurationUtils.getBooleanConfigOption; import static org.apache.flink.configuration.SecurityOptions.DELEGATION_TOKENS_RENEWAL_RETRY_INITIAL_BACKOFF; @@ -80,6 +89,11 @@ /** Test for {@link DelegationTokenManager}. */ public class DefaultDelegationTokenManagerTest { + @RegisterExtension + private final LoggerAuditingExtension loggerAuditingExtension = + new LoggerAuditingExtension( + DefaultDelegationTokenManager.class, org.slf4j.event.Level.DEBUG); + @BeforeEach public void beforeEach() { ExceptionThrowingDelegationTokenProvider.reset(); @@ -758,6 +772,342 @@ public void execute(Runnable command) { "A re-obtain after a scheduler failure must schedule a fresh obtain cycle"); } + @ParameterizedTest(name = "configured={0}ms, submission retry={1}ms") + @CsvSource({"0, 1000", "100, 1000", "2500, 2500"}) + public void submissionRetryDelayMustRespectMinimumAndConfiguredBackoff( + long configuredBackoffMillis, long expectedRetryMillis) throws Exception { + try (SchedulingRejectionTestContext context = + new SchedulingRejectionTestContext( + false, Duration.ofMillis(configuredBackoffMillis))) { + context.manager.start(tokens -> {}); + context.rejectScheduling = true; + context.manager.reobtainDelegationTokens(); + assertThat(onlyScheduledDelayMillis(context.retryExecutor)) + .isEqualTo(expectedRetryMillis); + + context.clock.advanceTime(Duration.ofMillis(expectedRetryMillis)); + context.retryExecutor.triggerNonPeriodicScheduledTask(); + assertThat(onlyScheduledDelayMillis(context.retryExecutor)) + .isEqualTo(expectedRetryMillis); + + context.rejectScheduling = false; + context.clock.advanceTime(Duration.ofMillis(expectedRetryMillis)); + context.retryExecutor.triggerNonPeriodicScheduledTask(); + context.rejectIoExecution = true; + context.scheduledExecutor.triggerNonPeriodicScheduledTask(); + assertThat(onlyScheduledDelayMillis(context.scheduledExecutor)) + .isEqualTo(expectedRetryMillis); + + context.clock.advanceTime(Duration.ofMillis(expectedRetryMillis)); + context.scheduledExecutor.triggerNonPeriodicScheduledTask(); + assertThat(onlyScheduledDelayMillis(context.scheduledExecutor)) + .isEqualTo(expectedRetryMillis); + + context.rejectIoExecution = false; + context.clock.advanceTime(Duration.ofMillis(expectedRetryMillis)); + context.scheduledExecutor.triggerNonPeriodicScheduledTask(); + context.ioExecutor.triggerAll(); + assertThat(context.obtains).hasValue(2); + } + } + + @Test + public void submissionRejectionsMustShareRateLimitedWarnings() throws Exception { + try (SchedulingRejectionTestContext context = new SchedulingRejectionTestContext(false)) { + context.manager.start(tokens -> {}); + context.manager.reobtainDelegationTokens(); + context.rejectIoExecution = true; + context.rejectScheduling = true; + context.scheduledExecutor.triggerNonPeriodicScheduledTask(); + + final List initialRejections = submissionRejectionEvents(); + assertThat(initialRejections) + .extracting(LogEvent::getLevel) + .containsExactly(Level.WARN, Level.DEBUG); + assertThat(initialRejections.get(0).getThrown()) + .isInstanceOf(RejectedExecutionException.class); + assertThat(initialRejections.get(1).getThrown()).isNull(); + + context.clock.advanceTime(Duration.ofMillis(59_999L)); + context.retryExecutor.triggerNonPeriodicScheduledTask(); + assertThat(submissionRejectionEvents()) + .extracting(LogEvent::getLevel) + .containsExactly(Level.WARN, Level.DEBUG, Level.DEBUG); + + context.clock.advanceTime(Duration.ofMillis(1L)); + context.manager.maybeScheduleRenewal(0L); + final List rejections = submissionRejectionEvents(); + assertThat(rejections) + .extracting(LogEvent::getLevel) + .containsExactly(Level.WARN, Level.DEBUG, Level.DEBUG, Level.WARN); + assertThat(rejections.subList(1, rejections.size())) + .allSatisfy(event -> assertThat(event.getThrown()).isNull()); + assertThat(rejections.get(3).getMessage().getFormattedMessage()).contains("4 times"); + } + } + + @ParameterizedTest(name = "restart={0}") + @ValueSource(booleans = {false, true}) + public void successfulCycleOrRestartMustResetSubmissionRejectionWarnings(boolean restart) + throws Exception { + try (SchedulingRejectionTestContext context = new SchedulingRejectionTestContext(false)) { + context.manager.start(tokens -> {}); + context.rejectIoExecution = true; + context.manager.reobtainDelegationTokens(); + context.scheduledExecutor.triggerNonPeriodicScheduledTask(); + context.clock.advanceTime( + Duration.ofMillis(SchedulingRejectionTestContext.RETRY_DELAY_MILLIS)); + context.scheduledExecutor.triggerNonPeriodicScheduledTask(); + + // Accepting a retry timer is not recovery while the IO executor still rejects it. + assertThat(submissionRejectionEvents()) + .extracting(LogEvent::getLevel) + .containsExactly(Level.WARN, Level.DEBUG); + + context.rejectIoExecution = false; + if (restart) { + context.manager.stop(); + context.manager.start(tokens -> {}); + context.scheduledExecutor.triggerScheduledTasks(); + } else { + context.clock.advanceTime( + Duration.ofMillis(SchedulingRejectionTestContext.RETRY_DELAY_MILLIS)); + context.scheduledExecutor.triggerNonPeriodicScheduledTask(); + context.ioExecutor.triggerAll(); + } + assertThat(context.obtains).hasValue(2); + + context.rejectIoExecution = true; + context.manager.reobtainDelegationTokens(); + context.scheduledExecutor.triggerNonPeriodicScheduledTask(); + assertThat(submissionRejectionEvents()) + .extracting(LogEvent::getLevel) + .containsExactly(Level.WARN, Level.DEBUG, Level.WARN); + assertThat(submissionRejectionEvents().get(2).getThrown()) + .isInstanceOf(RejectedExecutionException.class); + + context.clock.advanceTime(Duration.ofMinutes(1)); + context.scheduledExecutor.triggerNonPeriodicScheduledTask(); + final List rejections = submissionRejectionEvents(); + assertThat(rejections) + .extracting(LogEvent::getLevel) + .containsExactly(Level.WARN, Level.DEBUG, Level.WARN, Level.WARN); + assertThat(rejections.get(3).getThrown()).isNull(); + assertThat(rejections.get(3).getMessage().getFormattedMessage()).contains("2 times"); + } + } + + private List submissionRejectionEvents() { + return loggerAuditingExtension.getEvents().stream() + .filter( + event -> + event.getMessage() + .getFormattedMessage() + .startsWith("Token update submission")) + .collect(Collectors.toList()); + } + + @ParameterizedTest(name = "periodic={0}") + @ValueSource(booleans = {false, true}) + public void schedulerRejectionMustRetryWithoutAnotherRequest(boolean periodic) + throws Exception { + try (SchedulingRejectionTestContext context = + new SchedulingRejectionTestContext(periodic)) { + context.rejectScheduling = true; + context.manager.start(tokens -> {}); + if (!periodic) { + context.manager.reobtainDelegationTokens(); + } + + assertThat(context.obtains).hasValue(1); + assertThat(context.scheduledExecutor.getActiveScheduledTasks()).isEmpty(); + final long retryDelay = + periodic + ? SchedulingRejectionTestContext.RENEWAL_DELAY_MILLIS + : SchedulingRejectionTestContext.RETRY_DELAY_MILLIS; + assertThat(onlyScheduledDelayMillis(context.retryExecutor)).isEqualTo(retryDelay); + + context.rejectScheduling = false; + context.clock.advanceTime(Duration.ofMillis(retryDelay)); + context.retryExecutor.triggerNonPeriodicScheduledTask(); + assertThat(onlyScheduledDelayMillis(context.scheduledExecutor)).isZero(); + context.scheduledExecutor.triggerNonPeriodicScheduledTask(); + context.ioExecutor.triggerAll(); + + assertThat(context.obtains).hasValue(2); + assertThat(context.retryExecutor.getActiveScheduledTasks()).isEmpty(); + if (periodic) { + assertThat(onlyScheduledDelayMillis(context.scheduledExecutor)) + .isEqualTo(SchedulingRejectionTestContext.RENEWAL_DELAY_MILLIS); + } else { + assertThat(context.scheduledExecutor.getActiveScheduledTasks()).isEmpty(); + } + } + } + + @Test + public void rejectedOnDemandReplacementMustRetainRenewalProgress() throws Exception { + try (SchedulingRejectionTestContext context = new SchedulingRejectionTestContext(true)) { + context.manager.start(tokens -> {}); + assertThat(onlyScheduledDelayMillis(context.scheduledExecutor)) + .isEqualTo(SchedulingRejectionTestContext.RENEWAL_DELAY_MILLIS); + + context.rejectScheduling = true; + context.manager.reobtainDelegationTokens(); + assertThat(context.scheduledExecutor.getActiveScheduledTasks()).isEmpty(); + assertThat(onlyScheduledDelayMillis(context.retryExecutor)) + .isEqualTo(SchedulingRejectionTestContext.RETRY_DELAY_MILLIS); + + context.rejectScheduling = false; + context.clock.advanceTime( + Duration.ofMillis(SchedulingRejectionTestContext.RETRY_DELAY_MILLIS)); + context.retryExecutor.triggerNonPeriodicScheduledTask(); + context.scheduledExecutor.triggerScheduledTasks(); + context.ioExecutor.triggerAll(); + + assertThat(context.obtains).hasValue(2); + assertThat(context.retryExecutor.getActiveScheduledTasks()).isEmpty(); + assertThat(onlyScheduledDelayMillis(context.scheduledExecutor)) + .isEqualTo(SchedulingRejectionTestContext.RENEWAL_DELAY_MILLIS); + } + } + + @Test + public void ioRejectionMustRetryWithoutAnotherRequest() throws Exception { + try (SchedulingRejectionTestContext context = new SchedulingRejectionTestContext(true)) { + context.manager.start(tokens -> {}); + context.rejectIoExecution = true; + context.clock.advanceTime( + Duration.ofMillis(SchedulingRejectionTestContext.RENEWAL_DELAY_MILLIS)); + context.scheduledExecutor.triggerNonPeriodicScheduledTask(); + + assertThat(context.obtains).hasValue(1); + assertThat(context.ioExecutor.numQueuedRunnables()).isZero(); + assertThat(onlyScheduledDelayMillis(context.scheduledExecutor)) + .isEqualTo(SchedulingRejectionTestContext.RETRY_DELAY_MILLIS); + + context.rejectIoExecution = false; + context.clock.advanceTime( + Duration.ofMillis(SchedulingRejectionTestContext.RETRY_DELAY_MILLIS)); + context.scheduledExecutor.triggerNonPeriodicScheduledTask(); + context.ioExecutor.triggerAll(); + + assertThat(context.obtains).hasValue(2); + assertThat(onlyScheduledDelayMillis(context.scheduledExecutor)) + .isEqualTo(SchedulingRejectionTestContext.RENEWAL_DELAY_MILLIS); + } + } + + @Test + public void staleIoRejectionMustNotReplaceNewerCycle() throws Exception { + try (SchedulingRejectionTestContext context = new SchedulingRejectionTestContext(true)) { + context.manager.start(tokens -> {}); + context.rejectIoExecution = true; + context.beforeIoRejection = () -> context.manager.maybeScheduleRenewal(123L); + context.clock.advanceTime( + Duration.ofMillis(SchedulingRejectionTestContext.RENEWAL_DELAY_MILLIS)); + + // Replace the cycle after dispatch begins but before the rejection is handled. + context.scheduledExecutor.triggerNonPeriodicScheduledTask(); + assertThat(onlyScheduledDelayMillis(context.scheduledExecutor)).isEqualTo(123L); + assertThat(context.obtains).hasValue(1); + + context.beforeIoRejection = () -> {}; + context.rejectIoExecution = false; + context.clock.advanceTime(Duration.ofMillis(123L)); + context.scheduledExecutor.triggerNonPeriodicScheduledTask(); + context.ioExecutor.triggerAll(); + + assertThat(context.obtains).hasValue(2); + assertThat(context.retryExecutor.getActiveScheduledTasks()).isEmpty(); + assertThat(onlyScheduledDelayMillis(context.scheduledExecutor)) + .isEqualTo(SchedulingRejectionTestContext.RENEWAL_DELAY_MILLIS); + } + } + + @Test + public void ioShutdownMustNotScheduleAnotherRetry() throws Exception { + try (SchedulingRejectionTestContext context = new SchedulingRejectionTestContext(true)) { + context.manager.start(tokens -> {}); + context.ioShutdown = true; + context.scheduledExecutor.triggerNonPeriodicScheduledTask(); + + assertThat(context.obtains).hasValue(1); + assertThat(context.ioExecutor.numQueuedRunnables()).isZero(); + assertThat(context.scheduledExecutor.getActiveScheduledTasks()).isEmpty(); + assertThat(context.retryExecutor.getActiveScheduledTasks()).isEmpty(); + } + } + + @Test + public void repeatedSchedulerRejectionsMustKeepRequestsCoalesced() throws Exception { + try (SchedulingRejectionTestContext context = new SchedulingRejectionTestContext(false)) { + context.manager.start(tokens -> {}); + context.rejectScheduling = true; + context.manager.reobtainDelegationTokens(); + context.manager.reobtainDelegationTokens(); + assertThat(context.schedulingRetries).hasSize(1); + + context.clock.advanceTime( + Duration.ofMillis(SchedulingRejectionTestContext.RETRY_DELAY_MILLIS)); + context.retryExecutor.triggerNonPeriodicScheduledTask(); + context.manager.reobtainDelegationTokens(); + context.manager.reobtainDelegationTokens(); + + assertThat(context.schedulingRetries).hasSize(2); + assertThat(onlyScheduledDelayMillis(context.retryExecutor)) + .isEqualTo(SchedulingRejectionTestContext.RETRY_DELAY_MILLIS); + assertThat(context.scheduledExecutor.getActiveScheduledTasks()).isEmpty(); + assertThat(context.obtains).hasValue(1); + + context.rejectScheduling = false; + context.clock.advanceTime( + Duration.ofMillis(SchedulingRejectionTestContext.RETRY_DELAY_MILLIS)); + context.retryExecutor.triggerNonPeriodicScheduledTask(); + context.scheduledExecutor.triggerNonPeriodicScheduledTask(); + context.ioExecutor.triggerAll(); + + assertThat(context.obtains).hasValue(2); + assertThat(context.retryExecutor.getActiveScheduledTasks()).isEmpty(); + assertThat(context.scheduledExecutor.getActiveScheduledTasks()).isEmpty(); + } + } + + @ParameterizedTest(name = "restart={0}") + @ValueSource(booleans = {false, true}) + public void stoppedSessionSchedulingRetryMustNotSubmitWork(boolean restart) throws Exception { + try (SchedulingRejectionTestContext context = new SchedulingRejectionTestContext(false)) { + context.manager.start(tokens -> {}); + context.rejectScheduling = true; + context.manager.reobtainDelegationTokens(); + assertThat(context.schedulingRetries).hasSize(1); + + context.manager.stop(); + assertThat(context.schedulingRetries.get(0)).isCancelled(); + context.rejectScheduling = false; + if (restart) { + context.manager.start(tokens -> {}); + context.manager.reobtainDelegationTokens(); + } + + // The delayed completion can still run after its logical retry was cancelled. + context.retryExecutor.triggerNonPeriodicScheduledTask(); + assertThat(context.retryExecutor.getActiveScheduledTasks()).isEmpty(); + assertThat(context.ioExecutor.numQueuedRunnables()).isZero(); + if (restart) { + assertThat(context.scheduledExecutor.getAllScheduledTasks()).hasSize(1); + context.manager.reobtainDelegationTokens(); + assertThat(context.scheduledExecutor.getAllScheduledTasks()).hasSize(1); + context.scheduledExecutor.triggerNonPeriodicScheduledTask(); + context.ioExecutor.triggerAll(); + assertThat(context.obtains).hasValue(3); + } else { + assertThat(context.scheduledExecutor.getActiveScheduledTasks()).isEmpty(); + assertThat(context.obtains).hasValue(1); + } + } + } + @Test public void stopShouldKeepProvidersUsableForSubsequentStart() throws Exception { final ManuallyTriggeredScheduledExecutor scheduledExecutor = @@ -1603,6 +1953,86 @@ public void registerJobMustNotExposeCallersConfigurationToProviders() throws Exc "A provider-side mutation must not be visible in the caller's configuration"); } + private static final class SchedulingRejectionTestContext implements AutoCloseable { + private static final long RETRY_DELAY_MILLIS = 1000L; + private static final long RENEWAL_DELAY_MILLIS = 10_000L; + + private final ManualClock clock = new ManualClock(); + private final AtomicInteger obtains = new AtomicInteger(); + private final List> schedulingRetries = new ArrayList<>(); + private final ManuallyTriggeredScheduledExecutor retryExecutor = + new ManuallyTriggeredScheduledExecutor(); + private boolean rejectScheduling; + private boolean rejectIoExecution; + private boolean ioShutdown; + private Runnable beforeIoRejection = () -> {}; + private final ManuallyTriggeredScheduledExecutor scheduledExecutor = + new ManuallyTriggeredScheduledExecutor() { + @Override + public ScheduledFuture schedule( + Runnable command, long delay, TimeUnit unit) { + if (rejectScheduling) { + throw new RejectedExecutionException("scheduler is saturated"); + } + return super.schedule(command, delay, unit); + } + }; + private final ManuallyTriggeredScheduledExecutorService ioExecutor = + new ManuallyTriggeredScheduledExecutorService() { + @Override + public void execute(Runnable command) { + if (rejectIoExecution || ioShutdown) { + beforeIoRejection.run(); + throw new RejectedExecutionException("IO executor is saturated"); + } + super.execute(command); + } + + @Override + public boolean isShutdown() { + return ioShutdown; + } + }; + private final DefaultDelegationTokenManager manager; + + private SchedulingRejectionTestContext(boolean periodic) { + this(periodic, Duration.ofMillis(RETRY_DELAY_MILLIS)); + } + + private SchedulingRejectionTestContext(boolean periodic, Duration retryBackoff) { + final Configuration configuration = hermeticCooldownConfig(Duration.ZERO); + configuration.set(DELEGATION_TOKENS_RENEWAL_RETRY_INITIAL_BACKOFF, retryBackoff); + configuration.set(DELEGATION_TOKENS_RENEWAL_TIME_RATIO, 1.0); + manager = + new DefaultDelegationTokenManager( + configuration, null, scheduledExecutor, ioExecutor, clock) { + @Override + protected Optional obtainDelegationTokensAndGetNextRenewal( + DelegationTokenContainer container) { + obtains.incrementAndGet(); + return periodic + ? Optional.of(clock.absoluteTimeMillis() + RENEWAL_DELAY_MILLIS) + : Optional.empty(); + } + + @Override + void completeSchedulingRetry( + CompletableFuture retryFuture, long delayMillis) { + schedulingRetries.add(retryFuture); + retryExecutor.schedule( + () -> retryFuture.complete(null), + delayMillis, + TimeUnit.MILLISECONDS); + } + }; + } + + @Override + public void close() { + manager.close(); + } + } + /** * Configuration for cooldown-scheduling tests: sets the cooldown and disables all providers * that could fail the obtain cycle (hadoopfs/hbase need a real Hadoop setup, and the throw From 07593f9fc07432c31a2c5400c4d6ff37ce4cbd73 Mon Sep 17 00:00:00 2001 From: Aleksandr Savonin Date: Fri, 25 Sep 2026 19:53:09 +0200 Subject: [PATCH 22/22] [FLINK-40019][runtime] Anchor token re-obtain cooldown to cycle start Record the monotonic cooldown anchor when a current-session cycle starts serving pending demand. Scheduling or bringing work forward must not move the anchor before acquisition starts. Cover queued renewal and retry workers, delayed IO execution, initial and bring-forward scheduling failures, periodic cycles without demand, and requests arriving during acquisition. Generated-by: Codex (GPT-6 Astra) --- .../token/DefaultDelegationTokenManager.java | 23 +- .../DefaultDelegationTokenManagerTest.java | 245 +++++++++++++++++- 2 files changed, 252 insertions(+), 16 deletions(-) diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java index ae4d8314a484e6..7da9a5aa29bb73 100644 --- a/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java +++ b/flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java @@ -168,10 +168,9 @@ public class DefaultDelegationTokenManager implements DelegationTokenManager { private boolean reobtainScheduled; /** - * Relative (monotonic) clock time (millis) at which the last on-demand re-obtain cycle was - * scheduled to execute, or {@link #NO_PREVIOUS_REOBTAIN}. Anchored to the execution time rather - * than the request time, so the cooldown spaces cycle executions. Updated only by on-demand - * re-obtains. + * Relative (monotonic) clock time (millis) at which the last cycle serving pending on-demand + * requests began, or {@link #NO_PREVIOUS_REOBTAIN}. Ordinary periodic renewals without pending + * demand do not move this cooldown anchor. */ @GuardedBy("schedulingLock") private long lastReobtainAtMillis = NO_PREVIOUS_REOBTAIN; @@ -487,6 +486,9 @@ void startTokensUpdate() { + "changed while waiting."); return; } + if (reobtainScheduled) { + lastReobtainAtMillis = clock.relativeTimeMillis(); + } // Keep requests coalesced while waiting for the previous obtain, so they // cannot fill the IO pool with workers blocked on renewalCycleLock. reobtainScheduled = false; @@ -726,9 +728,7 @@ long maybeScheduleRenewal(long delayMs) { Math.max(0L, nextScheduledAtMillis - clock.relativeTimeMillis()); if (delayMs < pendingInMillis) { // Bring the pending on-demand cycle forward. scheduleRenewalLocked() leaves - // reobtainScheduled set, so coalescing still holds. Move the cooldown anchor - // to the time the cycle now actually runs. - lastReobtainAtMillis = clock.relativeTimeMillis() + delayMs; + // reobtainScheduled set, so coalescing still holds until the cycle starts. return scheduleRenewalLocked(delayMs); } LOG.debug( @@ -886,17 +886,14 @@ public void reobtainDelegationTokens() { lastReobtainAtMillis == NO_PREVIOUS_REOBTAIN ? 0L : Math.max(0L, lastReobtainAtMillis + reobtainCooldownMillis - now); - // Only bring the next cycle forward, never push a pending cycle later, or a - // short-lived token could expire before it is renewed. The nextScheduledAtMillis > - // now guard skips an already-fired future, so this never bypasses the cooldown. + // scheduleRenewalLocked() replaces the pending cycle, so never schedule later than it: + // a short-lived token could expire first. The earlier cycle serves this demand too. + // Ignore already-fired futures when comparing delays. if (tokensUpdateFuture != null && nextScheduledAtMillis > now && nextScheduledAtMillis - now < delayMillis) { delayMillis = nextScheduledAtMillis - now; } - // Anchor the cooldown to when the cycle will run, not to this request, so a request - // arriving right after a deferred cycle fired cannot run a second cycle back to back. - lastReobtainAtMillis = now + delayMillis; reobtainScheduled = true; LOG.debug( "Re-obtain of delegation tokens requested, scheduling an obtain cycle in {}", diff --git a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java index c8d67798731f9e..76242e4cd60961 100644 --- a/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java +++ b/flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java @@ -708,8 +708,10 @@ public void reobtainBeforeStartMustNotScheduleObtainCycle() { "A re-obtain before start() must not schedule an obtain cycle"); } - @Test - public void schedulerFailureMustNotWedgeSubsequentReobtains() throws Exception { + @ParameterizedTest + @ValueSource(longs = {0, 60_000}) + public void schedulerFailureMustNotWedgeSubsequentReobtains(long cooldownMillis) + throws Exception { final ManuallyTriggeredScheduledExecutor delegate = new ManuallyTriggeredScheduledExecutor(); final ManuallyTriggeredScheduledExecutorService scheduler = @@ -755,7 +757,11 @@ public void execute(Runnable command) { DefaultDelegationTokenManager delegationTokenManager = new DefaultDelegationTokenManager( - hermeticCooldownConfig(Duration.ZERO), null, throwOnce, scheduler); + hermeticCooldownConfig(Duration.ofMillis(cooldownMillis)), + null, + throwOnce, + scheduler, + new ManualClock()); delegationTokenManager.start(tokens -> {}); // The first re-obtain hits a scheduler that blows up with something other than the @@ -770,6 +776,9 @@ public void execute(Runnable command) { 1, delegate.getActiveScheduledTasks().size(), "A re-obtain after a scheduler failure must schedule a fresh obtain cycle"); + assertThat(onlyScheduledDelayMillis(delegate)) + .as("a failed scheduling attempt must not start the cooldown") + .isZero(); } @ParameterizedTest(name = "configured={0}ms, submission retry={1}ms") @@ -1677,6 +1686,236 @@ public void broughtForwardReobtainMustMoveCooldownAnchor() throws Exception { "The cooldown anchor must follow a brought-forward on-demand cycle"); } + @Test + public void failedBringForwardMustPreservePreviousCooldownAnchor() throws Exception { + final AtomicBoolean throwNext = new AtomicBoolean(); + final ManuallyTriggeredScheduledExecutor scheduledExecutor = + new ManuallyTriggeredScheduledExecutor() { + @Override + public ScheduledFuture schedule( + Runnable command, long delay, TimeUnit unit) { + if (throwNext.compareAndSet(true, false)) { + throw new IllegalStateException("simulated scheduler failure"); + } + return super.schedule(command, delay, unit); + } + }; + final ManuallyTriggeredScheduledExecutorService ioExecutor = + new ManuallyTriggeredScheduledExecutorService(); + final ManualClock clock = new ManualClock(); + final DefaultDelegationTokenManager manager = + new DefaultDelegationTokenManager( + hermeticCooldownConfig(Duration.ofMinutes(1)), + null, + scheduledExecutor, + ioExecutor, + clock); + try { + manager.start(tokens -> {}); + manager.reobtainDelegationTokens(); + scheduledExecutor.triggerScheduledTasks(); + ioExecutor.triggerAll(); + + clock.advanceTime(Duration.ofSeconds(10)); + manager.reobtainDelegationTokens(); + assertThat(onlyScheduledDelayMillis(scheduledExecutor)).isEqualTo(50_000L); + throwNext.set(true); + assertThatThrownBy(() -> manager.maybeScheduleRenewal(5_000L)) + .isInstanceOf(IllegalStateException.class); + + clock.advanceTime(Duration.ofSeconds(1)); + manager.reobtainDelegationTokens(); + assertThat(onlyScheduledDelayMillis(scheduledExecutor)) + .as("failed rescheduling must preserve the last actual cycle start at t=0") + .isEqualTo(49_000L); + } finally { + manager.close(); + } + } + + @ParameterizedTest(name = "previous cycle failed: {0}") + @ValueSource(booleans = {false, true}) + public void queuedRenewalServingDemandMustAnchorCooldownAtCycleStart( + boolean previousCycleFailed) throws Exception { + final ManuallyTriggeredScheduledExecutor scheduledExecutor = + new ManuallyTriggeredScheduledExecutor(); + final ManuallyTriggeredScheduledExecutorService ioExecutor = + new ManuallyTriggeredScheduledExecutorService(); + final ManualClock clock = new ManualClock(); + final AtomicInteger obtains = new AtomicInteger(); + final Configuration configuration = hermeticCooldownConfig(Duration.ofMinutes(1)); + configuration.set(DELEGATION_TOKENS_RENEWAL_TIME_RATIO, 1.0); + final DefaultDelegationTokenManager manager = + new DefaultDelegationTokenManager( + configuration, null, scheduledExecutor, ioExecutor, clock) { + @Override + protected Optional obtainDelegationTokensAndGetNextRenewal( + DelegationTokenContainer container) { + if (obtains.incrementAndGet() == 2) { + if (previousCycleFailed) { + throw new IllegalStateException("simulated obtain failure"); + } + return Optional.of(clock.absoluteTimeMillis() + 10_000L); + } + return Optional.of(clock.absoluteTimeMillis() + 300_000L); + } + + @Override + long calculateRetryDelay(Clock ignored) { + return 10_000L; + } + }; + try { + manager.start(tokens -> {}); + manager.reobtainDelegationTokens(); + scheduledExecutor.triggerScheduledTasks(); + ioExecutor.triggerAll(); + assertThat(obtains).hasValue(2); + assertThat(onlyScheduledDelayMillis(scheduledExecutor)).isEqualTo(10_000L); + + // Queue the periodic/retry worker before demand schedules a cycle for t=60s. + clock.advanceTime(Duration.ofSeconds(10)); + scheduledExecutor.triggerScheduledTasks(); + manager.reobtainDelegationTokens(); + assertThat(onlyScheduledDelayMillis(scheduledExecutor)).isEqualTo(50_000L); + ioExecutor.triggerAll(); + assertThat(obtains).hasValue(3); + assertThat(onlyScheduledDelayMillis(scheduledExecutor)).isEqualTo(300_000L); + + clock.advanceTime(Duration.ofSeconds(1)); + manager.reobtainDelegationTokens(); + assertThat(onlyScheduledDelayMillis(scheduledExecutor)) + .as("the queued worker served demand at t=10s, not the planned t=60s") + .isEqualTo(59_000L); + } finally { + manager.close(); + } + } + + @Test + public void delayedReobtainMustAnchorCooldownAtCycleStart() throws Exception { + final ManuallyTriggeredScheduledExecutor scheduledExecutor = + new ManuallyTriggeredScheduledExecutor(); + final ManuallyTriggeredScheduledExecutorService ioExecutor = + new ManuallyTriggeredScheduledExecutorService(); + final ManualClock clock = new ManualClock(); + final DefaultDelegationTokenManager manager = + new DefaultDelegationTokenManager( + hermeticCooldownConfig(Duration.ofMinutes(1)), + null, + scheduledExecutor, + ioExecutor, + clock); + try { + manager.start(tokens -> {}); + manager.reobtainDelegationTokens(); + assertThat(onlyScheduledDelayMillis(scheduledExecutor)).isZero(); + scheduledExecutor.triggerScheduledTasks(); + + // The timer fired immediately, but the IO worker cannot start for two minutes. + clock.advanceTime(Duration.ofMinutes(2)); + ioExecutor.triggerAll(); + clock.advanceTime(Duration.ofSeconds(1)); + manager.reobtainDelegationTokens(); + assertThat(onlyScheduledDelayMillis(scheduledExecutor)) + .as("the cooldown starts when the IO worker begins the obtain cycle") + .isEqualTo(59_000L); + } finally { + manager.close(); + } + } + + @Test + public void reobtainDuringObtainMustUseActualCycleStart() throws Exception { + final ManuallyTriggeredScheduledExecutor scheduledExecutor = + new ManuallyTriggeredScheduledExecutor(); + final ManuallyTriggeredScheduledExecutorService ioExecutor = + new ManuallyTriggeredScheduledExecutorService(); + final ManualClock clock = new ManualClock(); + final AtomicInteger obtains = new AtomicInteger(); + final Configuration configuration = hermeticCooldownConfig(Duration.ofMinutes(1)); + configuration.set(DELEGATION_TOKENS_RENEWAL_TIME_RATIO, 1.0); + final DefaultDelegationTokenManager manager = + new DefaultDelegationTokenManager( + configuration, null, scheduledExecutor, ioExecutor, clock) { + @Override + protected Optional obtainDelegationTokensAndGetNextRenewal( + DelegationTokenContainer container) { + if (obtains.incrementAndGet() == 2) { + clock.advanceTime(Duration.ofSeconds(10)); + reobtainDelegationTokens(); + assertThat(onlyScheduledDelayMillis(scheduledExecutor)) + .as("demand during obtain uses the cycle start at t=120s") + .isEqualTo(50_000L); + } + return Optional.of(clock.absoluteTimeMillis() + 300_000L); + } + }; + try { + manager.start(tokens -> {}); + manager.reobtainDelegationTokens(); + scheduledExecutor.triggerScheduledTasks(); + + clock.advanceTime(Duration.ofMinutes(2)); + ioExecutor.triggerAll(); + assertThat(obtains).hasValue(2); + assertThat(onlyScheduledDelayMillis(scheduledExecutor)) + .as("successful obtain must preserve the earlier pending demand") + .isEqualTo(50_000L); + + clock.advanceTime(Duration.ofSeconds(50)); + scheduledExecutor.triggerScheduledTasks(); + ioExecutor.triggerAll(); + assertThat(obtains).hasValue(3); + assertThat(onlyScheduledDelayMillis(scheduledExecutor)).isEqualTo(300_000L); + } finally { + manager.close(); + } + } + + @Test + public void periodicRenewalWithoutDemandMustNotMoveCooldownAnchor() throws Exception { + final ManuallyTriggeredScheduledExecutor scheduledExecutor = + new ManuallyTriggeredScheduledExecutor(); + final ManuallyTriggeredScheduledExecutorService ioExecutor = + new ManuallyTriggeredScheduledExecutorService(); + final ManualClock clock = new ManualClock(); + final AtomicInteger obtains = new AtomicInteger(); + final Configuration configuration = hermeticCooldownConfig(Duration.ofMinutes(1)); + configuration.set(DELEGATION_TOKENS_RENEWAL_TIME_RATIO, 1.0); + final DefaultDelegationTokenManager manager = + new DefaultDelegationTokenManager( + configuration, null, scheduledExecutor, ioExecutor, clock) { + @Override + protected Optional obtainDelegationTokensAndGetNextRenewal( + DelegationTokenContainer container) { + final long renewalDelay = + obtains.incrementAndGet() == 2 ? 10_000L : 300_000L; + return Optional.of(clock.absoluteTimeMillis() + renewalDelay); + } + }; + try { + manager.start(tokens -> {}); + manager.reobtainDelegationTokens(); + scheduledExecutor.triggerScheduledTasks(); + ioExecutor.triggerAll(); + assertThat(onlyScheduledDelayMillis(scheduledExecutor)).isEqualTo(10_000L); + + clock.advanceTime(Duration.ofSeconds(10)); + scheduledExecutor.triggerScheduledTasks(); + ioExecutor.triggerAll(); + assertThat(obtains).hasValue(3); + + clock.advanceTime(Duration.ofSeconds(1)); + manager.reobtainDelegationTokens(); + assertThat(onlyScheduledDelayMillis(scheduledExecutor)) + .as("ordinary periodic renewal must preserve the on-demand anchor at t=0") + .isEqualTo(49_000L); + } finally { + manager.close(); + } + } + @Test public void startAfterStopMustResetRetryState() throws Exception { final ManuallyTriggeredScheduledExecutor scheduledExecutor =