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

TestYarnClient#testReservationDelete fails

    Details

    • Type: Test
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.8.0, 3.0.0-alpha1
    • Component/s: None
    • Labels:
    • Hadoop Flags:
      Reviewed

      Description

      In build report report, below test fails.

      Tests run: 28, Failures: 3, Errors: 0, Skipped: 0, Time elapsed: 26.066 sec <<< FAILURE! - in org.apache.hadoop.yarn.client.api.impl.TestYarnClient
      testReservationDelete(org.apache.hadoop.yarn.client.api.impl.TestYarnClient)  Time elapsed: 2.213 sec  <<< FAILURE!
      java.lang.AssertionError: Exhausted attempts in checking if node capacity was added to the plan
      	at org.junit.Assert.fail(Assert.java:88)
      	at org.apache.hadoop.yarn.client.api.impl.TestYarnClient.setupMiniYARNCluster(TestYarnClient.java:1222)
      	at org.apache.hadoop.yarn.client.api.impl.TestYarnClient.testReservationDelete(TestYarnClient.java:1584)
      
      testListReservationsByInvalidTimeInterval(org.apache.hadoop.yarn.client.api.impl.TestYarnClient)  Time elapsed: 2.215 sec  <<< FAILURE!
      java.lang.AssertionError: Exhausted attempts in checking if node capacity was added to the plan
      	at org.junit.Assert.fail(Assert.java:88)
      	at org.apache.hadoop.yarn.client.api.impl.TestYarnClient.setupMiniYARNCluster(TestYarnClient.java:1222)
      	at org.apache.hadoop.yarn.client.api.impl.TestYarnClient.testListReservationsByInvalidTimeInterval(TestYarnClient.java:1444)
      
      testListReservationsByTimeIntervalContainingNoReservations(org.apache.hadoop.yarn.client.api.impl.TestYarnClient)  Time elapsed: 2.206 sec  <<< FAILURE!
      java.lang.AssertionError: Exhausted attempts in checking if node capacity was added to the plan
      	at org.junit.Assert.fail(Assert.java:88)
      	at org.apache.hadoop.yarn.client.api.impl.TestYarnClient.setupMiniYARNCluster(TestYarnClient.java:1222)
      	at org.apache.hadoop.yarn.client.api.impl.TestYarnClient.testListReservationsByTimeIntervalContainingNoReservations(TestYarnClient.java:1494)
      
      1. YARN-5389.v1.patch
        0.8 kB
        Sean Po
      2. YARN-5389.v2.patch
        3 kB
        Sean Po
      3. YARN-5389.v3.patch
        15 kB
        Sean Po
      4. YARN-5389.v4.patch
        5 kB
        Sean Po

        Issue Links

          Activity

          Hide
          seanpo03 Sean Po added a comment -

          The issue is a timeout issue that occurs when it takes longer than a second for the nodes to connect. YARN-5389.v1.patch increases the timeout from one second to ten seconds.

          Show
          seanpo03 Sean Po added a comment - The issue is a timeout issue that occurs when it takes longer than a second for the nodes to connect. YARN-5389 .v1.patch increases the timeout from one second to ten seconds.
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 19s 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 46s trunk passed
          +1 compile 0m 20s trunk passed
          +1 checkstyle 0m 16s trunk passed
          +1 mvnsite 0m 24s trunk passed
          +1 mvneclipse 0m 15s trunk passed
          +1 findbugs 0m 28s trunk passed
          +1 javadoc 0m 15s trunk passed
          +1 mvninstall 0m 20s the patch passed
          +1 compile 0m 17s the patch passed
          +1 javac 0m 17s the patch passed
          +1 checkstyle 0m 13s the patch passed
          +1 mvnsite 0m 22s the patch passed
          +1 mvneclipse 0m 12s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 34s the patch passed
          +1 javadoc 0m 12s the patch passed
          +1 unit 16m 0s hadoop-yarn-client in the patch passed.
          +1 asflicense 0m 15s The patch does not generate ASF License warnings.
          28m 7s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12824653/YARN-5389.v1.patch
          JIRA Issue YARN-5389
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 315d00b7e737 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 / 723facf
          Default Java 1.8.0_101
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12838/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/12838/console
          Powered by Apache Yetus 0.3.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 19s 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 46s trunk passed +1 compile 0m 20s trunk passed +1 checkstyle 0m 16s trunk passed +1 mvnsite 0m 24s trunk passed +1 mvneclipse 0m 15s trunk passed +1 findbugs 0m 28s trunk passed +1 javadoc 0m 15s trunk passed +1 mvninstall 0m 20s the patch passed +1 compile 0m 17s the patch passed +1 javac 0m 17s the patch passed +1 checkstyle 0m 13s the patch passed +1 mvnsite 0m 22s the patch passed +1 mvneclipse 0m 12s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 34s the patch passed +1 javadoc 0m 12s the patch passed +1 unit 16m 0s hadoop-yarn-client in the patch passed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 28m 7s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12824653/YARN-5389.v1.patch JIRA Issue YARN-5389 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 315d00b7e737 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 / 723facf Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12838/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client Console output https://builds.apache.org/job/PreCommit-YARN-Build/12838/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          jlowe Jason Lowe added a comment -

          Thanks for the patch!

          We should not add longer sleeps in tests. It just makes the tests run much slower overall. If the problem is that the polling loop doesn't run long enough then instead of increasing the sleep time per loop it would be better to increase the number of loop iterations. Otherwise we could waste up to a full second. I know that doesn't sound like much, but when that loss is aggregated over thousands of tests doing things like this it quickly adds up to tens of minutes. See the discussion in YARN-5393.

          My preference would be to lower the sleep time to 10 msec and increase the iterations accordingly to achieve the targeted total duration we will wait. GenericTestUtils#waitFor can be leveraged if desired.

          Show
          jlowe Jason Lowe added a comment - Thanks for the patch! We should not add longer sleeps in tests. It just makes the tests run much slower overall. If the problem is that the polling loop doesn't run long enough then instead of increasing the sleep time per loop it would be better to increase the number of loop iterations. Otherwise we could waste up to a full second. I know that doesn't sound like much, but when that loss is aggregated over thousands of tests doing things like this it quickly adds up to tens of minutes. See the discussion in YARN-5393 . My preference would be to lower the sleep time to 10 msec and increase the iterations accordingly to achieve the targeted total duration we will wait. GenericTestUtils#waitFor can be leveraged if desired.
          Hide
          seanpo03 Sean Po added a comment -

          Thanks for the good feedback Jason Lowe, YARN-5389.v2.patch uses GenericTestUtils#waitFor to verify that nodes have connected. It polls every 100ms for 10s.

          Show
          seanpo03 Sean Po added a comment - Thanks for the good feedback Jason Lowe , YARN-5389 .v2.patch uses GenericTestUtils#waitFor to verify that nodes have connected. It polls every 100ms for 10s.
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 18s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 1s The patch appears to include 1 new or modified test files.
          +1 mvninstall 6m 46s trunk passed
          +1 compile 0m 20s trunk passed
          +1 checkstyle 0m 16s trunk passed
          +1 mvnsite 0m 27s trunk passed
          +1 mvneclipse 0m 15s trunk passed
          +1 findbugs 0m 28s trunk passed
          +1 javadoc 0m 14s trunk passed
          +1 mvninstall 0m 18s the patch passed
          +1 compile 0m 16s the patch passed
          +1 javac 0m 16s the patch passed
          +1 checkstyle 0m 11s the patch passed
          +1 mvnsite 0m 21s the patch passed
          +1 mvneclipse 0m 12s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 33s the patch passed
          +1 javadoc 0m 11s the patch passed
          +1 unit 15m 50s hadoop-yarn-client in the patch passed.
          +1 asflicense 0m 15s The patch does not generate ASF License warnings.
          27m 50s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12824952/YARN-5389.v2.patch
          JIRA Issue YARN-5389
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux fcefc9ddec02 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 / 3ca4d6d
          Default Java 1.8.0_101
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12854/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/12854/console
          Powered by Apache Yetus 0.3.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 18s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 1s The patch appears to include 1 new or modified test files. +1 mvninstall 6m 46s trunk passed +1 compile 0m 20s trunk passed +1 checkstyle 0m 16s trunk passed +1 mvnsite 0m 27s trunk passed +1 mvneclipse 0m 15s trunk passed +1 findbugs 0m 28s trunk passed +1 javadoc 0m 14s trunk passed +1 mvninstall 0m 18s the patch passed +1 compile 0m 16s the patch passed +1 javac 0m 16s the patch passed +1 checkstyle 0m 11s the patch passed +1 mvnsite 0m 21s the patch passed +1 mvneclipse 0m 12s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 33s the patch passed +1 javadoc 0m 11s the patch passed +1 unit 15m 50s hadoop-yarn-client in the patch passed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 27m 50s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12824952/YARN-5389.v2.patch JIRA Issue YARN-5389 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux fcefc9ddec02 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 / 3ca4d6d Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12854/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client Console output https://builds.apache.org/job/PreCommit-YARN-Build/12854/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          jlowe Jason Lowe added a comment -

          Thanks for updating the patch!

          I'm curious why we don't just let the timeout or interrupted exceptions bubble up and fail the test like any other uncaught exception during a unit test. The resulting stacktrace will show us exactly where the timeout or interrupt occurred, so I'm not sure the try/catch clause is worth it. Ah, I see now. The original tests are doing stuff like this:

                try {
                  dResponse = client.deleteReservation(dRequest);
                } catch (Exception e) {
                  Assert.fail(e.getMessage());
                }
          

          That's not good. It suppresses the stack trace of the original exception and instead replaces it with a not-so-useful one. For example if something throws an NullPointerException, this type of code will give us a stacktrace telling us "yep, we got an NPE while running this test, but I'm not going to tell you exactly where the NPE occurred."

          It's standard practice to have the unit test methods declare the exceptions they can naturally throw (often people just have all the unit test methods declared as throwing Exception) then let the test framework handle diagnostics if the unit test throws during execution. I strongly suggest we remove the Exception catching in the methods and just let the exceptions bubble up to the test framework so we can get useful stacktraces when something fails.

          Also curious why we're going with 100ms sleeps instead of 10 as I suggested above. Are there concerns of CPU usage? The check every iteration seems very cheap, so it's overall impact should be minimal. Not a must-fix, but it is something that's reused in multiple tests (and likely by additional unit tests in the future) so it'd be nice to eliminate the extra waste.

          Show
          jlowe Jason Lowe added a comment - Thanks for updating the patch! I'm curious why we don't just let the timeout or interrupted exceptions bubble up and fail the test like any other uncaught exception during a unit test. The resulting stacktrace will show us exactly where the timeout or interrupt occurred, so I'm not sure the try/catch clause is worth it. Ah, I see now. The original tests are doing stuff like this: try { dResponse = client.deleteReservation(dRequest); } catch (Exception e) { Assert.fail(e.getMessage()); } That's not good. It suppresses the stack trace of the original exception and instead replaces it with a not-so-useful one. For example if something throws an NullPointerException, this type of code will give us a stacktrace telling us "yep, we got an NPE while running this test, but I'm not going to tell you exactly where the NPE occurred." It's standard practice to have the unit test methods declare the exceptions they can naturally throw (often people just have all the unit test methods declared as throwing Exception) then let the test framework handle diagnostics if the unit test throws during execution. I strongly suggest we remove the Exception catching in the methods and just let the exceptions bubble up to the test framework so we can get useful stacktraces when something fails. Also curious why we're going with 100ms sleeps instead of 10 as I suggested above. Are there concerns of CPU usage? The check every iteration seems very cheap, so it's overall impact should be minimal. Not a must-fix, but it is something that's reused in multiple tests (and likely by additional unit tests in the future) so it'd be nice to eliminate the extra waste.
          Hide
          seanpo03 Sean Po added a comment -

          Thanks Jason Lowe, the 10ms suggestion was overlooked in your previous suggestion, but the change is made in v3 of the patch. I also removed any try catch that asserts fail in the event of an exception to let the exception bubble up.

          Show
          seanpo03 Sean Po added a comment - Thanks Jason Lowe , the 10ms suggestion was overlooked in your previous suggestion, but the change is made in v3 of the patch. I also removed any try catch that asserts fail in the event of an exception to let the exception bubble up.
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 18s 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 53s trunk passed
          +1 compile 0m 19s trunk passed
          +1 checkstyle 0m 15s trunk passed
          +1 mvnsite 0m 24s trunk passed
          +1 mvneclipse 0m 15s trunk passed
          +1 findbugs 0m 28s trunk passed
          +1 javadoc 0m 15s trunk passed
          +1 mvninstall 0m 18s the patch passed
          +1 compile 0m 17s the patch passed
          +1 javac 0m 17s the patch passed
          +1 checkstyle 0m 12s the patch passed
          +1 mvnsite 0m 21s the patch passed
          +1 mvneclipse 0m 12s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 34s the patch passed
          +1 javadoc 0m 12s the patch passed
          +1 unit 15m 55s hadoop-yarn-client in the patch passed.
          +1 asflicense 0m 16s The patch does not generate ASF License warnings.
          28m 0s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12825321/YARN-5389.v3.patch
          JIRA Issue YARN-5389
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 9ab8a8fb3ec3 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 / 3476156
          Default Java 1.8.0_101
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12879/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/12879/console
          Powered by Apache Yetus 0.3.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 18s 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 53s trunk passed +1 compile 0m 19s trunk passed +1 checkstyle 0m 15s trunk passed +1 mvnsite 0m 24s trunk passed +1 mvneclipse 0m 15s trunk passed +1 findbugs 0m 28s trunk passed +1 javadoc 0m 15s trunk passed +1 mvninstall 0m 18s the patch passed +1 compile 0m 17s the patch passed +1 javac 0m 17s the patch passed +1 checkstyle 0m 12s the patch passed +1 mvnsite 0m 21s the patch passed +1 mvneclipse 0m 12s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 34s the patch passed +1 javadoc 0m 12s the patch passed +1 unit 15m 55s hadoop-yarn-client in the patch passed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 28m 0s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12825321/YARN-5389.v3.patch JIRA Issue YARN-5389 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 9ab8a8fb3ec3 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 / 3476156 Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12879/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client Console output https://builds.apache.org/job/PreCommit-YARN-Build/12879/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          jlowe Jason Lowe added a comment -

          Thanks for updating the patch!

          Nit: The getNewReservation method is no longer useful since it's just a one-liner that callers can do directly.

          Since we're cleaning up bad exception catching practices in the file, there are still some cases where we're needlessly suppressing exceptions:

               try {
                  client.submitApplication(context);
                } catch (YarnException e) {
                  Assert.fail("Exception is not expected.");
                } catch (IOException e) {
                  Assert.fail("Exception is not expected.");
                }
          
              try {
                conf.setBoolean(YarnConfiguration.TIMELINE_SERVICE_CLIENT_BEST_EFFORT, true);
                client.serviceInit(conf);
                client.getTimelineDelegationToken();
              } catch (Exception e) {
                Assert.fail("Should not have thrown an exception");
              }
          
              try {
                sResponse = client.submitReservation(sRequest);
              } catch (Exception e) {
                Assert.fail(e.getMessage());
              }
          

          and this needless wrapping:

              } catch (Exception ex) {
                throw new Exception(ex);
              }
          

          Actually there's so enough cleanup in this file that it may be better to do the minimal exception changes to fix testReservationDelete's handling of exceptions and get this unit test failure fixed, then we can tackle cleaning up the exception handling of the other test methods in this file in another JIRA.

          Show
          jlowe Jason Lowe added a comment - Thanks for updating the patch! Nit: The getNewReservation method is no longer useful since it's just a one-liner that callers can do directly. Since we're cleaning up bad exception catching practices in the file, there are still some cases where we're needlessly suppressing exceptions: try { client.submitApplication(context); } catch (YarnException e) { Assert.fail( "Exception is not expected." ); } catch (IOException e) { Assert.fail( "Exception is not expected." ); } try { conf.setBoolean(YarnConfiguration.TIMELINE_SERVICE_CLIENT_BEST_EFFORT, true ); client.serviceInit(conf); client.getTimelineDelegationToken(); } catch (Exception e) { Assert.fail( "Should not have thrown an exception" ); } try { sResponse = client.submitReservation(sRequest); } catch (Exception e) { Assert.fail(e.getMessage()); } and this needless wrapping: } catch (Exception ex) { throw new Exception(ex); } Actually there's so enough cleanup in this file that it may be better to do the minimal exception changes to fix testReservationDelete's handling of exceptions and get this unit test failure fixed, then we can tackle cleaning up the exception handling of the other test methods in this file in another JIRA.
          Hide
          seanpo03 Sean Po added a comment -

          Thanks for the comments Jason Lowe. I agree - in the next patch, I will revert changes that remove code that does something similar to the following:

              try {
                sResponse = client.submitReservation(sRequest);
              } catch (Exception e) {
                Assert.fail(e.getMessage());
              }
          

          I've created YARN-5560 to track cleaning up such patterns in TestYarnClient.

          Show
          seanpo03 Sean Po added a comment - Thanks for the comments Jason Lowe . I agree - in the next patch, I will revert changes that remove code that does something similar to the following: try { sResponse = client.submitReservation(sRequest); } catch (Exception e) { Assert.fail(e.getMessage()); } I've created YARN-5560 to track cleaning up such patterns in TestYarnClient.
          Hide
          seanpo03 Sean Po added a comment -

          YARN-5389.v4.patch contains the changes spoken about in my previous comment.

          Show
          seanpo03 Sean Po added a comment - YARN-5389 .v4.patch contains the changes spoken about in my previous comment.
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 14s 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 7m 24s trunk passed
          +1 compile 0m 21s trunk passed
          +1 checkstyle 0m 15s trunk passed
          +1 mvnsite 0m 25s trunk passed
          +1 mvneclipse 0m 15s trunk passed
          +1 findbugs 0m 30s trunk passed
          +1 javadoc 0m 15s trunk passed
          +1 mvninstall 0m 20s the patch passed
          +1 compile 0m 17s the patch passed
          +1 javac 0m 17s the patch passed
          +1 checkstyle 0m 12s the patch passed
          +1 mvnsite 0m 22s the patch passed
          +1 mvneclipse 0m 12s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 34s the patch passed
          +1 javadoc 0m 12s the patch passed
          +1 unit 15m 52s hadoop-yarn-client in the patch passed.
          +1 asflicense 0m 17s The patch does not generate ASF License warnings.
          28m 31s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12825378/YARN-5389.v4.patch
          JIRA Issue YARN-5389
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 0f892f2fdda4 3.13.0-92-generic #139-Ubuntu SMP Tue Jun 28 20:42:26 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / a1f3293
          Default Java 1.8.0_101
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12885/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/12885/console
          Powered by Apache Yetus 0.3.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 14s 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 7m 24s trunk passed +1 compile 0m 21s trunk passed +1 checkstyle 0m 15s trunk passed +1 mvnsite 0m 25s trunk passed +1 mvneclipse 0m 15s trunk passed +1 findbugs 0m 30s trunk passed +1 javadoc 0m 15s trunk passed +1 mvninstall 0m 20s the patch passed +1 compile 0m 17s the patch passed +1 javac 0m 17s the patch passed +1 checkstyle 0m 12s the patch passed +1 mvnsite 0m 22s the patch passed +1 mvneclipse 0m 12s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 34s the patch passed +1 javadoc 0m 12s the patch passed +1 unit 15m 52s hadoop-yarn-client in the patch passed. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 28m 31s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12825378/YARN-5389.v4.patch JIRA Issue YARN-5389 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 0f892f2fdda4 3.13.0-92-generic #139-Ubuntu SMP Tue Jun 28 20:42:26 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / a1f3293 Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12885/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client Console output https://builds.apache.org/job/PreCommit-YARN-Build/12885/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          jlowe Jason Lowe added a comment -

          +1 for the latest patch. Committing this.

          Show
          jlowe Jason Lowe added a comment - +1 for the latest patch. Committing this.
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10345 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10345/)
          YARN-5389. TestYarnClient#testReservationDelete fails. Contributed by (jlowe: rev 3d86110a5ccfdaff8671fb6ad8f67b4ab66f33da)

          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/api/impl/TestYarnClient.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10345 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10345/ ) YARN-5389 . TestYarnClient#testReservationDelete fails. Contributed by (jlowe: rev 3d86110a5ccfdaff8671fb6ad8f67b4ab66f33da) (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/api/impl/TestYarnClient.java
          Hide
          jlowe Jason Lowe added a comment -

          Thanks, Sean Po! I committed this to trunk, branch-2, and branch-2.8.

          Show
          jlowe Jason Lowe added a comment - Thanks, Sean Po ! I committed this to trunk, branch-2, and branch-2.8.
          Hide
          seanpo03 Sean Po added a comment -

          Thanks for the review Jason Lowe!

          Show
          seanpo03 Sean Po added a comment - Thanks for the review Jason Lowe !

            People

            • Assignee:
              seanpo03 Sean Po
              Reporter:
              rohithsharma Rohith Sharma K S
            • Votes:
              0 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development