Savonitar commented on code in PR #28639:
URL: https://github.com/apache/flink/pull/28639#discussion_r3895718494


##########
flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java:
##########
@@ -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()) {

Review Comment:
   1. Renamed stop() to close() in [[FLINK-40019][core][runtime] Rename 
DelegationTokenProvider.stop() to 
close()](https://github.com/apache/flink/pull/28639/changes/5056c32432662a83a9d8a92b85e4e4eb3292e421)
 . Agree it meets naming API better.
   2.
   > correct me if I'm wrong
   > stop (together with start) can happen when HA kicks in during failover and 
it makes sure that new leader re-obtained tokens.
   > stop has nothing to do with freeing resources allocated by the provider 
and this function called regularly.
   
   yes, your understanding is correct 👍 
   
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to