Details
-
Type:
Bug
-
Status: Resolved
-
Priority:
Major
-
Resolution: Fixed
-
Affects Version/s: None
-
Fix Version/s: 2.8.0, 3.0.0-alpha1
-
Component/s: None
-
Labels:None
-
Hadoop Flags:Reviewed
Description
((Token<AbstractDelegationTokenIdentifier>)token).decodeIdentifier() may throw NPE if RM does not have the corresponding toke kind class.
Activity
- All
- Comments
- Work Log
- History
- Activity
- Transitions
HI Jian He,
Thanks for catching the issue and the patch!
The patch looks good to me. Some questions,
1. what circumstance that RM does not have the token kind class?
2. This method is to check whether we need to skip renewing a token, if RM does not have the corresponding token kind class, does it mean we should not skip renewing thus RM need to renew?
3. If RM need to renew, does it mean RM need to have the token kind class?
Thanks.
Hi Yongjun
what circumstance that RM does not have the token kind class?
e.g. If user pass in an HFTP token, RM may not have the token kind class.
does it mean we should not skip renewing thus RM need to renew; If RM need to renew, does it mean RM need to have the token kind class?
Even without the token kind class, RM may still able to renew because renew is called by Token#renew, and the token kind class is needed for decodeIdentifier method only.
Thanks for the explanation Jian He.
I'm about to +1, but saw some test failures. Would you please check on? thanks.
Those tests are not related. I've run them locally. thanks
Thanks Jian He.
Noticed one little thing, we need to move @SuppressWarnings("unchecked") to right before the code you added, as
@SuppressWarnings("unchecked")
AbstractDelegationTokenIdentifier identifier =
((Token<AbstractDelegationTokenIdentifier>) token).decodeIdentifier();
if (identifier == null) {
return false;
}
+1 after that.
BTW, I wonder why our build system doesn't point that out as a warning.
nice catch ! uploaded a patch to fix this.
 -1 overall |
| Vote | Subsystem | Runtime | Comment |
|---|---|---|---|
| 0 | reexec | 0m 11s | Docker mode activated. |
| +1 | @author | 0m 0s | The patch does not contain any @author tags. |
| -1 | test4tests | 0m 0s | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. |
| +1 | mvninstall | 8m 25s | trunk passed |
| +1 | compile | 0m 38s | trunk passed with JDK v1.8.0_91 |
| +1 | compile | 0m 36s | trunk passed with JDK v1.7.0_95 |
| +1 | checkstyle | 0m 23s | trunk passed |
| +1 | mvnsite | 0m 43s | trunk passed |
| +1 | mvneclipse | 0m 18s | trunk passed |
| +1 | findbugs | 1m 24s | trunk passed |
| +1 | javadoc | 0m 30s | trunk passed with JDK v1.8.0_91 |
| +1 | javadoc | 0m 32s | trunk passed with JDK v1.7.0_95 |
| +1 | mvninstall | 0m 38s | the patch passed |
| +1 | compile | 0m 34s | the patch passed with JDK v1.8.0_91 |
| +1 | javac | 0m 34s | the patch passed |
| +1 | compile | 0m 34s | the patch passed with JDK v1.7.0_95 |
| +1 | javac | 0m 34s | the patch passed |
| +1 | checkstyle | 0m 19s | the patch passed |
| +1 | mvnsite | 0m 40s | the patch passed |
| +1 | mvneclipse | 0m 16s | the patch passed |
| +1 | whitespace | 0m 0s | Patch has no whitespace issues. |
| +1 | findbugs | 1m 35s | the patch passed |
| +1 | javadoc | 0m 27s | the patch passed with JDK v1.8.0_91 |
| +1 | javadoc | 0m 31s | the patch passed with JDK v1.7.0_95 |
| -1 | unit | 36m 8s | hadoop-yarn-server-resourcemanager in the patch failed with JDK v1.8.0_91. |
| -1 | unit | 36m 17s | hadoop-yarn-server-resourcemanager in the patch failed with JDK v1.7.0_95. |
| +1 | asflicense | 0m 23s | Patch does not generate ASF License warnings. |
| 93m 17s |
| Reason | Tests |
|---|---|
| JDK v1.8.0_91 Failed junit tests | hadoop.yarn.server.resourcemanager.TestClientRMTokens |
| hadoop.yarn.server.resourcemanager.TestAMAuthorization | |
| hadoop.yarn.server.resourcemanager.TestRMRestart | |
| hadoop.yarn.server.resourcemanager.security.TestDelegationTokenRenewer | |
| JDK v1.7.0_95 Failed junit tests | hadoop.yarn.server.resourcemanager.TestClientRMTokens |
| hadoop.yarn.server.resourcemanager.TestAMAuthorization | |
| hadoop.yarn.server.resourcemanager.TestRMRestart | |
| hadoop.yarn.server.resourcemanager.security.TestDelegationTokenRenewer | |
| hadoop.yarn.server.resourcemanager.TestContainerResourceUsage |
This message was automatically generated.
Thanks Jian He for the rev. +1.
Thanks Yongjun Zhang, could you help committing this in ?
Sure Jian, will soon.
FAILURE: Integrated in Hadoop-trunk-Commit #9734 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9734/)
YARN-5048. DelegationTokenRenewer#skipTokenRenewal may throw NPE (Jian (yzhang: rev 47c41e7ac7e6b905a58550f8899f629c1cf8b138)
- hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/security/DelegationTokenRenewer.java
YARN-5048This message was automatically generated.