Uploaded image for project: 'Hadoop YARN'
  1. Hadoop YARN
  2. YARN-4955

Add retry for SocketTimeoutException in TimelineClient

Details

    • Bug
    • Status: Resolved
    • Critical
    • Resolution: Fixed
    • None
    • 2.8.0, 3.0.0-alpha1
    • 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

        1. YARN-4955.1.patch
          1 kB
          Xuan Gong
        2. YARN-4955.2.patch
          1 kB
          Xuan Gong
        3. YARN-4955.3.patch
          1 kB
          Xuan Gong
        4. YARN-4955.4.patch
          7 kB
          Xuan Gong
        5. YARN-4955.4-1.patch
          7 kB
          Xuan Gong
        6. YARN-4955.5.patch
          7 kB
          Xuan Gong
        7. YARN-4955.6.patch
          7 kB
          Xuan Gong

        Issue Links

          Activity

            junping_du Junping Du added a comment -

            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:

                public Object retryOn(TimelineClientRetryOp op)
                    throws RuntimeException, IOException {
                  int leftRetries = maxRetries;
                  retried = false;
            
                  // keep trying
                  while (true) {
                    try {
                      // try perform the op, if fail, keep retrying
                      return op.run();
                    } catch (IOException | RuntimeException e) {
                      // break if there's no retries left
            

            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.

            junping_du Junping Du added a comment - 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: public Object retryOn(TimelineClientRetryOp op) throws RuntimeException, IOException { int leftRetries = maxRetries; retried = false ; // keep trying while ( true ) { try { // try perform the op, if fail, keep retrying return op.run(); } catch (IOException | RuntimeException e) { // break if there's no retries left 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.
            hadoopqa Hadoop QA added a comment -
            -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 YARN-4955
            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.

            hadoopqa Hadoop QA added a comment - -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 YARN-4955 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.
            xgong Xuan Gong added a comment -

            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.

            xgong Xuan Gong added a comment - 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.
            gtcarrera9 Li Lu added a comment - - edited

            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.

            gtcarrera9 Li Lu added a comment - - edited 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.

            vinodkv Vinod Kumar Vavilapalli added a comment - 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.
            hadoopqa Hadoop QA added a comment -
            -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 YARN-4955
            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.

            hadoopqa Hadoop QA added a comment - -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 YARN-4955 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.
            xgong Xuan Gong added a comment -

            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.

            xgong Xuan Gong added a comment - 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.
            hadoopqa Hadoop QA added a comment -
            -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 YARN-4955
            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.

            hadoopqa Hadoop QA added a comment - -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 YARN-4955 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.
            junping_du Junping Du added a comment -

            03 patch LGTM.
            If no further comments from others, I will commit it shortly after HADOOP-13026 get commit.

            junping_du Junping Du added a comment - 03 patch LGTM. If no further comments from others, I will commit it shortly after HADOOP-13026 get commit.
            gtcarrera9 Li Lu added a comment -

            New fix looks fine. Thanks!

            gtcarrera9 Li Lu added a comment - New fix looks fine. Thanks!

            This looks good to me too. xgong, can you please add a simple test which validates that the client retries on socket-timeout now?

            vinodkv Vinod Kumar Vavilapalli added a comment - This looks good to me too. xgong , can you please add a simple test which validates that the client retries on socket-timeout now?
            xgong Xuan Gong added a comment -

            Thanks for the review.

            New patch added a testcase for this.

            xgong Xuan Gong added a comment - Thanks for the review. New patch added a testcase for this.
            hadoopqa Hadoop QA added a comment -
            -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 YARN-4955
            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.

            hadoopqa Hadoop QA added a comment - -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 YARN-4955 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.
            xgong Xuan Gong added a comment -

            Fixed the checkstyle and whitespace warning.

            xgong Xuan Gong added a comment - Fixed the checkstyle and whitespace warning.
            hadoopqa Hadoop QA added a comment -
            -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 YARN-4955
            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.

            hadoopqa Hadoop QA added a comment - -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 YARN-4955 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.
            junping_du Junping Du added a comment -

            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 Junping Du added a comment - 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...
            vinodkv Vinod Kumar Vavilapalli added a comment - - edited

            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.

            vinodkv Vinod Kumar Vavilapalli added a comment - - edited 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.
            gtcarrera9 Li Lu added a comment -

            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?

            gtcarrera9 Li Lu added a comment - 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?
            xgong Xuan Gong added a comment -

            gtCarrera9

            in order to test retry on SocketTimeoutException, I have to override TimelineClientRetryOp#run() to throw out SocketTimeoutException.

            xgong Xuan Gong added a comment - gtCarrera9 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.

            vinodkv Vinod Kumar Vavilapalli added a comment - 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.
            junping_du Junping Du added a comment -

            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 Junping Du added a comment - 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.
            vinodkv Vinod Kumar Vavilapalli added a comment - - edited

            junping_du, whatever works - nested class or mocking. xgong, let's fix the test so that we can get it in today, tx.

            vinodkv Vinod Kumar Vavilapalli added a comment - - edited junping_du , whatever works - nested class or mocking. xgong , let's fix the test so that we can get it in today, tx.
            xgong Xuan Gong added a comment -

            update a new patch to fix the testcase

            xgong Xuan Gong added a comment - update a new patch to fix the testcase
            hadoopqa Hadoop QA added a comment -
            -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 YARN-4955
            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.

            hadoopqa Hadoop QA added a comment - -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 YARN-4955 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.

            vinodkv Vinod Kumar Vavilapalli added a comment - 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.
            vinodkv Vinod Kumar Vavilapalli added a comment - - edited

            Committed this to trunk, branch-2 and branch-2.8. Thanks xgong!

            Tx for the review help gtCarrera9 / junping_du.

            vinodkv Vinod Kumar Vavilapalli added a comment - - edited Committed this to trunk, branch-2 and branch-2.8. Thanks xgong ! Tx for the review help gtCarrera9 / junping_du .
            junping_du Junping Du added a comment -

            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 Junping Du added a comment - 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.
            vinodkv Vinod Kumar Vavilapalli added a comment - - edited

            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.

            vinodkv Vinod Kumar Vavilapalli added a comment - - edited 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.
            junping_du Junping Du added a comment -

            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.

            junping_du Junping Du added a comment - 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.
            andrew.wang Andrew Wang added a comment -

            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 -0700

            Fixed TimelineClient to retry SocketTimeoutException too. Contributed by Xuan Gong.

            andrew.wang Andrew Wang added a comment - 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 -0700 Fixed TimelineClient to retry SocketTimeoutException too. Contributed by Xuan Gong.

            People

              xgong Xuan Gong
              xgong Xuan Gong
              Votes:
              0 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: