Details

    • Hadoop Flags:
      Reviewed

      Description

      FairScheduler#nodeUpdate() and CapacityScheduler#nodeUpdate() have a lot of commonality in their code. See about refactoring the common parts into AbstractYARNScheduler.

      1. YARN-5047.012.patch
        35 kB
        Ray Chiang
      2. YARN-5047.011.patch
        36 kB
        Ray Chiang
      3. YARN-5047.010.patch
        33 kB
        Ray Chiang
      4. YARN-5047.009.patch
        34 kB
        Ray Chiang
      5. YARN-5047.008.patch
        34 kB
        Ray Chiang
      6. YARN-5047.007.patch
        25 kB
        Ray Chiang
      7. YARN-5047.006.patch
        25 kB
        Ray Chiang
      8. YARN-5047.005.patch
        30 kB
        Ray Chiang
      9. YARN-5047.004.patch
        30 kB
        Ray Chiang
      10. YARN-5047.003.patch
        29 kB
        Ray Chiang
      11. YARN-5047.002.patch
        36 kB
        Ray Chiang
      12. YARN-5047.001.patch
        36 kB
        Ray Chiang

        Issue Links

          Activity

          Hide
          rchiang Ray Chiang added a comment -
          • First attempt at refactoring
          Show
          rchiang Ray Chiang added a comment - First attempt at refactoring
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 12s 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.
          0 mvndep 0m 18s Maven dependency ordering for branch
          +1 mvninstall 9m 12s trunk passed
          +1 compile 10m 31s trunk passed with JDK v1.8.0_91
          +1 compile 9m 29s trunk passed with JDK v1.7.0_95
          +1 checkstyle 1m 53s trunk passed
          +1 mvnsite 1m 11s trunk passed
          +1 mvneclipse 0m 36s trunk passed
          +1 findbugs 2m 8s trunk passed
          +1 javadoc 0m 55s trunk passed with JDK v1.8.0_91
          +1 javadoc 0m 52s trunk passed with JDK v1.7.0_95
          0 mvndep 0m 19s Maven dependency ordering for patch
          +1 mvninstall 1m 1s the patch passed
          +1 compile 10m 39s the patch passed with JDK v1.8.0_91
          +1 javac 10m 39s the patch passed
          +1 compile 9m 23s the patch passed with JDK v1.7.0_95
          +1 javac 9m 23s the patch passed
          -1 checkstyle 1m 50s root: patch generated 1 new + 321 unchanged - 15 fixed = 322 total (was 336)
          +1 mvnsite 1m 10s the patch passed
          +1 mvneclipse 0m 34s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          -1 findbugs 1m 46s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager generated 2 new + 0 unchanged - 0 fixed = 2 total (was 0)
          +1 javadoc 0m 53s the patch passed with JDK v1.8.0_91
          +1 javadoc 0m 53s the patch passed with JDK v1.7.0_95
          -1 unit 37m 51s hadoop-yarn-server-resourcemanager in the patch failed with JDK v1.8.0_91.
          +1 unit 1m 9s hadoop-sls in the patch passed with JDK v1.8.0_91.
          -1 unit 35m 34s hadoop-yarn-server-resourcemanager in the patch failed with JDK v1.7.0_95.
          +1 unit 1m 4s hadoop-sls in the patch passed with JDK v1.7.0_95.
          +1 asflicense 0m 27s Patch does not generate ASF License warnings.
          144m 27s



          Reason Tests
          FindBugs module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
            Inconsistent synchronization of org.apache.hadoop.yarn.server.resourcemanager.scheduler.fair.FairScheduler.eventLog; locked 50% of time Unsynchronized access at FairScheduler.java:50% of time Unsynchronized access at FairScheduler.java:[line 598]
            Inconsistent synchronization of org.apache.hadoop.yarn.server.resourcemanager.scheduler.fifo.FifoScheduler.metrics; locked 80% of time Unsynchronized access at FifoScheduler.java:80% of time Unsynchronized access at FifoScheduler.java:[line 738]
          JDK v1.8.0_91 Failed junit tests hadoop.yarn.server.resourcemanager.TestClientRMTokens
            hadoop.yarn.server.resourcemanager.TestAMAuthorization
            hadoop.yarn.server.resourcemanager.TestRMRestart
            hadoop.yarn.server.resourcemanager.security.TestDelegationTokenRenewer
            hadoop.yarn.server.resourcemanager.TestContainerResourceUsage
          JDK v1.7.0_95 Failed junit tests hadoop.yarn.server.resourcemanager.TestClientRMTokens
            hadoop.yarn.server.resourcemanager.TestAMAuthorization
            hadoop.yarn.server.resourcemanager.security.TestDelegationTokenRenewer
            hadoop.yarn.server.resourcemanager.TestContainerResourceUsage



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:cf2ee45
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12803122/YARN-5047.001.patch
          JIRA Issue YARN-5047
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux ed9c0fa29b85 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 / 996a210
          Default Java 1.7.0_95
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_91 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/11389/artifact/patchprocess/diff-checkstyle-root.txt
          findbugs https://builds.apache.org/job/PreCommit-YARN-Build/11389/artifact/patchprocess/new-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.html
          unit https://builds.apache.org/job/PreCommit-YARN-Build/11389/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.8.0_91.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/11389/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.7.0_95.txt
          unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/11389/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.8.0_91.txt https://builds.apache.org/job/PreCommit-YARN-Build/11389/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.7.0_95.txt
          JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11389/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-tools/hadoop-sls U: .
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/11389/console
          Powered by Apache Yetus 0.2.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 12s 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. 0 mvndep 0m 18s Maven dependency ordering for branch +1 mvninstall 9m 12s trunk passed +1 compile 10m 31s trunk passed with JDK v1.8.0_91 +1 compile 9m 29s trunk passed with JDK v1.7.0_95 +1 checkstyle 1m 53s trunk passed +1 mvnsite 1m 11s trunk passed +1 mvneclipse 0m 36s trunk passed +1 findbugs 2m 8s trunk passed +1 javadoc 0m 55s trunk passed with JDK v1.8.0_91 +1 javadoc 0m 52s trunk passed with JDK v1.7.0_95 0 mvndep 0m 19s Maven dependency ordering for patch +1 mvninstall 1m 1s the patch passed +1 compile 10m 39s the patch passed with JDK v1.8.0_91 +1 javac 10m 39s the patch passed +1 compile 9m 23s the patch passed with JDK v1.7.0_95 +1 javac 9m 23s the patch passed -1 checkstyle 1m 50s root: patch generated 1 new + 321 unchanged - 15 fixed = 322 total (was 336) +1 mvnsite 1m 10s the patch passed +1 mvneclipse 0m 34s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. -1 findbugs 1m 46s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager generated 2 new + 0 unchanged - 0 fixed = 2 total (was 0) +1 javadoc 0m 53s the patch passed with JDK v1.8.0_91 +1 javadoc 0m 53s the patch passed with JDK v1.7.0_95 -1 unit 37m 51s hadoop-yarn-server-resourcemanager in the patch failed with JDK v1.8.0_91. +1 unit 1m 9s hadoop-sls in the patch passed with JDK v1.8.0_91. -1 unit 35m 34s hadoop-yarn-server-resourcemanager in the patch failed with JDK v1.7.0_95. +1 unit 1m 4s hadoop-sls in the patch passed with JDK v1.7.0_95. +1 asflicense 0m 27s Patch does not generate ASF License warnings. 144m 27s Reason Tests FindBugs module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager   Inconsistent synchronization of org.apache.hadoop.yarn.server.resourcemanager.scheduler.fair.FairScheduler.eventLog; locked 50% of time Unsynchronized access at FairScheduler.java:50% of time Unsynchronized access at FairScheduler.java: [line 598]   Inconsistent synchronization of org.apache.hadoop.yarn.server.resourcemanager.scheduler.fifo.FifoScheduler.metrics; locked 80% of time Unsynchronized access at FifoScheduler.java:80% of time Unsynchronized access at FifoScheduler.java: [line 738] JDK v1.8.0_91 Failed junit tests hadoop.yarn.server.resourcemanager.TestClientRMTokens   hadoop.yarn.server.resourcemanager.TestAMAuthorization   hadoop.yarn.server.resourcemanager.TestRMRestart   hadoop.yarn.server.resourcemanager.security.TestDelegationTokenRenewer   hadoop.yarn.server.resourcemanager.TestContainerResourceUsage JDK v1.7.0_95 Failed junit tests hadoop.yarn.server.resourcemanager.TestClientRMTokens   hadoop.yarn.server.resourcemanager.TestAMAuthorization   hadoop.yarn.server.resourcemanager.security.TestDelegationTokenRenewer   hadoop.yarn.server.resourcemanager.TestContainerResourceUsage Subsystem Report/Notes Docker Image:yetus/hadoop:cf2ee45 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12803122/YARN-5047.001.patch JIRA Issue YARN-5047 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux ed9c0fa29b85 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 / 996a210 Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_91 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/11389/artifact/patchprocess/diff-checkstyle-root.txt findbugs https://builds.apache.org/job/PreCommit-YARN-Build/11389/artifact/patchprocess/new-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.html unit https://builds.apache.org/job/PreCommit-YARN-Build/11389/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.8.0_91.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/11389/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.7.0_95.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/11389/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.8.0_91.txt https://builds.apache.org/job/PreCommit-YARN-Build/11389/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.7.0_95.txt JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11389/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-tools/hadoop-sls U: . Console output https://builds.apache.org/job/PreCommit-YARN-Build/11389/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
          Hide
          rchiang Ray Chiang added a comment -
          • Fix missing synchronize in overridden method FairScheduler#nodeUpdate()
          Show
          rchiang Ray Chiang added a comment - Fix missing synchronize in overridden method FairScheduler#nodeUpdate()
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 11s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
          0 mvndep 0m 31s Maven dependency ordering for branch
          +1 mvninstall 6m 55s trunk passed
          +1 compile 7m 23s trunk passed
          +1 checkstyle 1m 31s trunk passed
          +1 mvnsite 0m 58s trunk passed
          +1 mvneclipse 0m 24s trunk passed
          +1 findbugs 1m 24s trunk passed
          +1 javadoc 0m 38s trunk passed
          0 mvndep 0m 13s Maven dependency ordering for patch
          +1 mvninstall 0m 46s the patch passed
          +1 compile 7m 56s the patch passed
          +1 javac 7m 56s the patch passed
          -1 checkstyle 1m 25s root: patch generated 1 new + 322 unchanged - 15 fixed = 323 total (was 337)
          +1 mvnsite 0m 56s the patch passed
          +1 mvneclipse 0m 25s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          -1 findbugs 1m 13s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
          +1 javadoc 0m 37s the patch passed
          -1 unit 30m 30s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 unit 0m 52s hadoop-sls in the patch passed.
          +1 asflicense 0m 20s Patch does not generate ASF License warnings.
          66m 36s



          Reason Tests
          FindBugs module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
            Inconsistent synchronization of org.apache.hadoop.yarn.server.resourcemanager.scheduler.fifo.FifoScheduler.metrics; locked 80% of time Unsynchronized access at FifoScheduler.java:80% of time Unsynchronized access at FifoScheduler.java:[line 738]
          Failed junit tests hadoop.yarn.server.resourcemanager.TestAMAuthorization
            hadoop.yarn.server.resourcemanager.TestRMRestart
            hadoop.yarn.server.resourcemanager.TestContainerResourceUsage
            hadoop.yarn.server.resourcemanager.TestClientRMTokens



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:2c91fd8
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12804492/YARN-5047.002.patch
          JIRA Issue YARN-5047
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 9fb69c81e28c 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 / 7cd5ae6
          Default Java 1.8.0_91
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/11509/artifact/patchprocess/diff-checkstyle-root.txt
          findbugs https://builds.apache.org/job/PreCommit-YARN-Build/11509/artifact/patchprocess/new-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.html
          unit https://builds.apache.org/job/PreCommit-YARN-Build/11509/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/11509/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11509/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-tools/hadoop-sls U: .
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/11509/console
          Powered by Apache Yetus 0.2.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 11s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. 0 mvndep 0m 31s Maven dependency ordering for branch +1 mvninstall 6m 55s trunk passed +1 compile 7m 23s trunk passed +1 checkstyle 1m 31s trunk passed +1 mvnsite 0m 58s trunk passed +1 mvneclipse 0m 24s trunk passed +1 findbugs 1m 24s trunk passed +1 javadoc 0m 38s trunk passed 0 mvndep 0m 13s Maven dependency ordering for patch +1 mvninstall 0m 46s the patch passed +1 compile 7m 56s the patch passed +1 javac 7m 56s the patch passed -1 checkstyle 1m 25s root: patch generated 1 new + 322 unchanged - 15 fixed = 323 total (was 337) +1 mvnsite 0m 56s the patch passed +1 mvneclipse 0m 25s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. -1 findbugs 1m 13s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) +1 javadoc 0m 37s the patch passed -1 unit 30m 30s hadoop-yarn-server-resourcemanager in the patch failed. +1 unit 0m 52s hadoop-sls in the patch passed. +1 asflicense 0m 20s Patch does not generate ASF License warnings. 66m 36s Reason Tests FindBugs module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager   Inconsistent synchronization of org.apache.hadoop.yarn.server.resourcemanager.scheduler.fifo.FifoScheduler.metrics; locked 80% of time Unsynchronized access at FifoScheduler.java:80% of time Unsynchronized access at FifoScheduler.java: [line 738] Failed junit tests hadoop.yarn.server.resourcemanager.TestAMAuthorization   hadoop.yarn.server.resourcemanager.TestRMRestart   hadoop.yarn.server.resourcemanager.TestContainerResourceUsage   hadoop.yarn.server.resourcemanager.TestClientRMTokens Subsystem Report/Notes Docker Image:yetus/hadoop:2c91fd8 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12804492/YARN-5047.002.patch JIRA Issue YARN-5047 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 9fb69c81e28c 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 / 7cd5ae6 Default Java 1.8.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/11509/artifact/patchprocess/diff-checkstyle-root.txt findbugs https://builds.apache.org/job/PreCommit-YARN-Build/11509/artifact/patchprocess/new-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.html unit https://builds.apache.org/job/PreCommit-YARN-Build/11509/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/11509/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11509/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-tools/hadoop-sls U: . Console output https://builds.apache.org/job/PreCommit-YARN-Build/11509/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
          Hide
          rchiang Ray Chiang added a comment -

          RE: no new tests

          Refactoring relying on existing tests for compatibility.

          RE: checkstyle

          Standard usage of a protected variable

          RE: findbugs

          Looks like the problem exists in FifoScheduler before

          RE: failed unit tests

          YARN-5091 and YARN-5092 filed for unit test issues

          Show
          rchiang Ray Chiang added a comment - RE: no new tests Refactoring relying on existing tests for compatibility. RE: checkstyle Standard usage of a protected variable RE: findbugs Looks like the problem exists in FifoScheduler before RE: failed unit tests YARN-5091 and YARN-5092 filed for unit test issues
          Hide
          kasha Karthik Kambatla added a comment -

          Excited to see progress here. Thanks for working on this, Ray Chiang.

          Comments:

          1. I wonder what the @Lock annotation on getNode does? Looking at the @interface Lock
          2. AbstractYarnScheduler#nodeUpdate:
            1. While we are formatting, we should probably move the definition of releaseResources closer to its use at realeasedContainers, and may be also renamed to "released" resources?
            2. Java 7 and beyond, we don't can emit the type when calling constructor. Definitions of newlyLaunchedContainers and completedContainers
            3. Is the TODO still required? If yes, mind filing a follow-up JIRA and annotating that TODO
            4. Shouldn't nodeUpdateInternal be called right at the end? The log message says we are looking at node for allocations. And, the scheduling code might want to use the latest info on aggregated containers' and node utilization. YARN-1011 does this.
            5. Should we mark this final so it can't be overriden? I see that FairScheduler overrides this to capture duration of node update. I don't remember the details clearly; are we primarily interested in the duration to schedule or processing the entire node heartbeat?
          3. There is interest to track SchedulerHealth for FairScheduler as well. Can we file a follow-up JIRA here to move the corresponding update code also to AbstractYarnScheduler so we can reuse it in FairScheduler?
          4. Why do we need the change to FiCaSchedulerApp? Looks like this comes from FifoScheduler#nodeUpdateInternal. Shouldn't the type of getNode there be FiCaSchedulerNode?
          Show
          kasha Karthik Kambatla added a comment - Excited to see progress here. Thanks for working on this, Ray Chiang . Comments: I wonder what the @Lock annotation on getNode does? Looking at the @interface Lock AbstractYarnScheduler#nodeUpdate : While we are formatting, we should probably move the definition of releaseResources closer to its use at realeasedContainers , and may be also renamed to "released" resources? Java 7 and beyond, we don't can emit the type when calling constructor. Definitions of newlyLaunchedContainers and completedContainers Is the TODO still required? If yes, mind filing a follow-up JIRA and annotating that TODO Shouldn't nodeUpdateInternal be called right at the end? The log message says we are looking at node for allocations. And, the scheduling code might want to use the latest info on aggregated containers' and node utilization. YARN-1011 does this. Should we mark this final so it can't be overriden? I see that FairScheduler overrides this to capture duration of node update. I don't remember the details clearly; are we primarily interested in the duration to schedule or processing the entire node heartbeat? There is interest to track SchedulerHealth for FairScheduler as well. Can we file a follow-up JIRA here to move the corresponding update code also to AbstractYarnScheduler so we can reuse it in FairScheduler? Why do we need the change to FiCaSchedulerApp? Looks like this comes from FifoScheduler#nodeUpdateInternal . Shouldn't the type of getNode there be FiCaSchedulerNode?
          Hide
          leftnoteasy Wangda Tan added a comment -

          Thanks Ray Chiang,

          I have a similar comment which is mentioned by Karthik Kambatla:
          Now we have nodeUpdate and nodeUpdateInternal, which is a little confusing to me. Basically we have two choices:
          1) Leave nodeUpdate only, and sub classes can override it.
          2) Keep both of nodeUpdate and nodeUpdateInternal, make nodeUpdate cannot be overriden

          Personally I would prefer #1, I'm not sure if we really need to call nodeUpdateInternal inside nodeUpdate.

          Beyond comments from Karthik Kambatla, rest part of the patch looks good to me.

          Show
          leftnoteasy Wangda Tan added a comment - Thanks Ray Chiang , I have a similar comment which is mentioned by Karthik Kambatla : Now we have nodeUpdate and nodeUpdateInternal, which is a little confusing to me. Basically we have two choices: 1) Leave nodeUpdate only, and sub classes can override it. 2) Keep both of nodeUpdate and nodeUpdateInternal, make nodeUpdate cannot be overriden Personally I would prefer #1, I'm not sure if we really need to call nodeUpdateInternal inside nodeUpdate . Beyond comments from Karthik Kambatla , rest part of the patch looks good to me.
          Hide
          kasha Karthik Kambatla added a comment -

          Thanks for the review, Wangda.

          Agree that #1 is simpler. In either case, we all just need to be more disciplined with trying to fix things in the parent class instead of individual schedulers.

          Show
          kasha Karthik Kambatla added a comment - Thanks for the review, Wangda. Agree that #1 is simpler. In either case, we all just need to be more disciplined with trying to fix things in the parent class instead of individual schedulers.
          Hide
          rchiang Ray Chiang added a comment -

          Thanks for the feedback guys. I agree with doing #1, but it looks like the FairScheduler code will change some.

          I'll have a patch up soon.

          Show
          rchiang Ray Chiang added a comment - Thanks for the feedback guys. I agree with doing #1, but it looks like the FairScheduler code will change some. I'll have a patch up soon.
          Hide
          rchiang Ray Chiang added a comment -

          Filed follow up JIRAs YARN-5128 and YARN-5129.

          Show
          rchiang Ray Chiang added a comment - Filed follow up JIRAs YARN-5128 and YARN-5129 .
          Hide
          rchiang Ray Chiang added a comment -
          • Implement earlier feedback
          • Removes optimization to assignMultiple logic in FairScheduler
          Show
          rchiang Ray Chiang added a comment - Implement earlier feedback Removes optimization to assignMultiple logic in FairScheduler
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 10s 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 7m 28s trunk passed
          +1 compile 0m 35s trunk passed
          +1 checkstyle 0m 25s trunk passed
          +1 mvnsite 0m 40s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          +1 findbugs 0m 59s trunk passed
          +1 javadoc 0m 26s trunk passed
          +1 mvninstall 0m 35s the patch passed
          +1 compile 0m 30s the patch passed
          +1 javac 0m 30s the patch passed
          -1 checkstyle 0m 21s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: patch generated 1 new + 271 unchanged - 13 fixed = 272 total (was 284)
          +1 mvnsite 0m 34s the patch passed
          +1 mvneclipse 0m 12s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 1m 12s the patch passed
          +1 javadoc 0m 23s the patch passed
          -1 unit 29m 50s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 asflicense 0m 17s Patch does not generate ASF License warnings.
          45m 27s



          Reason Tests
          Failed junit tests hadoop.yarn.server.resourcemanager.TestAMAuthorization
            hadoop.yarn.server.resourcemanager.TestClientRMTokens



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:2c91fd8
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12805752/YARN-5047.003.patch
          JIRA Issue YARN-5047
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux a7e1d6e5d97a 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 / eebb39a
          Default Java 1.8.0_91
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/11639/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/11639/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/11639/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11639/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/11639/console
          Powered by Apache Yetus 0.2.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 10s 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 7m 28s trunk passed +1 compile 0m 35s trunk passed +1 checkstyle 0m 25s trunk passed +1 mvnsite 0m 40s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 0m 59s trunk passed +1 javadoc 0m 26s trunk passed +1 mvninstall 0m 35s the patch passed +1 compile 0m 30s the patch passed +1 javac 0m 30s the patch passed -1 checkstyle 0m 21s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: patch generated 1 new + 271 unchanged - 13 fixed = 272 total (was 284) +1 mvnsite 0m 34s the patch passed +1 mvneclipse 0m 12s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 1m 12s the patch passed +1 javadoc 0m 23s the patch passed -1 unit 29m 50s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 17s Patch does not generate ASF License warnings. 45m 27s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.TestAMAuthorization   hadoop.yarn.server.resourcemanager.TestClientRMTokens Subsystem Report/Notes Docker Image:yetus/hadoop:2c91fd8 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12805752/YARN-5047.003.patch JIRA Issue YARN-5047 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux a7e1d6e5d97a 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 / eebb39a Default Java 1.8.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/11639/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/11639/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/11639/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11639/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/11639/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
          Hide
          rchiang Ray Chiang added a comment -

          RE: checkstyle

          Same issue with protected member variable.

          RE: failing unit tests

          Still YARN-5091. Tests pass in my tree.

          Show
          rchiang Ray Chiang added a comment - RE: checkstyle Same issue with protected member variable. RE: failing unit tests Still YARN-5091 . Tests pass in my tree.
          Hide
          kasha Karthik Kambatla added a comment -

          Thanks for the update, Ray. Almost there. Comments:

          1. The following piece is in all schedulers. FifoScheduler has it in nodeUpdate, while the other two have in attemptScheduling and allocateContainersToNode. We should eventually move this to nodeUpdate. May be, the right way to do that would be to define an abstract method called assignContainers(Node) in AbstractYarnScheduler and rename the existing methods in CS and FS. I am comfortable with doing this in a follow-up JIRA.
                if (rmContext.isWorkPreservingRecoveryEnabled()
                    && !rmContext.isSchedulerReadyForAllocatingContainers()) {
                  return;
                }
            
          2. Unlike other schedulers, CS seems to call allocateContainersToNode in handle() instead of nodeUpdate. May be CS should also override nodeUpdate and call this method there for uniformity as we refactor things around this? This would in preparation for moving handle() itself to AbstractYarnScheduler. /cc Wangda Tan
          3. FifoScheduler seems to explicitly check the node resources are greater than minimumAllocation. This seems like a good check and something other schedulers should likely do upfront. Another candidate for this umbrella JIRA?
          4. initScheduler has some common code across schedulers. Another JIRA to move the common pieces to AbstractYarnScheduler?
          Show
          kasha Karthik Kambatla added a comment - Thanks for the update, Ray. Almost there. Comments: The following piece is in all schedulers. FifoScheduler has it in nodeUpdate, while the other two have in attemptScheduling and allocateContainersToNode. We should eventually move this to nodeUpdate. May be, the right way to do that would be to define an abstract method called assignContainers(Node) in AbstractYarnScheduler and rename the existing methods in CS and FS. I am comfortable with doing this in a follow-up JIRA. if (rmContext.isWorkPreservingRecoveryEnabled() && !rmContext.isSchedulerReadyForAllocatingContainers()) { return ; } Unlike other schedulers, CS seems to call allocateContainersToNode in handle() instead of nodeUpdate. May be CS should also override nodeUpdate and call this method there for uniformity as we refactor things around this? This would in preparation for moving handle() itself to AbstractYarnScheduler. /cc Wangda Tan FifoScheduler seems to explicitly check the node resources are greater than minimumAllocation. This seems like a good check and something other schedulers should likely do upfront. Another candidate for this umbrella JIRA? initScheduler has some common code across schedulers. Another JIRA to move the common pieces to AbstractYarnScheduler?
          Hide
          leftnoteasy Wangda Tan added a comment -

          Karthik Kambatla/Ray Chiang.

          Unlike other schedulers, CS seems to call allocateContainersToNode in handle() instead of nodeUpdate. May be CS should also override nodeUpdate and call this method there for uniformity as we refactor things around this? This would in preparation for moving handle() itself to AbstractYarnScheduler. /cc Wangda Tan

          Not entirely sure if I understand the proposal. It looks like a good solution if we can add an abstract assignContainers to AbstractYarnScheduler as you proposed above, call assginContainers from AbstractYarnScheduler#assignContainers, and move allocateContainersToNode to assignContainers.

          Show
          leftnoteasy Wangda Tan added a comment - Karthik Kambatla / Ray Chiang . Unlike other schedulers, CS seems to call allocateContainersToNode in handle() instead of nodeUpdate. May be CS should also override nodeUpdate and call this method there for uniformity as we refactor things around this? This would in preparation for moving handle() itself to AbstractYarnScheduler. /cc Wangda Tan Not entirely sure if I understand the proposal. It looks like a good solution if we can add an abstract assignContainers to AbstractYarnScheduler as you proposed above, call assginContainers from AbstractYarnScheduler#assignContainers, and move allocateContainersToNode to assignContainers.
          Hide
          rchiang Ray Chiang added a comment -
          • Added overriding nodeUpdate() to CapacityScheduler. Will make handle() refactoring easier.
          Show
          rchiang Ray Chiang added a comment - Added overriding nodeUpdate() to CapacityScheduler. Will make handle() refactoring easier.
          Show
          rchiang Ray Chiang added a comment - Thanks Tan, Wangda and Karthik Kambatla . I've filed the following JIRAs: YARN-5283 Refactor container assignment into AbstractYarnScheduler#assignContainers YARN-5284 Refactor handle across schedulers YARN-5285 Refactor common code in initScheduler across schedulers
          Hide
          rchiang Ray Chiang added a comment -

          Adding same patch for Jenkins run.

          Show
          rchiang Ray Chiang added a comment - Adding same patch for Jenkins run.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 36s 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 32s trunk passed
          +1 compile 0m 37s trunk passed
          +1 checkstyle 0m 25s trunk passed
          +1 mvnsite 0m 37s trunk passed
          +1 mvneclipse 0m 16s trunk passed
          +1 findbugs 1m 5s trunk passed
          +1 javadoc 0m 25s trunk passed
          +1 mvninstall 0m 34s the patch passed
          +1 compile 0m 34s the patch passed
          +1 javac 0m 34s the patch passed
          -1 checkstyle 0m 24s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 1 new + 267 unchanged - 13 fixed = 268 total (was 280)
          +1 mvnsite 0m 40s the patch passed
          +1 mvneclipse 0m 13s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 12s the patch passed
          +1 javadoc 0m 21s the patch passed
          -1 unit 37m 50s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 asflicense 0m 15s The patch does not generate ASF License warnings.
          55m 27s



          Reason Tests
          Failed junit tests hadoop.yarn.server.resourcemanager.TestRMAdminService
            hadoop.yarn.server.resourcemanager.scheduler.capacity.TestCapacityScheduler



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:85209cc
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12813810/YARN-5047.005.patch
          JIRA Issue YARN-5047
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 3e2eeb561252 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 / 9683eab
          Default Java 1.8.0_91
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12133/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/12133/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/12133/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12133/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/12133/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 36s 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 32s trunk passed +1 compile 0m 37s trunk passed +1 checkstyle 0m 25s trunk passed +1 mvnsite 0m 37s trunk passed +1 mvneclipse 0m 16s trunk passed +1 findbugs 1m 5s trunk passed +1 javadoc 0m 25s trunk passed +1 mvninstall 0m 34s the patch passed +1 compile 0m 34s the patch passed +1 javac 0m 34s the patch passed -1 checkstyle 0m 24s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 1 new + 267 unchanged - 13 fixed = 268 total (was 280) +1 mvnsite 0m 40s the patch passed +1 mvneclipse 0m 13s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 12s the patch passed +1 javadoc 0m 21s the patch passed -1 unit 37m 50s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 55m 27s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.TestRMAdminService   hadoop.yarn.server.resourcemanager.scheduler.capacity.TestCapacityScheduler Subsystem Report/Notes Docker Image:yetus/hadoop:85209cc JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12813810/YARN-5047.005.patch JIRA Issue YARN-5047 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 3e2eeb561252 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 / 9683eab Default Java 1.8.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12133/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/12133/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/12133/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12133/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/12133/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          kasha Karthik Kambatla added a comment -

          Ray Chiang - the latest patch looks good. Is the CS test failure related?

          Show
          kasha Karthik Kambatla added a comment - Ray Chiang - the latest patch looks good. Is the CS test failure related?
          Hide
          rchiang Ray Chiang added a comment -

          RE: No new tests

          Pure code refactoring.

          RE: Checkstyle

          Complaining about no accessors for a protected member variable. Not critical.

          RE: Failing unit tests

          Tests pass in my tree.

          Show
          rchiang Ray Chiang added a comment - RE: No new tests Pure code refactoring. RE: Checkstyle Complaining about no accessors for a protected member variable. Not critical. RE: Failing unit tests Tests pass in my tree.
          Hide
          kasha Karthik Kambatla added a comment -

          Sorry for missing this in my previous review. AbstractYarnScheduler#getNode can return N instead of SchedulerNode. That way, you wouldn't need to typecast in FifoScheduler. Otherwise, the patch looks good to me.

          Wangda Tan - mind taking a look at this, when you get a chance. Would like to get this in soon to unblock further de-dup work.

          Show
          kasha Karthik Kambatla added a comment - Sorry for missing this in my previous review. AbstractYarnScheduler#getNode can return N instead of SchedulerNode . That way, you wouldn't need to typecast in FifoScheduler. Otherwise, the patch looks good to me. Wangda Tan - mind taking a look at this, when you get a chance. Would like to get this in soon to unblock further de-dup work.
          Hide
          leftnoteasy Wangda Tan added a comment -

          Karthik Kambatla, please give me one more day to look at this, please feel free to commit if I cannot review it by tomorrow (Friday).

          Thanks,

          Show
          leftnoteasy Wangda Tan added a comment - Karthik Kambatla , please give me one more day to look at this, please feel free to commit if I cannot review it by tomorrow (Friday). Thanks,
          Hide
          leftnoteasy Wangda Tan added a comment -

          Patch LGTM, +1, Thanks Ray Chiang and reviews from Karthik Kambatla.

          Karthik Kambatla, do you want yourself commit this patch or I?

          Show
          leftnoteasy Wangda Tan added a comment - Patch LGTM, +1, Thanks Ray Chiang and reviews from Karthik Kambatla . Karthik Kambatla , do you want yourself commit this patch or I?
          Hide
          kasha Karthik Kambatla added a comment -

          There is one pending comment that needs addressing. I can +1 it once that is fixed, and Ray should be able to commit. I should also fix whatever is wrong with my permissions.

          Show
          kasha Karthik Kambatla added a comment - There is one pending comment that needs addressing. I can +1 it once that is fixed, and Ray should be able to commit. I should also fix whatever is wrong with my permissions.
          Hide
          rchiang Ray Chiang added a comment -
          • Changed return type of AbstractYarnScheduler#getSchedulerNode() to templated value. Updated unit test to match.
          Show
          rchiang Ray Chiang added a comment - Changed return type of AbstractYarnScheduler#getSchedulerNode() to templated value. Updated unit test to match.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 20s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
          0 mvndep 0m 15s Maven dependency ordering for branch
          +1 mvninstall 7m 5s trunk passed
          +1 compile 7m 44s trunk passed
          +1 checkstyle 1m 42s trunk passed
          +1 mvnsite 1m 15s trunk passed
          +1 mvneclipse 0m 37s trunk passed
          +1 findbugs 1m 39s trunk passed
          +1 javadoc 0m 39s trunk passed
          0 mvndep 0m 15s Maven dependency ordering for patch
          +1 mvninstall 0m 58s the patch passed
          +1 compile 9m 10s the patch passed
          +1 javac 9m 10s the patch passed
          -1 checkstyle 1m 36s root: The patch generated 1 new + 942 unchanged - 5 fixed = 943 total (was 947)
          +1 mvnsite 1m 3s the patch passed
          +1 mvneclipse 0m 33s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 43s the patch passed
          -1 javadoc 0m 22s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 unit 32m 49s hadoop-yarn-server-resourcemanager in the patch passed.
          +1 unit 0m 55s hadoop-sls in the patch passed.
          +1 asflicense 0m 22s The patch does not generate ASF License warnings.
          72m 7s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12818236/YARN-5047.006.patch
          JIRA Issue YARN-5047
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 81bbb1f49c6a 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 / 4421620
          Default Java 1.8.0_91
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12344/artifact/patchprocess/diff-checkstyle-root.txt
          javadoc https://builds.apache.org/job/PreCommit-YARN-Build/12344/artifact/patchprocess/patch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12344/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-tools/hadoop-sls U: .
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/12344/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 20s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files. 0 mvndep 0m 15s Maven dependency ordering for branch +1 mvninstall 7m 5s trunk passed +1 compile 7m 44s trunk passed +1 checkstyle 1m 42s trunk passed +1 mvnsite 1m 15s trunk passed +1 mvneclipse 0m 37s trunk passed +1 findbugs 1m 39s trunk passed +1 javadoc 0m 39s trunk passed 0 mvndep 0m 15s Maven dependency ordering for patch +1 mvninstall 0m 58s the patch passed +1 compile 9m 10s the patch passed +1 javac 9m 10s the patch passed -1 checkstyle 1m 36s root: The patch generated 1 new + 942 unchanged - 5 fixed = 943 total (was 947) +1 mvnsite 1m 3s the patch passed +1 mvneclipse 0m 33s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 43s the patch passed -1 javadoc 0m 22s hadoop-yarn-server-resourcemanager in the patch failed. +1 unit 32m 49s hadoop-yarn-server-resourcemanager in the patch passed. +1 unit 0m 55s hadoop-sls in the patch passed. +1 asflicense 0m 22s The patch does not generate ASF License warnings. 72m 7s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12818236/YARN-5047.006.patch JIRA Issue YARN-5047 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 81bbb1f49c6a 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 / 4421620 Default Java 1.8.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12344/artifact/patchprocess/diff-checkstyle-root.txt javadoc https://builds.apache.org/job/PreCommit-YARN-Build/12344/artifact/patchprocess/patch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12344/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-tools/hadoop-sls U: . Console output https://builds.apache.org/job/PreCommit-YARN-Build/12344/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          rchiang Ray Chiang added a comment -

          RE: checkstyle

          Same issue with protected variable

          RE: Javadoc

          Looks like JDK issues, covered by HADOOP-13369

          Show
          rchiang Ray Chiang added a comment - RE: checkstyle Same issue with protected variable RE: Javadoc Looks like JDK issues, covered by HADOOP-13369
          Hide
          ajisakaa Akira Ajisaka added a comment -

          mvn javadoc:javadoc fails by error.

          [ERROR] /testptch/hadoop/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/AbstractYarnScheduler.java:802: error: unknown tag: returns
          [ERROR] * @returns true if the scheduler is cleared to call assignContainer().
          [ERROR] ^
          

          Would you fix the error by unknown tag @returns?

            /**
             * Method determine whether assignContainers can be called.
             * @returns true if the scheduler is cleared to call assignContainer().
             */
            public boolean isReadyToAssignContainers() {
              // Determine if work-preserving restart recovery time has not yet passed
              return (!rmContext.isWorkPreservingRecoveryEnabled()
                  || rmContext.isSchedulerReadyForAllocatingContainers());
            }
          
          Show
          ajisakaa Akira Ajisaka added a comment - mvn javadoc:javadoc fails by error. [ERROR] /testptch/hadoop/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/AbstractYarnScheduler.java:802: error: unknown tag: returns [ERROR] * @returns true if the scheduler is cleared to call assignContainer(). [ERROR] ^ Would you fix the error by unknown tag @returns ? /** * Method determine whether assignContainers can be called. * @returns true if the scheduler is cleared to call assignContainer(). */ public boolean isReadyToAssignContainers() { // Determine if work-preserving restart recovery time has not yet passed return (!rmContext.isWorkPreservingRecoveryEnabled() || rmContext.isSchedulerReadyForAllocatingContainers()); }
          Hide
          rchiang Ray Chiang added a comment -

          Will do. Thanks Akira Ajisaka.

          Show
          rchiang Ray Chiang added a comment - Will do. Thanks Akira Ajisaka .
          Hide
          rchiang Ray Chiang added a comment -
          • Fix javadoc tag
          Show
          rchiang Ray Chiang added a comment - Fix javadoc tag
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 13m 8s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
          0 mvndep 0m 15s Maven dependency ordering for branch
          +1 mvninstall 8m 3s trunk passed
          +1 compile 7m 41s trunk passed
          +1 checkstyle 1m 36s trunk passed
          +1 mvnsite 1m 2s trunk passed
          +1 mvneclipse 0m 33s trunk passed
          +1 findbugs 1m 27s trunk passed
          +1 javadoc 0m 35s trunk passed
          0 mvndep 0m 14s Maven dependency ordering for patch
          +1 mvninstall 0m 49s the patch passed
          +1 compile 6m 46s the patch passed
          +1 javac 6m 46s the patch passed
          -1 checkstyle 1m 36s root: The patch generated 1 new + 942 unchanged - 5 fixed = 943 total (was 947)
          +1 mvnsite 0m 59s the patch passed
          +1 mvneclipse 0m 33s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 35s the patch passed
          +1 javadoc 0m 34s the patch passed
          +1 unit 33m 27s hadoop-yarn-server-resourcemanager in the patch passed.
          +1 unit 0m 55s hadoop-sls in the patch passed.
          +1 asflicense 0m 21s The patch does not generate ASF License warnings.
          83m 2s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12818613/YARN-5047.007.patch
          JIRA Issue YARN-5047
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 1068b2e6555b 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 / c2bcffb
          Default Java 1.8.0_91
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12357/artifact/patchprocess/diff-checkstyle-root.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12357/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-tools/hadoop-sls U: .
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/12357/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 13m 8s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files. 0 mvndep 0m 15s Maven dependency ordering for branch +1 mvninstall 8m 3s trunk passed +1 compile 7m 41s trunk passed +1 checkstyle 1m 36s trunk passed +1 mvnsite 1m 2s trunk passed +1 mvneclipse 0m 33s trunk passed +1 findbugs 1m 27s trunk passed +1 javadoc 0m 35s trunk passed 0 mvndep 0m 14s Maven dependency ordering for patch +1 mvninstall 0m 49s the patch passed +1 compile 6m 46s the patch passed +1 javac 6m 46s the patch passed -1 checkstyle 1m 36s root: The patch generated 1 new + 942 unchanged - 5 fixed = 943 total (was 947) +1 mvnsite 0m 59s the patch passed +1 mvneclipse 0m 33s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 35s the patch passed +1 javadoc 0m 34s the patch passed +1 unit 33m 27s hadoop-yarn-server-resourcemanager in the patch passed. +1 unit 0m 55s hadoop-sls in the patch passed. +1 asflicense 0m 21s The patch does not generate ASF License warnings. 83m 2s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12818613/YARN-5047.007.patch JIRA Issue YARN-5047 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 1068b2e6555b 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 / c2bcffb Default Java 1.8.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12357/artifact/patchprocess/diff-checkstyle-root.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12357/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-tools/hadoop-sls U: . Console output https://builds.apache.org/job/PreCommit-YARN-Build/12357/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          rchiang Ray Chiang added a comment -

          RE: checkstyle

          Same issue as before (protected variable being used by subclasses).

          Show
          rchiang Ray Chiang added a comment - RE: checkstyle Same issue as before (protected variable being used by subclasses).
          Hide
          rchiang Ray Chiang added a comment -

          Not sure what happened, but versions 006 and 007 of the patch ended up confused with another JIRA I was working on.

          Show
          rchiang Ray Chiang added a comment - Not sure what happened, but versions 006 and 007 of the patch ended up confused with another JIRA I was working on.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 24s 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 55s trunk passed
          +1 compile 0m 34s trunk passed
          +1 checkstyle 0m 29s trunk passed
          +1 mvnsite 0m 37s trunk passed
          +1 mvneclipse 0m 17s trunk passed
          +1 findbugs 0m 56s trunk passed
          +1 javadoc 0m 20s trunk passed
          +1 mvninstall 0m 31s the patch passed
          +1 compile 0m 29s the patch passed
          +1 javac 0m 29s the patch passed
          -1 checkstyle 0m 27s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 1 new + 676 unchanged - 13 fixed = 677 total (was 689)
          +1 mvnsite 0m 34s the patch passed
          +1 mvneclipse 0m 14s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 0s the patch passed
          +1 javadoc 0m 18s the patch passed
          +1 unit 37m 55s hadoop-yarn-server-resourcemanager in the patch passed.
          +1 asflicense 0m 16s The patch does not generate ASF License warnings.
          52m 53s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12819544/YARN-5047.008.patch
          JIRA Issue YARN-5047
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 0bd304b856d4 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 / 132deb4
          Default Java 1.8.0_91
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12458/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12458/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/12458/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 24s 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 55s trunk passed +1 compile 0m 34s trunk passed +1 checkstyle 0m 29s trunk passed +1 mvnsite 0m 37s trunk passed +1 mvneclipse 0m 17s trunk passed +1 findbugs 0m 56s trunk passed +1 javadoc 0m 20s trunk passed +1 mvninstall 0m 31s the patch passed +1 compile 0m 29s the patch passed +1 javac 0m 29s the patch passed -1 checkstyle 0m 27s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 1 new + 676 unchanged - 13 fixed = 677 total (was 689) +1 mvnsite 0m 34s the patch passed +1 mvneclipse 0m 14s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 0s the patch passed +1 javadoc 0m 18s the patch passed +1 unit 37m 55s hadoop-yarn-server-resourcemanager in the patch passed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 52m 53s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12819544/YARN-5047.008.patch JIRA Issue YARN-5047 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 0bd304b856d4 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 / 132deb4 Default Java 1.8.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12458/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12458/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/12458/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          rchiang Ray Chiang added a comment -

          RE: checkstyle

          Same protected variable warning as before.

          Show
          rchiang Ray Chiang added a comment - RE: checkstyle Same protected variable warning as before.
          Hide
          templedf Daniel Templeton added a comment -

          Looks like the current patch needs to be rebased. I see merge errors on AbstractYarnScheduler, CapacityScheduler, and the test class.

          Show
          templedf Daniel Templeton added a comment - Looks like the current patch needs to be rebased. I see merge errors on AbstractYarnScheduler , CapacityScheduler , and the test class.
          Hide
          rchiang Ray Chiang added a comment -
          Show
          rchiang Ray Chiang added a comment - Rebased after YARN-4091 and YARN-5495 merges.
          Hide
          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 7m 10s trunk passed
          +1 compile 0m 33s trunk passed
          +1 checkstyle 0m 29s trunk passed
          +1 mvnsite 0m 43s trunk passed
          +1 mvneclipse 0m 17s trunk passed
          +1 findbugs 0m 59s trunk passed
          +1 javadoc 0m 23s trunk passed
          +1 mvninstall 0m 37s the patch passed
          +1 compile 0m 32s the patch passed
          +1 javac 0m 32s the patch passed
          -1 checkstyle 0m 27s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 20 new + 664 unchanged - 13 fixed = 684 total (was 677)
          +1 mvnsite 0m 36s the patch passed
          +1 mvneclipse 0m 15s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 10s the patch passed
          +1 javadoc 0m 19s the patch passed
          +1 unit 37m 47s hadoop-yarn-server-resourcemanager in the patch passed.
          +1 asflicense 0m 15s The patch does not generate ASF License warnings.
          53m 28s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12823126/YARN-5047.009.patch
          JIRA Issue YARN-5047
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux ef4ae99e3f45 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 / 89c0bff
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12729/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12729/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/12729/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 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 7m 10s trunk passed +1 compile 0m 33s trunk passed +1 checkstyle 0m 29s trunk passed +1 mvnsite 0m 43s trunk passed +1 mvneclipse 0m 17s trunk passed +1 findbugs 0m 59s trunk passed +1 javadoc 0m 23s trunk passed +1 mvninstall 0m 37s the patch passed +1 compile 0m 32s the patch passed +1 javac 0m 32s the patch passed -1 checkstyle 0m 27s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 20 new + 664 unchanged - 13 fixed = 684 total (was 677) +1 mvnsite 0m 36s the patch passed +1 mvneclipse 0m 15s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 10s the patch passed +1 javadoc 0m 19s the patch passed +1 unit 37m 47s hadoop-yarn-server-resourcemanager in the patch passed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 53m 28s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12823126/YARN-5047.009.patch JIRA Issue YARN-5047 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux ef4ae99e3f45 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 / 89c0bff Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12729/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12729/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/12729/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          rchiang Ray Chiang added a comment -
          • Removed unused imports in CapacityScheduler
          Show
          rchiang Ray Chiang added a comment - Removed unused imports in CapacityScheduler
          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 40s trunk passed
          +1 compile 0m 31s trunk passed
          +1 checkstyle 0m 29s trunk passed
          +1 mvnsite 0m 37s trunk passed
          +1 mvneclipse 0m 17s trunk passed
          +1 findbugs 0m 55s trunk passed
          +1 javadoc 0m 21s trunk passed
          +1 mvninstall 0m 33s the patch passed
          +1 compile 0m 31s the patch passed
          +1 javac 0m 31s the patch passed
          -1 checkstyle 0m 26s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 2 new + 664 unchanged - 13 fixed = 666 total (was 677)
          +1 mvnsite 0m 38s the patch passed
          +1 mvneclipse 0m 16s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 17s the patch passed
          +1 javadoc 0m 21s the patch passed
          -1 unit 39m 39s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 asflicense 0m 15s The patch does not generate ASF License warnings.
          54m 50s



          Reason Tests
          Failed junit tests hadoop.yarn.server.resourcemanager.applicationsmanager.TestAMRestart



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12823777/YARN-5047.010.patch
          JIRA Issue YARN-5047
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux fa5c31d3e5dc 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 / 864f878
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12778/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/12778/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/12778/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12778/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/12778/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 40s trunk passed +1 compile 0m 31s trunk passed +1 checkstyle 0m 29s trunk passed +1 mvnsite 0m 37s trunk passed +1 mvneclipse 0m 17s trunk passed +1 findbugs 0m 55s trunk passed +1 javadoc 0m 21s trunk passed +1 mvninstall 0m 33s the patch passed +1 compile 0m 31s the patch passed +1 javac 0m 31s the patch passed -1 checkstyle 0m 26s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 2 new + 664 unchanged - 13 fixed = 666 total (was 677) +1 mvnsite 0m 38s the patch passed +1 mvneclipse 0m 16s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 17s the patch passed +1 javadoc 0m 21s the patch passed -1 unit 39m 39s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 54m 50s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.applicationsmanager.TestAMRestart Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12823777/YARN-5047.010.patch JIRA Issue YARN-5047 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux fa5c31d3e5dc 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 / 864f878 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12778/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/12778/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/12778/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12778/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/12778/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          templedf Daniel Templeton added a comment -

          Thanks for the patch, Ray Chiang! A few comments:

          • The AbstractYarnScheduler.nodeUpdate() method is pretty rambly. Think you could refactor it into a couple of more bite-sized methods?
          • In this code:
                  Resource rs = container.getAllocatedResource();
                  if (rs != null) {
                    Resources.addTo(releasedResources, rs);
                  }
                  rs = container.getReservedResource();
                  if (rs != null) {
                    Resources.addTo(releasedResources, rs);
                  }
          

          I'd rather not reuse the rs variable.

          • For this comment:
              // TODO: Fix possible race-condition when request comes in before
              // update is propagated
          

          is there a JIRA filed? If so, can you include it in the comment? (I realize that came from the CS code, but it would be nice to clean it up.)

          • In this code:
              if (rmContext.isWorkPreservingRecoveryEnabled()
                  && !rmContext.isSchedulerReadyForAllocatingContainers()) {
                return;
              }
              if (Resources.greaterThanOrEqual(resourceCalculator, getClusterResource(),
                  node.getUnallocatedResource(), minimumAllocation)) {
          ...
              }
          

          I'd much rather you add the first if's conditions to the second if, rather than use the if-return.

          Show
          templedf Daniel Templeton added a comment - Thanks for the patch, Ray Chiang ! A few comments: The AbstractYarnScheduler.nodeUpdate() method is pretty rambly. Think you could refactor it into a couple of more bite-sized methods? In this code: Resource rs = container.getAllocatedResource(); if (rs != null ) { Resources.addTo(releasedResources, rs); } rs = container.getReservedResource(); if (rs != null ) { Resources.addTo(releasedResources, rs); } I'd rather not reuse the rs variable. For this comment: // TODO: Fix possible race-condition when request comes in before // update is propagated is there a JIRA filed? If so, can you include it in the comment? (I realize that came from the CS code, but it would be nice to clean it up.) In this code: if (rmContext.isWorkPreservingRecoveryEnabled() && !rmContext.isSchedulerReadyForAllocatingContainers()) { return ; } if (Resources.greaterThanOrEqual(resourceCalculator, getClusterResource(), node.getUnallocatedResource(), minimumAllocation)) { ... } I'd much rather you add the first if's conditions to the second if, rather than use the if-return.
          Hide
          kasha Karthik Kambatla added a comment -

          Canceling patch to address review comments.

          Show
          kasha Karthik Kambatla added a comment - Canceling patch to address review comments.
          Hide
          rchiang Ray Chiang added a comment -
          • Split nodeUpdate() into submethods
          • Fixed rs variable
          • Added JIRA to TODO comment
          • Can't fix last comment--would require duplicating the logic of the if statement or something much messier.
          Show
          rchiang Ray Chiang added a comment - Split nodeUpdate() into submethods Fixed rs variable Added JIRA to TODO comment Can't fix last comment--would require duplicating the logic of the if statement or something much messier.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 0s Docker mode activated.
          -1 patch 0m 8s YARN-5047 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help.



          Subsystem Report/Notes
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12831764/YARN-5047.011.patch
          JIRA Issue YARN-5047
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/13296/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 0s Docker mode activated. -1 patch 0m 8s YARN-5047 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help. Subsystem Report/Notes JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12831764/YARN-5047.011.patch JIRA Issue YARN-5047 Console output https://builds.apache.org/job/PreCommit-YARN-Build/13296/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          rchiang Ray Chiang added a comment -

          Lots of updating needed after YARN-3139.

          Show
          rchiang Ray Chiang added a comment - Lots of updating needed after YARN-3139 .
          Hide
          templedf Daniel Templeton added a comment -

          Latest patch looks great. Thanks for doing that refactor. The code is infinitely more readable now. +1 (non-binding)

          Show
          templedf Daniel Templeton added a comment - Latest patch looks great. Thanks for doing that refactor. The code is infinitely more readable now. +1 (non-binding)
          Hide
          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 7m 12s trunk passed
          +1 compile 0m 32s trunk passed
          +1 checkstyle 0m 28s trunk passed
          +1 mvnsite 0m 39s trunk passed
          +1 mvneclipse 0m 16s trunk passed
          +1 findbugs 1m 3s trunk passed
          +1 javadoc 0m 20s trunk passed
          +1 mvninstall 0m 32s the patch passed
          +1 compile 0m 30s the patch passed
          +1 javac 0m 30s the patch passed
          -1 checkstyle 0m 25s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 2 new + 622 unchanged - 5 fixed = 624 total (was 627)
          +1 mvnsite 0m 37s the patch passed
          +1 mvneclipse 0m 13s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 4s the patch passed
          +1 javadoc 0m 17s the patch passed
          -1 unit 35m 5s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 asflicense 0m 14s The patch does not generate ASF License warnings.
          50m 25s



          Reason Tests
          Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.fair.TestContinuousScheduling



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12832210/YARN-5047.012.patch
          JIRA Issue YARN-5047
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux f119cf665123 3.13.0-93-generic #140-Ubuntu SMP Mon Jul 18 21:21:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / d7d87de
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13463/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/13463/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/13463/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13463/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/13463/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 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 7m 12s trunk passed +1 compile 0m 32s trunk passed +1 checkstyle 0m 28s trunk passed +1 mvnsite 0m 39s trunk passed +1 mvneclipse 0m 16s trunk passed +1 findbugs 1m 3s trunk passed +1 javadoc 0m 20s trunk passed +1 mvninstall 0m 32s the patch passed +1 compile 0m 30s the patch passed +1 javac 0m 30s the patch passed -1 checkstyle 0m 25s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 2 new + 622 unchanged - 5 fixed = 624 total (was 627) +1 mvnsite 0m 37s the patch passed +1 mvneclipse 0m 13s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 4s the patch passed +1 javadoc 0m 17s the patch passed -1 unit 35m 5s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 14s The patch does not generate ASF License warnings. 50m 25s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.fair.TestContinuousScheduling Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12832210/YARN-5047.012.patch JIRA Issue YARN-5047 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux f119cf665123 3.13.0-93-generic #140-Ubuntu SMP Mon Jul 18 21:21:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / d7d87de Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13463/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/13463/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/13463/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13463/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/13463/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 22s 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 8m 54s trunk passed
          +1 compile 0m 43s trunk passed
          +1 checkstyle 0m 34s trunk passed
          +1 mvnsite 0m 47s trunk passed
          +1 mvneclipse 0m 20s trunk passed
          +1 findbugs 1m 11s trunk passed
          +1 javadoc 0m 28s trunk passed
          +1 mvninstall 0m 42s the patch passed
          +1 compile 0m 39s the patch passed
          +1 javac 0m 39s the patch passed
          -1 checkstyle 0m 31s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 2 new + 622 unchanged - 5 fixed = 624 total (was 627)
          +1 mvnsite 0m 46s the patch passed
          +1 mvneclipse 0m 19s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 20s the patch passed
          +1 javadoc 0m 24s the patch passed
          -1 unit 39m 16s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 asflicense 0m 15s The patch does not generate ASF License warnings.
          58m 21s



          Reason Tests
          Failed junit tests hadoop.yarn.server.resourcemanager.TestRMAdminService



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12832210/YARN-5047.012.patch
          JIRA Issue YARN-5047
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux e46ce9b94fe2 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / d7d87de
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13462/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/13462/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/13462/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13462/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/13462/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 22s 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 8m 54s trunk passed +1 compile 0m 43s trunk passed +1 checkstyle 0m 34s trunk passed +1 mvnsite 0m 47s trunk passed +1 mvneclipse 0m 20s trunk passed +1 findbugs 1m 11s trunk passed +1 javadoc 0m 28s trunk passed +1 mvninstall 0m 42s the patch passed +1 compile 0m 39s the patch passed +1 javac 0m 39s the patch passed -1 checkstyle 0m 31s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 2 new + 622 unchanged - 5 fixed = 624 total (was 627) +1 mvnsite 0m 46s the patch passed +1 mvneclipse 0m 19s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 20s the patch passed +1 javadoc 0m 24s the patch passed -1 unit 39m 16s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 58m 21s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.TestRMAdminService Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12832210/YARN-5047.012.patch JIRA Issue YARN-5047 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux e46ce9b94fe2 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / d7d87de Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13462/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/13462/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/13462/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13462/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/13462/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          kasha Karthik Kambatla added a comment -

          The checkstyle issues reported are benign, and test failures are flaky. The last two builds have two different test failures.

          +1. Checking this in..

          Show
          kasha Karthik Kambatla added a comment - The checkstyle issues reported are benign, and test failures are flaky. The last two builds have two different test failures. +1. Checking this in..
          Hide
          kasha Karthik Kambatla added a comment -

          Thanks for working on this much-need refactoring, Ray. Thanks Wangda and Daniel for your reviews.

          Just committed this to trunk and branch-2.

          Show
          kasha Karthik Kambatla added a comment - Thanks for working on this much-need refactoring, Ray. Thanks Wangda and Daniel for your reviews. Just committed this to trunk and branch-2.
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10652 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10652/)
          YARN-5047. Refactor nodeUpdate across schedulers. (Ray Chiang via kasha) (kasha: rev 754cb4e30fac1c5fe8d44626968c0ddbfe459335)

          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/FairScheduler.java
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/monitor/capacity/TestProportionalCapacityPreemptionPolicy.java
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/AbstractYarnScheduler.java
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/CapacityScheduler.java
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fifo/FifoScheduler.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10652 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10652/ ) YARN-5047 . Refactor nodeUpdate across schedulers. (Ray Chiang via kasha) (kasha: rev 754cb4e30fac1c5fe8d44626968c0ddbfe459335) (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/FairScheduler.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/monitor/capacity/TestProportionalCapacityPreemptionPolicy.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/AbstractYarnScheduler.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/CapacityScheduler.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fifo/FifoScheduler.java

            People

            • Assignee:
              rchiang Ray Chiang
              Reporter:
              rchiang Ray Chiang
            • Votes:
              0 Vote for this issue
              Watchers:
              14 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development