You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by GitBox <gi...@apache.org> on 2022/04/25 02:46:09 UTC

[GitHub] [hadoop] zuston commented on a diff in pull request #4198: YARN-11112 Avoid renewing delegation token when app is first submitte…

zuston commented on code in PR #4198:
URL: https://github.com/apache/hadoop/pull/4198#discussion_r857222530


##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/security/DelegationTokenRenewer.java:
##########
@@ -491,12 +492,13 @@ private void handleAppSubmitEvent(AbstractDelegationTokenRenewerAppEvent evt)
     Set<DelegationTokenToRenew> tokenList = new HashSet<DelegationTokenToRenew>();
     boolean hasHdfsToken = false;
     for (Token<?> token : tokens) {
+      AtomicLong tokenExpiredTime = new AtomicLong(now);
       if (token.isManaged()) {
         if (token.getKind().equals(HDFS_DELEGATION_KIND)) {
           LOG.info(applicationId + " found existing hdfs token " + token);
           hasHdfsToken = true;
         }
-        if (skipTokenRenewal(token)) {
+        if (skipTokenRenewal(token, tokenExpiredTime)) {

Review Comment:
   The var of `tokenExpiredTime` will be changed in the method of `skipTokenRenewal`, however we can't found it according to its method name directly. Maybe `skipTokenRenewal` should be renamed to `skipTokenRenewalAndUpdateExpiredTime`.



##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/security/DelegationTokenRenewer.java:
##########
@@ -627,6 +634,7 @@ private boolean skipTokenRenewal(Token<?> token)
     if (identifier == null) {
       return false;
     }
+    expiredTime.set(identifier.getMaxDate());

Review Comment:
   Do we just to reset the expireTime only when the result of `skipTokenRenewal` is false?



##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/security/DelegationTokenRenewer.java:
##########
@@ -535,7 +541,7 @@ private void handleAppSubmitEvent(AbstractDelegationTokenRenewerAppEvent evt)
                   + " on recovery as it expired, requesting new hdfs token for "
                   + applicationId + ", user=" + evt.getUser(), ioe);
               requestNewHdfsDelegationTokenAsProxyUser(
-                  Arrays.asList(applicationId), evt.getUser(),
+                      Collections.singletonList(applicationId), evt.getUser(),

Review Comment:
   Why do this change? Please let me what u think thanks



-- 
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: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org