steveloughran commented on code in PR #4198:
URL: https://github.com/apache/hadoop/pull/4198#discussion_r873912895
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/security/TestDelegationTokenRenewer.java:
##########
@@ -450,8 +456,15 @@ public void testDTRenewal () throws Exception {
token2 = dfs.getDelegationToken("user2");
token3 = dfs.getDelegationToken("user3");
+ // make token3 expire time short.
+ AbstractDelegationTokenIdentifier identifier = token3.decodeIdentifier();
+ identifier.setMaxDate(System.currentTimeMillis());
+ token3.setID(identifier.getBytes());
+
//to cause this one to be set for renew in 2 secs
- Renewer.tokenToRenewIn2Sec = token1;
+ AbstractDelegationTokenIdentifier identifier1 = token1.decodeIdentifier();
+ identifier1.setMaxDate(System.currentTimeMillis() + 2 * 1000);
Review Comment:
nit, use 2_000 for representing 2 seconds
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/security/TestDelegationTokenRenewer.java:
##########
@@ -50,6 +50,7 @@
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
+import
org.apache.hadoop.security.token.delegation.AbstractDelegationTokenIdentifier;
Review Comment:
nit, should go in the right place in the org.apache imports.
##########
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 +540,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(),
+ Arrays.asList(applicationId), evt.getUser(),
Review Comment:
roll this change back just to keep the diff smaller
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/security/DelegationTokenRenewer.java:
##########
@@ -523,9 +524,13 @@ private void
handleAppSubmitEvent(AbstractDelegationTokenRenewerAppEvent evt)
tokenConf = getConfig();
}
dttr = new DelegationTokenToRenew(Arrays.asList(applicationId),
token,
- tokenConf, now, shouldCancelAtEnd, evt.getUser());
+ tokenConf, tokenExpiredTime.get(), shouldCancelAtEnd,
evt.getUser());
+
try {
- renewToken(dttr);
+ // if expire date is not greater than now, renew token.
+ if (tokenExpiredTime.get() <= now) {
Review Comment:
this is too brittle. it should allow for some clock skew. maybe renew if
within 10 minutes of expiry.
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/security/DelegationTokenRenewer.java:
##########
@@ -72,9 +72,9 @@
import org.apache.hadoop.yarn.server.resourcemanager.rmapp.RMAppEvent;
import org.apache.hadoop.yarn.server.resourcemanager.rmapp.RMAppEventType;
import org.apache.hadoop.yarn.server.utils.YarnServerBuilderUtils;
+import org.slf4j.Logger;
Review Comment:
revert moving these...they were in the right place.
imports are where much needless backport/cherrypick issues surface, so
minimising changes here is really important.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
