Details
-
Bug
-
Status: Resolved
-
Critical
-
Resolution: Fixed
-
None
-
None
-
None
-
Reviewed
Description
We saw this exception several times when we tried to getDelegationToken from ATS.
java.io.IOException: org.apache.hadoop.security.authentication.client.AuthenticationException: java.net.SocketTimeoutException: Read timed out
at org.apache.hadoop.yarn.client.api.impl.TimelineClientImpl$5.run(TimelineClientImpl.java:569)
at org.apache.hadoop.yarn.client.api.impl.TimelineClientImpl$TimelineClientConnectionRetry.retryOn(TimelineClientImpl.java:234)
at org.apache.hadoop.yarn.client.api.impl.TimelineClientImpl.operateDelegationToken(TimelineClientImpl.java:582)
at org.apache.hadoop.yarn.client.api.impl.TimelineClientImpl.getDelegationToken(TimelineClientImpl.java:479)
at org.apache.hadoop.yarn.client.api.impl.YarnClientImpl.getTimelineDelegationToken(YarnClientImpl.java:349)
at org.apache.hadoop.yarn.client.api.impl.YarnClientImpl.addTimelineDelegationToken(YarnClientImpl.java:330)
at org.apache.hadoop.yarn.client.api.impl.YarnClientImpl.submitApplication(YarnClientImpl.java:250)
at org.apache.hadoop.mapred.ResourceMgrDelegate.submitApplication(ResourceMgrDelegate.java:291)
at org.apache.hadoop.mapred.YARNRunner.submitJob(YARNRunner.java:290)
at org.apache.hadoop.mapreduce.JobSubmitter.submitJobInternal(JobSubmitter.java:240)
at org.apache.hadoop.mapreduce.Job$10.run(Job.java:1290)
at org.apache.hadoop.mapreduce.Job$10.run(Job.java:1287)
at java.security.AccessController.doPrivileged(Native Method)
at javax.security.auth.Subject.doAs(Subject.java:415)
at org.apache.hadoop.security.UserGroupInformation.doAs(UserGroupInformation.java:1657)
at org.apache.hadoop.mapreduce.Job.submit(Job.java:1287)
at org.apache.hadoop.mapreduce.lib.jobcontrol.ControlledJob.submit(ControlledJob.java:335)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:606)
at org.apache.pig.backend.hadoop23.PigJobControl.submit(PigJobControl.java:128)
at org.apache.pig.backend.hadoop23.PigJobControl.run(PigJobControl.java:194)
at java.lang.Thread.run(Thread.java:745)
at org.apache.pig.backend.hadoop.executionengine.mapReduceLayer.MapReduceLauncher$1.run(MapReduceLauncher.java:276)
Caused by: org.apache.hadoop.security.authentication.client.AuthenticationException: java.net.SocketTimeoutException: Read timed out
at org.apache.hadoop.security.authentication.client.KerberosAuthenticator.doSpnegoSequence(KerberosAuthenticator.java:332)
at org.apache.hadoop.security.authentication.client.KerberosAuthenticator.authenticate(KerberosAuthenticator.java:205)
at org.apache.hadoop.security.token.delegation.web.DelegationTokenAuthenticator.authenticate(DelegationTokenAuthenticator.java:128)
at org.apache.hadoop.security.authentication.client.AuthenticatedURL.openConnection(AuthenticatedURL.java:215)
at org.apache.hadoop.security.token.delegation.web.DelegationTokenAuthenticator.doDelegationTokenOperation(DelegationTokenAuthenticator.java:285)
at org.apache.hadoop.security.token.delegation.web.DelegationTokenAuthenticator.getDelegationToken(DelegationTokenAuthenticator.java:166)
at org.apache.hadoop.security.token.delegation.web.DelegationTokenAuthenticatedURL.getDelegationToken(DelegationTokenAuthenticatedURL.java:371)
at org.apache.hadoop.yarn.client.api.impl.TimelineClientImpl$2.run(TimelineClientImpl.java:475)
at org.apache.hadoop.yarn.client.api.impl.TimelineClientImpl$2.run(TimelineClientImpl.java:467)
at java.security.AccessController.doPrivileged(Native Method)
at javax.security.auth.Subject.doAs(Subject.java:415)
at org.apache.hadoop.security.UserGroupInformation.doAs(UserGroupInformation.java:1657)
at org.apache.hadoop.yarn.client.api.impl.TimelineClientImpl$5.run(TimelineClientImpl.java:567)
... 24 more
Caused by: java.net.SocketTimeoutException: Read timed out
at java.net.SocketInputStream.socketRead0(Native Method)
at java.net.SocketInputStream.read(SocketInputStream.java:152)
at java.net.SocketInputStream.read(SocketInputStream.java:122)
at java.io.BufferedInputStream.fill(BufferedInputStream.java:235)
at java.io.BufferedInputStream.read1(BufferedInputStream.java:275)
at java.io.BufferedInputStream.read(BufferedInputStream.java:334)
at sun.net.www.http.HttpClient.parseHTTPHeader(HttpClient.java:690)
at sun.net.www.http.HttpClient.parseHTTP(HttpClient.java:633)
at sun.net.www.protocol.http.HttpURLConnection.getInputStream(HttpURLConnection.java:1325)
at java.net.HttpURLConnection.getResponseCode(HttpURLConnection.java:468)
at org.apache.hadoop.security.authentication.client.KerberosAuthenticator.readToken(KerberosAuthenticator.java:357)
at org.apache.hadoop.security.authentication.client.KerberosAuthenticator.access$300(KerberosAuthenticator.java:54)
at org.apache.hadoop.security.authentication.client.KerberosAuthenticator$1.run(KerberosAuthenticator.java:317)
at org.apache.hadoop.security.authentication.client.KerberosAuthenticator$1.run(KerberosAuthenticator.java:287)
at java.security.AccessController.doPrivileged(Native Method)
at javax.security.auth.Subject.doAs(Subject.java:415)
at org.apache.hadoop.security.authentication.client.KerberosAuthenticator.doSpnegoSequence(KerberosAuthenticator.java:287)
... 36 more
Attachments
Attachments
- YARN-4955.1.patch
- 1 kB
- Xuan Gong
- YARN-4955.2.patch
- 1 kB
- Xuan Gong
- YARN-4955.3.patch
- 1 kB
- Xuan Gong
- YARN-4955.4.patch
- 7 kB
- Xuan Gong
- YARN-4955.4-1.patch
- 7 kB
- Xuan Gong
- YARN-4955.5.patch
- 7 kB
- Xuan Gong
- YARN-4955.6.patch
- 7 kB
- Xuan Gong
Issue Links
- depends upon
-
HADOOP-13026 Should not wrap IOExceptions into a AuthenticationException in KerberosAuthenticator
- Resolved
Activity
-1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 17s | 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 | 6m 44s | trunk passed |
+1 | compile | 0m 22s | trunk passed with JDK v1.8.0_77 |
+1 | compile | 0m 26s | trunk passed with JDK v1.7.0_95 |
+1 | checkstyle | 0m 21s | trunk passed |
+1 | mvnsite | 0m 30s | trunk passed |
+1 | mvneclipse | 0m 13s | trunk passed |
+1 | findbugs | 1m 7s | trunk passed |
+1 | javadoc | 0m 27s | trunk passed with JDK v1.8.0_77 |
+1 | javadoc | 0m 33s | trunk passed with JDK v1.7.0_95 |
+1 | mvninstall | 0m 26s | the patch passed |
+1 | compile | 0m 20s | the patch passed with JDK v1.8.0_77 |
+1 | javac | 0m 20s | the patch passed |
+1 | compile | 0m 24s | the patch passed with JDK v1.7.0_95 |
+1 | javac | 0m 24s | the patch passed |
+1 | checkstyle | 0m 17s | the patch passed |
+1 | mvnsite | 0m 28s | the patch passed |
+1 | mvneclipse | 0m 11s | the patch passed |
+1 | whitespace | 0m 0s | Patch has no whitespace issues. |
+1 | findbugs | 1m 16s | the patch passed |
+1 | javadoc | 0m 24s | the patch passed with JDK v1.8.0_77 |
+1 | javadoc | 0m 31s | the patch passed with JDK v1.7.0_95 |
+1 | unit | 1m 54s | hadoop-yarn-common in the patch passed with JDK v1.8.0_77. |
+1 | unit | 2m 12s | hadoop-yarn-common in the patch passed with JDK v1.7.0_95. |
+1 | asflicense | 0m 18s | Patch does not generate ASF License warnings. |
20m 42s |
Subsystem | Report/Notes |
---|---|
Docker | Image:yetus/hadoop:fbe3e86 |
JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12798534/YARN-4955.1.patch |
JIRA Issue | |
Optional Tests | asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle |
uname | Linux 23ddfb1806d8 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /testptch/hadoop/patchprocess/precommit/personality/provided.sh |
git revision | trunk / 903428b |
Default Java | 1.7.0_95 |
Multi-JDK versions | /usr/lib/jvm/java-8-oracle:1.8.0_77 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 |
findbugs | v3.0.0 |
JDK v1.7.0_95 Test Results | https://builds.apache.org/job/PreCommit-YARN-Build/11062/testReport/ |
modules | C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common |
Console output | https://builds.apache.org/job/PreCommit-YARN-Build/11062/console |
Powered by | Apache Yetus 0.2.0 http://yetus.apache.org |
This message was automatically generated.
Thanks for the review. Looks like in
public Object run() throws IOException { // Try pass the request, if fail, keep retrying authUgi.checkTGTAndReloginFromKeytab(); try { return authUgi.doAs(action); } catch (UndeclaredThrowableException e) { throw new IOException(e.getCause()); } catch (InterruptedException e) { throw new IOException(e); } }
All the exceptions would be wrapped as IOException. So, currently catch IOException should be enough.
Have modify the patch to get the cause from IOException to check whether it is AuthenticationException. and then further check the cause for this AuthenticationException.
Thanks for the work xgong! Yes from the exception stack the AuthenticationException was wrapped into an IOException, so fixing the shouldRetryOn() method of the delegation token retry should be fine. However, I do share the same concern as junping_du that we may want to build an UT to mock this case in TestTimelineClient#testDelegationTokenOperationsRetry.
I think retrying AuthenticationException (because of a boxed exception) is not correct.
The real problem is that SocketTimeoutException is getting wrapped into a AuthenticationException - kind of like the server is down, but we are informing the user that his/her user-name / password combination is wrong.
We should fix the lower layers to not wrap SocketTimeoutException into a AuthenticationException.
-1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 15s | 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 50s | trunk passed |
+1 | compile | 0m 41s | trunk passed with JDK v1.8.0_77 |
+1 | compile | 0m 33s | trunk passed with JDK v1.7.0_95 |
+1 | checkstyle | 0m 26s | trunk passed |
+1 | mvnsite | 0m 42s | trunk passed |
+1 | mvneclipse | 0m 16s | trunk passed |
+1 | findbugs | 1m 38s | trunk passed |
+1 | javadoc | 0m 50s | trunk passed with JDK v1.8.0_77 |
+1 | javadoc | 0m 50s | trunk passed with JDK v1.7.0_95 |
+1 | mvninstall | 0m 38s | the patch passed |
+1 | compile | 0m 44s | the patch passed with JDK v1.8.0_77 |
+1 | javac | 0m 44s | the patch passed |
+1 | compile | 0m 33s | the patch passed with JDK v1.7.0_95 |
+1 | javac | 0m 33s | the patch passed |
+1 | checkstyle | 0m 24s | the patch passed |
+1 | mvnsite | 0m 41s | the patch passed |
+1 | mvneclipse | 0m 14s | the patch passed |
+1 | whitespace | 0m 0s | Patch has no whitespace issues. |
+1 | findbugs | 1m 48s | the patch passed |
+1 | javadoc | 0m 45s | the patch passed with JDK v1.8.0_77 |
+1 | javadoc | 0m 39s | the patch passed with JDK v1.7.0_95 |
+1 | unit | 2m 47s | hadoop-yarn-common in the patch passed with JDK v1.8.0_77. |
+1 | unit | 2m 31s | hadoop-yarn-common in the patch passed with JDK v1.7.0_95. |
+1 | asflicense | 0m 21s | Patch does not generate ASF License warnings. |
28m 22s |
Subsystem | Report/Notes |
---|---|
Docker | Image:yetus/hadoop:fbe3e86 |
JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12798540/YARN-4955.2.patch |
JIRA Issue | |
Optional Tests | asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle |
uname | Linux e09a521fb3fe 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /testptch/hadoop/patchprocess/precommit/personality/provided.sh |
git revision | trunk / c970f1d |
Default Java | 1.7.0_95 |
Multi-JDK versions | /usr/lib/jvm/java-8-oracle:1.8.0_77 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 |
findbugs | v3.0.0 |
JDK v1.7.0_95 Test Results | https://builds.apache.org/job/PreCommit-YARN-Build/11080/testReport/ |
modules | C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common |
Console output | https://builds.apache.org/job/PreCommit-YARN-Build/11080/console |
Powered by | Apache Yetus 0.2.0 http://yetus.apache.org |
This message was automatically generated.
Thanks for the suggestion, vinod.
Create a https://issues.apache.org/jira/browse/HADOOP-13026 to not wrap SocketTimeoutException into a AuthenticationException.
Have a new patch to check the socketTimeoutException.
-1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 16s | 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 56s | trunk passed |
+1 | compile | 0m 48s | trunk passed with JDK v1.8.0_77 |
+1 | compile | 0m 39s | trunk passed with JDK v1.7.0_95 |
+1 | checkstyle | 0m 28s | trunk passed |
+1 | mvnsite | 0m 43s | trunk passed |
+1 | mvneclipse | 0m 15s | trunk passed |
+1 | findbugs | 1m 31s | trunk passed |
+1 | javadoc | 0m 56s | trunk passed with JDK v1.8.0_77 |
+1 | javadoc | 0m 46s | trunk passed with JDK v1.7.0_95 |
+1 | mvninstall | 0m 34s | the patch passed |
+1 | compile | 0m 49s | the patch passed with JDK v1.8.0_77 |
+1 | javac | 0m 49s | the patch passed |
+1 | compile | 0m 36s | the patch passed with JDK v1.7.0_95 |
+1 | javac | 0m 36s | the patch passed |
+1 | checkstyle | 0m 24s | the patch passed |
+1 | mvnsite | 0m 41s | the patch passed |
+1 | mvneclipse | 0m 14s | the patch passed |
+1 | whitespace | 0m 0s | Patch has no whitespace issues. |
+1 | findbugs | 1m 40s | the patch passed |
+1 | javadoc | 0m 32s | the patch passed with JDK v1.8.0_77 |
+1 | javadoc | 0m 40s | the patch passed with JDK v1.7.0_95 |
+1 | unit | 2m 56s | hadoop-yarn-common in the patch passed with JDK v1.8.0_77. |
+1 | unit | 2m 44s | hadoop-yarn-common in the patch passed with JDK v1.7.0_95. |
+1 | asflicense | 0m 23s | Patch does not generate ASF License warnings. |
28m 44s |
Subsystem | Report/Notes |
---|---|
Docker | Image:yetus/hadoop:fbe3e86 |
JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12798785/YARN-4955.3.patch |
JIRA Issue | |
Optional Tests | asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle |
uname | Linux 69fe3ebf5710 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /testptch/hadoop/patchprocess/precommit/personality/provided.sh |
git revision | trunk / a74580a |
Default Java | 1.7.0_95 |
Multi-JDK versions | /usr/lib/jvm/java-8-oracle:1.8.0_77 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 |
findbugs | v3.0.0 |
JDK v1.7.0_95 Test Results | https://builds.apache.org/job/PreCommit-YARN-Build/11082/testReport/ |
modules | C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common |
Console output | https://builds.apache.org/job/PreCommit-YARN-Build/11082/console |
Powered by | Apache Yetus 0.2.0 http://yetus.apache.org |
This message was automatically generated.
03 patch LGTM.
If no further comments from others, I will commit it shortly after HADOOP-13026 get commit.
This looks good to me too. xgong, can you please add a simple test which validates that the client retries on socket-timeout now?
-1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 15s | Docker mode activated. |
+1 | @author | 0m 0s | The patch does not contain any @author tags. |
+1 | test4tests | 0m 0s | The patch appears to include 1 new or modified test files. |
+1 | mvninstall | 6m 50s | trunk passed |
+1 | compile | 0m 24s | trunk passed with JDK v1.8.0_77 |
+1 | compile | 0m 26s | trunk passed with JDK v1.7.0_95 |
+1 | checkstyle | 0m 20s | trunk passed |
+1 | mvnsite | 0m 30s | trunk passed |
+1 | mvneclipse | 0m 13s | trunk passed |
+1 | findbugs | 1m 6s | trunk passed |
+1 | javadoc | 0m 27s | trunk passed with JDK v1.8.0_77 |
+1 | javadoc | 0m 33s | trunk passed with JDK v1.7.0_95 |
+1 | mvninstall | 0m 27s | the patch passed |
+1 | compile | 0m 21s | the patch passed with JDK v1.8.0_77 |
+1 | javac | 0m 21s | the patch passed |
+1 | compile | 0m 25s | the patch passed with JDK v1.7.0_95 |
+1 | javac | 0m 25s | the patch passed |
-1 | checkstyle | 0m 18s | hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common: patch generated 2 new + 19 unchanged - 0 fixed = 21 total (was 19) |
+1 | mvnsite | 0m 28s | the patch passed |
+1 | mvneclipse | 0m 11s | the patch passed |
-1 | whitespace | 0m 0s | The patch has 2 line(s) with tabs. |
+1 | findbugs | 1m 19s | the patch passed |
+1 | javadoc | 0m 25s | the patch passed with JDK v1.8.0_77 |
+1 | javadoc | 0m 30s | the patch passed with JDK v1.7.0_95 |
+1 | unit | 1m 58s | hadoop-yarn-common in the patch passed with JDK v1.8.0_77. |
+1 | unit | 2m 13s | hadoop-yarn-common in the patch passed with JDK v1.7.0_95. |
+1 | asflicense | 0m 18s | Patch does not generate ASF License warnings. |
20m 57s |
Subsystem | Report/Notes |
---|---|
Docker | Image:yetus/hadoop:fbe3e86 |
JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12799005/YARN-4955.4-1.patch |
JIRA Issue | |
Optional Tests | asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle |
uname | Linux fe2eac08a01c 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /testptch/hadoop/patchprocess/precommit/personality/provided.sh |
git revision | trunk / fdbafbc |
Default Java | 1.7.0_95 |
Multi-JDK versions | /usr/lib/jvm/java-8-oracle:1.8.0_77 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 |
findbugs | v3.0.0 |
checkstyle | https://builds.apache.org/job/PreCommit-YARN-Build/11102/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common.txt |
whitespace | https://builds.apache.org/job/PreCommit-YARN-Build/11102/artifact/patchprocess/whitespace-tabs.txt |
JDK v1.7.0_95 Test Results | https://builds.apache.org/job/PreCommit-YARN-Build/11102/testReport/ |
modules | C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common |
Console output | https://builds.apache.org/job/PreCommit-YARN-Build/11102/console |
Powered by | Apache Yetus 0.2.0 http://yetus.apache.org |
This message was automatically generated.
-1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 13s | Docker mode activated. |
+1 | @author | 0m 0s | The patch does not contain any @author tags. |
+1 | test4tests | 0m 0s | The patch appears to include 1 new or modified test files. |
+1 | mvninstall | 6m 32s | trunk passed |
+1 | compile | 0m 24s | trunk passed with JDK v1.8.0_77 |
+1 | compile | 0m 27s | trunk passed with JDK v1.7.0_95 |
+1 | checkstyle | 0m 19s | trunk passed |
+1 | mvnsite | 0m 31s | trunk passed |
+1 | mvneclipse | 0m 13s | trunk passed |
+1 | findbugs | 1m 9s | trunk passed |
+1 | javadoc | 0m 26s | trunk passed with JDK v1.8.0_77 |
+1 | javadoc | 0m 32s | trunk passed with JDK v1.7.0_95 |
+1 | mvninstall | 0m 26s | the patch passed |
+1 | compile | 0m 19s | the patch passed with JDK v1.8.0_77 |
+1 | javac | 0m 19s | the patch passed |
+1 | compile | 0m 25s | the patch passed with JDK v1.7.0_95 |
+1 | javac | 0m 25s | the patch passed |
-1 | checkstyle | 0m 18s | hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common: patch generated 1 new + 19 unchanged - 0 fixed = 20 total (was 19) |
+1 | mvnsite | 0m 28s | the patch passed |
+1 | mvneclipse | 0m 10s | the patch passed |
+1 | whitespace | 0m 0s | Patch has no whitespace issues. |
+1 | findbugs | 1m 18s | the patch passed |
+1 | javadoc | 0m 24s | the patch passed with JDK v1.8.0_77 |
+1 | javadoc | 0m 30s | the patch passed with JDK v1.7.0_95 |
+1 | unit | 1m 58s | hadoop-yarn-common in the patch passed with JDK v1.8.0_77. |
+1 | unit | 2m 12s | hadoop-yarn-common in the patch passed with JDK v1.7.0_95. |
+1 | asflicense | 0m 19s | Patch does not generate ASF License warnings. |
20m 34s |
Subsystem | Report/Notes |
---|---|
Docker | Image:yetus/hadoop:fbe3e86 |
JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12799008/YARN-4955.5.patch |
JIRA Issue | |
Optional Tests | asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle |
uname | Linux 0553c18b369c 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /testptch/hadoop/patchprocess/precommit/personality/provided.sh |
git revision | trunk / 69f3d42 |
Default Java | 1.7.0_95 |
Multi-JDK versions | /usr/lib/jvm/java-8-oracle:1.8.0_77 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 |
findbugs | v3.0.0 |
checkstyle | https://builds.apache.org/job/PreCommit-YARN-Build/11103/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common.txt |
JDK v1.7.0_95 Test Results | https://builds.apache.org/job/PreCommit-YARN-Build/11103/testReport/ |
modules | C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common |
Console output | https://builds.apache.org/job/PreCommit-YARN-Build/11103/console |
Powered by | Apache Yetus 0.2.0 http://yetus.apache.org |
This message was automatically generated.
The checkstyle issue seems to be noisy only. whitespace issue can be fixed in commit. Patch LGTM.
vinodkv and gtCarrera9, do you have more comments? If not, I will go ahead to commit this...
junping_du, I was reviewing the previous patch and gave some review comments, so I'll review this update and commit it if they are addressed, no need to rush.
Sorry I didn't get back earlier. One confusion, are we testing on the behavior of clientFake's retry instead of client's retry here?
in order to test retry on SocketTimeoutException, I have to override TimelineClientRetryOp#run() to throw out SocketTimeoutException.
I think I understand what you guys are saying.
xgong, you can do this by creating an inner class inside TimelineClientRetryOp say TimelineClientRetryOpForOperateDelegationToken (instead of createTimelineClientRetryOpForOperateDelegationToken()) and then override only the run() method of this class inside the test-case to throw a SocketTimeoutException.
I don't think it is a good idea to add inner/nested class just for unit test purpose - which make code less readable which is worse than no UT for a simple exception catch. If we must have unit test, I would suggest to go with v3 patch + mock exception - that make our main code cleaner.
junping_du, whatever works - nested class or mocking. xgong, let's fix the test so that we can get it in today, tx.
-1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 9s | Docker mode activated. |
+1 | @author | 0m 0s | The patch does not contain any @author tags. |
+1 | test4tests | 0m 0s | The patch appears to include 1 new or modified test files. |
+1 | mvninstall | 6m 52s | trunk passed |
+1 | compile | 0m 24s | trunk passed with JDK v1.8.0_77 |
+1 | compile | 0m 26s | trunk passed with JDK v1.7.0_95 |
+1 | checkstyle | 0m 20s | trunk passed |
+1 | mvnsite | 0m 30s | trunk passed |
+1 | mvneclipse | 0m 12s | trunk passed |
+1 | findbugs | 1m 6s | trunk passed |
+1 | javadoc | 0m 26s | trunk passed with JDK v1.8.0_77 |
+1 | javadoc | 0m 31s | trunk passed with JDK v1.7.0_95 |
+1 | mvninstall | 0m 26s | the patch passed |
+1 | compile | 0m 19s | the patch passed with JDK v1.8.0_77 |
+1 | javac | 0m 19s | the patch passed |
+1 | compile | 0m 23s | the patch passed with JDK v1.7.0_95 |
+1 | javac | 0m 23s | the patch passed |
-1 | checkstyle | 0m 17s | hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common: patch generated 2 new + 19 unchanged - 0 fixed = 21 total (was 19) |
+1 | mvnsite | 0m 27s | the patch passed |
+1 | mvneclipse | 0m 11s | the patch passed |
+1 | whitespace | 0m 0s | Patch has no whitespace issues. |
+1 | findbugs | 1m 15s | the patch passed |
+1 | javadoc | 0m 24s | the patch passed with JDK v1.8.0_77 |
+1 | javadoc | 0m 29s | the patch passed with JDK v1.7.0_95 |
+1 | unit | 1m 53s | hadoop-yarn-common in the patch passed with JDK v1.8.0_77. |
+1 | unit | 2m 10s | hadoop-yarn-common in the patch passed with JDK v1.7.0_95. |
+1 | asflicense | 0m 20s | Patch does not generate ASF License warnings. |
20m 28s |
Subsystem | Report/Notes |
---|---|
Docker | Image:yetus/hadoop:fbe3e86 |
JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12799298/YARN-4955.6.patch |
JIRA Issue | |
Optional Tests | asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle |
uname | Linux 26f6d006b6a3 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /testptch/hadoop/patchprocess/precommit/personality/provided.sh |
git revision | trunk / d8b729e |
Default Java | 1.7.0_95 |
Multi-JDK versions | /usr/lib/jvm/java-8-oracle:1.8.0_77 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 |
findbugs | v3.0.0 |
checkstyle | https://builds.apache.org/job/PreCommit-YARN-Build/11115/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common.txt |
JDK v1.7.0_95 Test Results | https://builds.apache.org/job/PreCommit-YARN-Build/11115/testReport/ |
modules | C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common |
Console output | https://builds.apache.org/job/PreCommit-YARN-Build/11115/console |
Powered by | Apache Yetus 0.2.0 http://yetus.apache.org |
This message was automatically generated.
The checkstyle warnings are spurious - they are private classes and so they (or the Private annotation) don't need javadoc.
The latest patch looks good to me. +1. Checking this in.
Committed this to trunk, branch-2 and branch-2.8. Thanks xgong!
Tx for the review help gtCarrera9 / junping_du.
vinodkv, can you follow the general practice to wait for all active reviewer give +1 or keep silent after going through their working hours?
I haven't agree to your proposal above as I mentioned that it make code hard to read which doesn't sound like good compromise for a simple exception catch UT.
Even I agree on this, the checkstyle issue is still worth discussable, as we are adding two public classes (even mark with private annotation ) which is visible within hadoop projects - that means it is better to put some java doc description which is most practice of our existing code.
+ @Private + @VisibleForTesting + public class TimelineClientRetryOpForOperateDelegationToken + extends TimelineClientRetryOp { +
I reopen this for addressing my comments - either reverting current patch or adding an addendum one.
junping_du, about the time for review, fair enough..
I haven't agree to your proposal above as I mentioned that it make code hard to read which doesn't sound like good compromise for a simple exception catch UT.
The rest of us clearly agree to what Xuan finally did. If you have an alternative proposal, please propose it and drive it. We are here primarily because the original code lacked unit tests - whatever adds more coverage is fine by me.
Even I agree on this, the checkstyle issue is still worth discussable, as we are adding two public classes (even mark with private annotation ) which is visible within hadoop projects - that means it is better to put some java doc description which is most practice of our existing code.
Completely off on this one. These are marked @Private - the convention is that they are really private. In fact, our javadoc tooling ignores them during generation of documentation. That said, if you want the javadoc be added, its meaningless but sure.
These are marked @Private - the convention is that they are really private. In fact, our javadoc tooling ignores them during generation of documentation.
If so, I think we should notice Yetus team to get rid of javadoc warning in case API marked with Private.
FYI for git log greppers that this one is missing commit message, here's the commit (msg a bit different from JIRA summary):
commit 477003730e6a7c7eff11892f5cedf74073ca867b
Author: Vinod Kumar Vavilapalli <vinodkv@apache.org>
Date: Mon Apr 18 11:47:06 2016 -0700Fixed TimelineClient to retry SocketTimeoutException too. Contributed by Xuan Gong.
Thanks xgong for reporting the issue and delivering the patch. The issue reported here is valid and I also see the same problem in some logs. For fix, I think we are missing the catch part:
As AuthenticationException is neither a IOException or RuntimeException, we will miss to catch here.
Can you add AuthenticationException in catch clause? Also, it would be nice if we can have a unit test to verify it works.