Details

    • Hadoop Flags:
      Reviewed

      Description

      This JIRA tracks policies in the AMRMProxy that decide how to forward ResourceRequests, without maintaining substantial state across decissions (e.g., broadcast).

      1. YARN-5325.01.patch
        48 kB
        Carlo Curino
      2. YARN-5325.02.patch
        51 kB
        Carlo Curino
      3. YARN-5325.03.patch
        50 kB
        Carlo Curino
      4. YARN-5325.04.patch
        52 kB
        Carlo Curino
      5. YARN-5325-YARN-2915.05.patch
        52 kB
        Carlo Curino
      6. YARN-5325-YARN-2915.06.patch
        83 kB
        Carlo Curino
      7. YARN-5325-YARN-2915.07.patch
        83 kB
        Carlo Curino
      8. YARN-5325-YARN-2915.08.patch
        90 kB
        Carlo Curino
      9. YARN-5325-YARN-2915.09.patch
        90 kB
        Carlo Curino
      10. YARN-5325-YARN-2915.10.patch
        91 kB
        Carlo Curino
      11. YARN-5325-YARN-2915.11.patch
        92 kB
        Carlo Curino
      12. YARN-5325-YARN-2915.12.patch
        94 kB
        Carlo Curino
      13. YARN-5325-YARN-2915.13.patch
        141 kB
        Subru Krishnan
      14. YARN-5325-YARN-2915.14.patch
        141 kB
        Subru Krishnan

        Issue Links

          Activity

          Hide
          curino Carlo Curino added a comment -

          The first patch provides 2 implementations for the ARMRMProxyFederationPolicy:

          1. a simple broadcast policy that replicate the user asks to all sub-clusters
          2. a sophisticated policy (LocalityMulticastAMRMProxyFederationPolicy that:
            1. Directs node are rack local asks to sub-clusters that own the node and the rack. (in some deployments we stripe across racks, so the racks are not resolved to one subcluster and are thus treated as an ANY)
            2. Directs ANY that correspond to node/rack local requests to the corresponding sub-clusters by "splitting" the ANY according to how many containers are requested (with locality) to each sub-clusters.
            3. Directs ANY that are not associated with a localized request to subclusters according to a linear combination of policy weights and headroom information. An alpha parameter is used to choose how much headroom plays a role here.
            4. Directs ANY requesting zero containers (user canceling their asks) to all sub-clusters we interacted with so far.
            5. This policy minimizes the work done by RMs by ensuring that the sum of the request we send out is close to the user ask (might exceed by at most |sub-clusters| due to rounding to the closest integral number of containers).

          The tests verify much of this behavior (though more should be done in testing).

          Per discussions with Subru Krishnan we should probably work on optimizing the policy by factoring out sum-of-weight recomputations, etc...

          Show
          curino Carlo Curino added a comment - The first patch provides 2 implementations for the ARMRMProxyFederationPolicy : a simple broadcast policy that replicate the user asks to all sub-clusters a sophisticated policy ( LocalityMulticastAMRMProxyFederationPolicy that: Directs node are rack local asks to sub-clusters that own the node and the rack. (in some deployments we stripe across racks, so the racks are not resolved to one subcluster and are thus treated as an ANY) Directs ANY that correspond to node/rack local requests to the corresponding sub-clusters by "splitting" the ANY according to how many containers are requested (with locality) to each sub-clusters. Directs ANY that are not associated with a localized request to subclusters according to a linear combination of policy weights and headroom information. An alpha parameter is used to choose how much headroom plays a role here. Directs ANY requesting zero containers (user canceling their asks) to all sub-clusters we interacted with so far. This policy minimizes the work done by RMs by ensuring that the sum of the request we send out is close to the user ask (might exceed by at most |sub-clusters| due to rounding to the closest integral number of containers). The tests verify much of this behavior (though more should be done in testing). Per discussions with Subru Krishnan we should probably work on optimizing the policy by factoring out sum-of-weight recomputations, etc...
          Hide
          curino Carlo Curino added a comment -

          Updated based on YARN-5390. Minor changes in the subcluster resolver APIs made this a little ugglier (catch exception, where a null was enough, and untype YarnException, based on discussion we might want to evolve subcluster resolver APIs)

          Show
          curino Carlo Curino added a comment - Updated based on YARN-5390 . Minor changes in the subcluster resolver APIs made this a little ugglier (catch exception, where a null was enough, and untype YarnException, based on discussion we might want to evolve subcluster resolver APIs)
          Hide
          curino Carlo Curino added a comment -

          Attaching simple protected->public change in subcluster resolver and changes resource file (to allow for tests of the AMRMProxy policies)

          Show
          curino Carlo Curino added a comment - Attaching simple protected->public change in subcluster resolver and changes resource file (to allow for tests of the AMRMProxy policies)
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 0s Docker mode activated.
          -1 patch 0m 13s YARN-5325 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/12823155/YARN-5325.04.patch
          JIRA Issue YARN-5325
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/12731/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 13s YARN-5325 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/12823155/YARN-5325.04.patch JIRA Issue YARN-5325 Console output https://builds.apache.org/job/PreCommit-YARN-Build/12731/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          curino Carlo Curino added a comment -

          (renaming to target branch for Jenkins)

          Show
          curino Carlo Curino added a comment - (renaming to target branch for Jenkins)
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 16s Docker mode activated.
          0 shelldocs 0m 0s Shelldocs was not available.
          +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.
          +1 mvninstall 7m 57s YARN-2915 passed
          +1 compile 0m 24s YARN-2915 passed
          +1 checkstyle 0m 15s YARN-2915 passed
          +1 mvnsite 0m 28s YARN-2915 passed
          +1 mvneclipse 0m 15s YARN-2915 passed
          +1 findbugs 0m 48s YARN-2915 passed
          +1 javadoc 0m 19s YARN-2915 passed
          -1 mvninstall 0m 16s hadoop-yarn-server-common in the patch failed.
          -1 compile 0m 16s hadoop-yarn-server-common in the patch failed.
          -1 javac 0m 16s hadoop-yarn-server-common in the patch failed.
          -1 checkstyle 0m 12s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common: The patch generated 38 new + 0 unchanged - 0 fixed = 38 total (was 0)
          -1 mvnsite 0m 17s hadoop-yarn-server-common in the patch failed.
          +1 mvneclipse 0m 11s the patch passed
          +1 shellcheck 0m 14s The patch generated 0 new + 74 unchanged - 1 fixed = 74 total (was 75)
          +1 whitespace 0m 0s The patch has no whitespace issues.
          -1 findbugs 0m 16s hadoop-yarn-server-common in the patch failed.
          -1 javadoc 0m 15s hadoop-yarn-server-common in the patch failed.
          -1 unit 0m 16s hadoop-yarn-server-common in the patch failed.
          +1 asflicense 0m 17s The patch does not generate ASF License warnings.
          13m 56s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12823158/YARN-5325-YARN-2915.05.patch
          JIRA Issue YARN-5325
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle shellcheck shelldocs
          uname Linux 6c666ec52622 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 YARN-2915 / b689f55
          Default Java 1.8.0_101
          shellcheck v0.4.4
          findbugs v3.0.0
          mvninstall https://builds.apache.org/job/PreCommit-YARN-Build/12735/artifact/patchprocess/patch-mvninstall-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.txt
          compile https://builds.apache.org/job/PreCommit-YARN-Build/12735/artifact/patchprocess/patch-compile-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.txt
          javac https://builds.apache.org/job/PreCommit-YARN-Build/12735/artifact/patchprocess/patch-compile-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.txt
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12735/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.txt
          mvnsite https://builds.apache.org/job/PreCommit-YARN-Build/12735/artifact/patchprocess/patch-mvnsite-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.txt
          findbugs https://builds.apache.org/job/PreCommit-YARN-Build/12735/artifact/patchprocess/patch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.txt
          javadoc https://builds.apache.org/job/PreCommit-YARN-Build/12735/artifact/patchprocess/patch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/12735/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12735/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/12735/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 16s Docker mode activated. 0 shelldocs 0m 0s Shelldocs was not available. +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. +1 mvninstall 7m 57s YARN-2915 passed +1 compile 0m 24s YARN-2915 passed +1 checkstyle 0m 15s YARN-2915 passed +1 mvnsite 0m 28s YARN-2915 passed +1 mvneclipse 0m 15s YARN-2915 passed +1 findbugs 0m 48s YARN-2915 passed +1 javadoc 0m 19s YARN-2915 passed -1 mvninstall 0m 16s hadoop-yarn-server-common in the patch failed. -1 compile 0m 16s hadoop-yarn-server-common in the patch failed. -1 javac 0m 16s hadoop-yarn-server-common in the patch failed. -1 checkstyle 0m 12s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common: The patch generated 38 new + 0 unchanged - 0 fixed = 38 total (was 0) -1 mvnsite 0m 17s hadoop-yarn-server-common in the patch failed. +1 mvneclipse 0m 11s the patch passed +1 shellcheck 0m 14s The patch generated 0 new + 74 unchanged - 1 fixed = 74 total (was 75) +1 whitespace 0m 0s The patch has no whitespace issues. -1 findbugs 0m 16s hadoop-yarn-server-common in the patch failed. -1 javadoc 0m 15s hadoop-yarn-server-common in the patch failed. -1 unit 0m 16s hadoop-yarn-server-common in the patch failed. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 13m 56s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12823158/YARN-5325-YARN-2915.05.patch JIRA Issue YARN-5325 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle shellcheck shelldocs uname Linux 6c666ec52622 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 YARN-2915 / b689f55 Default Java 1.8.0_101 shellcheck v0.4.4 findbugs v3.0.0 mvninstall https://builds.apache.org/job/PreCommit-YARN-Build/12735/artifact/patchprocess/patch-mvninstall-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.txt compile https://builds.apache.org/job/PreCommit-YARN-Build/12735/artifact/patchprocess/patch-compile-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.txt javac https://builds.apache.org/job/PreCommit-YARN-Build/12735/artifact/patchprocess/patch-compile-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.txt checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12735/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.txt mvnsite https://builds.apache.org/job/PreCommit-YARN-Build/12735/artifact/patchprocess/patch-mvnsite-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.txt findbugs https://builds.apache.org/job/PreCommit-YARN-Build/12735/artifact/patchprocess/patch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.txt javadoc https://builds.apache.org/job/PreCommit-YARN-Build/12735/artifact/patchprocess/patch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/12735/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12735/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common Console output https://builds.apache.org/job/PreCommit-YARN-Build/12735/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          curino Carlo Curino added a comment -

          Much of the errors are because this depends on YARN-5323 (compiles and run when all combined).

          Show
          curino Carlo Curino added a comment - Much of the errors are because this depends on YARN-5323 (compiles and run when all combined).
          Hide
          curino Carlo Curino added a comment -

          The latest patch has been rebased on YARN-5323/YARN-5324. And contains the following:
          1) Refactoring of the BaseWeighterRouterPolicy to be usable also as a base for {{FederationAMRMProxyPolicy}}s.
          2) A small fix of visibility for the resolver.
          3) Two policies BroadcastAMRMProxyPolicy and LocalityMulticastAMRMProxyPolicy.

          Show
          curino Carlo Curino added a comment - The latest patch has been rebased on YARN-5323 / YARN-5324 . And contains the following: 1) Refactoring of the BaseWeighterRouterPolicy to be usable also as a base for {{FederationAMRMProxyPolicy}}s. 2) A small fix of visibility for the resolver. 3) Two policies BroadcastAMRMProxyPolicy and LocalityMulticastAMRMProxyPolicy .
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 12m 48s Docker mode activated.
          0 shelldocs 0m 0s Shelldocs was not available.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 5 new or modified test files.
          +1 mvninstall 7m 40s YARN-2915 passed
          +1 compile 0m 25s YARN-2915 passed
          +1 checkstyle 0m 16s YARN-2915 passed
          +1 mvnsite 0m 27s YARN-2915 passed
          +1 mvneclipse 0m 16s YARN-2915 passed
          +1 findbugs 0m 50s YARN-2915 passed
          +1 javadoc 0m 20s YARN-2915 passed
          +1 mvninstall 0m 21s the patch passed
          +1 compile 0m 22s the patch passed
          +1 javac 0m 22s the patch passed
          -1 checkstyle 0m 11s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common: The patch generated 25 new + 0 unchanged - 0 fixed = 25 total (was 0)
          +1 mvnsite 0m 23s the patch passed
          +1 mvneclipse 0m 10s the patch passed
          +1 shellcheck 0m 12s The patch generated 0 new + 74 unchanged - 1 fixed = 74 total (was 75)
          +1 whitespace 0m 0s The patch has no whitespace issues.
          -1 findbugs 0m 55s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common generated 3 new + 0 unchanged - 0 fixed = 3 total (was 0)
          -1 javadoc 0m 16s hadoop-yarn-server-common in the patch failed.
          +1 unit 0m 51s hadoop-yarn-server-common in the patch passed.
          +1 asflicense 0m 17s The patch does not generate ASF License warnings.
          27m 43s



          Reason Tests
          FindBugs module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common
            Unused field:BroadcastAMRMProxyPolicy.java
            Dead store to tempPolicy in org.apache.hadoop.yarn.server.federation.policies.amrmproxy.LocalityMulticastAMRMProxyPolicy.reinitialize(FederationPolicyInitializationContext) At LocalityMulticastAMRMProxyPolicy.java:org.apache.hadoop.yarn.server.federation.policies.amrmproxy.LocalityMulticastAMRMProxyPolicy.reinitialize(FederationPolicyInitializationContext) At LocalityMulticastAMRMProxyPolicy.java:[line 107]
            Redundant nullcheck of targetIds which is known to be null in org.apache.hadoop.yarn.server.federation.policies.amrmproxy.LocalityMulticastAMRMProxyPolicy.splitResourceRequests(List) Redundant null check at LocalityMulticastAMRMProxyPolicy.java:is known to be null in org.apache.hadoop.yarn.server.federation.policies.amrmproxy.LocalityMulticastAMRMProxyPolicy.splitResourceRequests(List) Redundant null check at LocalityMulticastAMRMProxyPolicy.java:[line 190]



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12830390/YARN-5325-YARN-2915.06.patch
          JIRA Issue YARN-5325
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle shellcheck shelldocs
          uname Linux 5ee2da9b876d 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 YARN-2915 / 6f91338
          Default Java 1.8.0_101
          shellcheck v0.4.4
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13220/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.txt
          findbugs https://builds.apache.org/job/PreCommit-YARN-Build/13220/artifact/patchprocess/new-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.html
          javadoc https://builds.apache.org/job/PreCommit-YARN-Build/13220/artifact/patchprocess/patch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13220/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/13220/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 12m 48s Docker mode activated. 0 shelldocs 0m 0s Shelldocs was not available. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 5 new or modified test files. +1 mvninstall 7m 40s YARN-2915 passed +1 compile 0m 25s YARN-2915 passed +1 checkstyle 0m 16s YARN-2915 passed +1 mvnsite 0m 27s YARN-2915 passed +1 mvneclipse 0m 16s YARN-2915 passed +1 findbugs 0m 50s YARN-2915 passed +1 javadoc 0m 20s YARN-2915 passed +1 mvninstall 0m 21s the patch passed +1 compile 0m 22s the patch passed +1 javac 0m 22s the patch passed -1 checkstyle 0m 11s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common: The patch generated 25 new + 0 unchanged - 0 fixed = 25 total (was 0) +1 mvnsite 0m 23s the patch passed +1 mvneclipse 0m 10s the patch passed +1 shellcheck 0m 12s The patch generated 0 new + 74 unchanged - 1 fixed = 74 total (was 75) +1 whitespace 0m 0s The patch has no whitespace issues. -1 findbugs 0m 55s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common generated 3 new + 0 unchanged - 0 fixed = 3 total (was 0) -1 javadoc 0m 16s hadoop-yarn-server-common in the patch failed. +1 unit 0m 51s hadoop-yarn-server-common in the patch passed. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 27m 43s Reason Tests FindBugs module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common   Unused field:BroadcastAMRMProxyPolicy.java   Dead store to tempPolicy in org.apache.hadoop.yarn.server.federation.policies.amrmproxy.LocalityMulticastAMRMProxyPolicy.reinitialize(FederationPolicyInitializationContext) At LocalityMulticastAMRMProxyPolicy.java:org.apache.hadoop.yarn.server.federation.policies.amrmproxy.LocalityMulticastAMRMProxyPolicy.reinitialize(FederationPolicyInitializationContext) At LocalityMulticastAMRMProxyPolicy.java: [line 107]   Redundant nullcheck of targetIds which is known to be null in org.apache.hadoop.yarn.server.federation.policies.amrmproxy.LocalityMulticastAMRMProxyPolicy.splitResourceRequests(List) Redundant null check at LocalityMulticastAMRMProxyPolicy.java:is known to be null in org.apache.hadoop.yarn.server.federation.policies.amrmproxy.LocalityMulticastAMRMProxyPolicy.splitResourceRequests(List) Redundant null check at LocalityMulticastAMRMProxyPolicy.java: [line 190] Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12830390/YARN-5325-YARN-2915.06.patch JIRA Issue YARN-5325 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle shellcheck shelldocs uname Linux 5ee2da9b876d 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 YARN-2915 / 6f91338 Default Java 1.8.0_101 shellcheck v0.4.4 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13220/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.txt findbugs https://builds.apache.org/job/PreCommit-YARN-Build/13220/artifact/patchprocess/new-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.html javadoc https://builds.apache.org/job/PreCommit-YARN-Build/13220/artifact/patchprocess/patch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13220/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common Console output https://builds.apache.org/job/PreCommit-YARN-Build/13220/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 16s Docker mode activated.
          0 shelldocs 0m 0s Shelldocs was not available.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 5 new or modified test files.
          +1 mvninstall 9m 4s YARN-2915 passed
          +1 compile 0m 28s YARN-2915 passed
          +1 checkstyle 0m 19s YARN-2915 passed
          +1 mvnsite 0m 29s YARN-2915 passed
          +1 mvneclipse 0m 17s YARN-2915 passed
          +1 findbugs 0m 58s YARN-2915 passed
          +1 javadoc 0m 20s YARN-2915 passed
          +1 mvninstall 0m 24s the patch passed
          +1 compile 0m 20s the patch passed
          +1 javac 0m 20s the patch passed
          -1 checkstyle 0m 14s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common: The patch generated 5 new + 0 unchanged - 0 fixed = 5 total (was 0)
          +1 mvnsite 0m 26s the patch passed
          +1 mvneclipse 0m 13s the patch passed
          +1 shellcheck 0m 13s The patch generated 0 new + 74 unchanged - 1 fixed = 74 total (was 75)
          +1 whitespace 0m 1s The patch has no whitespace issues.
          +1 findbugs 0m 59s the patch passed
          +1 javadoc 0m 17s the patch passed
          +1 unit 0m 59s hadoop-yarn-server-common in the patch passed.
          +1 asflicense 0m 21s The patch does not generate ASF License warnings.
          17m 28s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12831016/YARN-5325-YARN-2915.07.patch
          JIRA Issue YARN-5325
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle shellcheck shelldocs
          uname Linux 977462d48cb3 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 YARN-2915 / 6f91338
          Default Java 1.8.0_101
          shellcheck v0.4.4
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13251/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13251/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/13251/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 16s Docker mode activated. 0 shelldocs 0m 0s Shelldocs was not available. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 5 new or modified test files. +1 mvninstall 9m 4s YARN-2915 passed +1 compile 0m 28s YARN-2915 passed +1 checkstyle 0m 19s YARN-2915 passed +1 mvnsite 0m 29s YARN-2915 passed +1 mvneclipse 0m 17s YARN-2915 passed +1 findbugs 0m 58s YARN-2915 passed +1 javadoc 0m 20s YARN-2915 passed +1 mvninstall 0m 24s the patch passed +1 compile 0m 20s the patch passed +1 javac 0m 20s the patch passed -1 checkstyle 0m 14s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common: The patch generated 5 new + 0 unchanged - 0 fixed = 5 total (was 0) +1 mvnsite 0m 26s the patch passed +1 mvneclipse 0m 13s the patch passed +1 shellcheck 0m 13s The patch generated 0 new + 74 unchanged - 1 fixed = 74 total (was 75) +1 whitespace 0m 1s The patch has no whitespace issues. +1 findbugs 0m 59s the patch passed +1 javadoc 0m 17s the patch passed +1 unit 0m 59s hadoop-yarn-server-common in the patch passed. +1 asflicense 0m 21s The patch does not generate ASF License warnings. 17m 28s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12831016/YARN-5325-YARN-2915.07.patch JIRA Issue YARN-5325 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle shellcheck shelldocs uname Linux 977462d48cb3 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 YARN-2915 / 6f91338 Default Java 1.8.0_101 shellcheck v0.4.4 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13251/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13251/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common Console output https://builds.apache.org/job/PreCommit-YARN-Build/13251/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          subru Subru Krishnan added a comment -

          Thanks Carlo Curino for working on this. I looked at the patch, please find my comments below.

          General:

          • We should rename Base* classes to Abstract* as that's the naming convention followed in YARN. For e.g. : AbstractYarnScheduler.
          • There are a few typos in Javadocs especially LocalityMulticastAMRMProxyPolicy.
          • The SubClusterResolver should not throw YarnException but instead fallback to home sub-cluster ala DEFAULT_RACK for the RackResolver.

          LocalityMulticastAMRMProxyPolicy:

          • Creating AllocationBookkeeper for each split request seems costly. Would it be to possible to pre-initialize and reuse?
          • Use Math.round instead of (int) Math.ceil in splitIndividualAny.
          • I feel in AllocationBookkeeper initialization, we should loop on weights as num(configured clusters) << num(active clusters).
          • We should collapse the three for loops in AllocationBookkeeper initialization into a single one and the above suggestion should help doing that.
          • I don't see the purpose of using AtomicLongs and TreeMaps in AllocationBookkeeper as we seem to be the incurring the overhead without using the additional attributes.

          Tests (generally a good job at coverage ):

          • The scenario that's missing is multiple/subsequent invocations with combination of state changing like policy weights, active sub-clusters, headroom. Can you please add.
          • We should throw rather than ignore exceptions as that has useful information for debugging in case of test failures.
          • I feel we should avoid System.outs.
          • Can we directly use ResourceRequest::newInstance instead of introducing a local custom FederationPoliciesTestUtil::createResourceRequest?
          • TestLocalityMulticastAMRMProxyFederationPolicy ::testStressPolicy doesn't seem to be doing any assertions?

          Questions:

          • Currently the implementation is not thread-safe. I assume that's a conscious decision given that AMRMProxyService creates a per-application pipeline?
          • I am still trying to grok the correctness and invariance satisfaction for getHeadroomWeighting. At first pass, looks good. I'll revert in case I have any concerns.
          • Similarly I am still in the process of verifying the splitting of numContainers in TestLocalityMulticastAMRMProxyFederationPolicy. If possible, can you add code comments.
          Show
          subru Subru Krishnan added a comment - Thanks Carlo Curino for working on this. I looked at the patch, please find my comments below. General : We should rename Base* classes to Abstract* as that's the naming convention followed in YARN. For e.g. : AbstractYarnScheduler . There are a few typos in Javadocs especially LocalityMulticastAMRMProxyPolicy . The SubClusterResolver should not throw YarnException but instead fallback to home sub-cluster ala DEFAULT_RACK for the RackResolver . LocalityMulticastAMRMProxyPolicy : Creating AllocationBookkeeper for each split request seems costly. Would it be to possible to pre-initialize and reuse? Use Math.round instead of (int) Math.ceil in splitIndividualAny . I feel in AllocationBookkeeper initialization, we should loop on weights as num(configured clusters) << num(active clusters) . We should collapse the three for loops in AllocationBookkeeper initialization into a single one and the above suggestion should help doing that. I don't see the purpose of using AtomicLongs and TreeMaps in AllocationBookkeeper as we seem to be the incurring the overhead without using the additional attributes. Tests (generally a good job at coverage ): The scenario that's missing is multiple/subsequent invocations with combination of state changing like policy weights, active sub-clusters, headroom. Can you please add. We should throw rather than ignore exceptions as that has useful information for debugging in case of test failures. I feel we should avoid System.outs . Can we directly use ResourceRequest::newInstance instead of introducing a local custom FederationPoliciesTestUtil::createResourceRequest ? TestLocalityMulticastAMRMProxyFederationPolicy ::testStressPolicy doesn't seem to be doing any assertions? Questions : Currently the implementation is not thread-safe. I assume that's a conscious decision given that AMRMProxyService creates a per-application pipeline? I am still trying to grok the correctness and invariance satisfaction for getHeadroomWeighting . At first pass, looks good. I'll revert in case I have any concerns. Similarly I am still in the process of verifying the splitting of numContainers in TestLocalityMulticastAMRMProxyFederationPolicy . If possible, can you add code comments.
          Hide
          curino Carlo Curino added a comment -

          Subru Krishnan Thanks for the review, I will start addressing it right away. Few comments answers:

          Regarding "Use Math.round instead of (int) Math.ceil in splitIndividualAny." I don't think we can, as this might lead to less than what the user asked, e.g., I have 3 subclusters with equal weights/load and you ask for 1 container anywhere... I will end up rounding down the 0.333 containers for each subclusters and give you zero containers back.

          testStressTest was mostly to measure how much it takes to run a large number of big allocations. Do you think we should measure time and assert it should be < X? (Seems bad, though the timing information are generally useful as we develop/evolve this). I guess as it stands it only checks that large/many allocations don't end up throwing off anything. I am unsure about this.

          Re: thread safety I am assuming that, though we should validate it carefully...

          I am happy to walk over the getHeadroomWeighting stuff with you... I am also mostly convinced is correct, but must be validated well.

          Thanks,
          Carlo

          Show
          curino Carlo Curino added a comment - Subru Krishnan Thanks for the review, I will start addressing it right away. Few comments answers: Regarding "Use Math.round instead of (int) Math.ceil in splitIndividualAny." I don't think we can, as this might lead to less than what the user asked, e.g., I have 3 subclusters with equal weights/load and you ask for 1 container anywhere... I will end up rounding down the 0.333 containers for each subclusters and give you zero containers back. testStressTest was mostly to measure how much it takes to run a large number of big allocations. Do you think we should measure time and assert it should be < X? (Seems bad, though the timing information are generally useful as we develop/evolve this). I guess as it stands it only checks that large/many allocations don't end up throwing off anything. I am unsure about this. Re: thread safety I am assuming that, though we should validate it carefully... I am happy to walk over the getHeadroomWeighting stuff with you... I am also mostly convinced is correct, but must be validated well. Thanks, Carlo
          Hide
          subru Subru Krishnan added a comment -

          Thanks Carlo Curino for the clarifications and helping me validate getHeadroomWeighting. I only have a nit that we are using memory as proxy for Resource.

          Regarding "Use Math.round instead of (int) Math.ceil in splitIndividualAny." I don't think we can, as this might lead to less than what the user asked, e.g., I have 3 subclusters with equal weights/load and you ask for 1 container anywhere... I will end up rounding down the 0.333 containers for each subclusters and give you zero containers back.

          Good point.

          testStressTest was mostly to measure how much it takes to run a large number of big allocations. Do you think we should measure time and assert it should be < X? (Seems bad, though the timing information are generally useful as we develop/evolve this). I guess as it stands it only checks that large/many allocations don't end up throwing off anything. I am unsure about this.

          Regarding testStressPolicy, I think it's useful. So you should uncomment the validate call and add a sane timeout.

          Re: thread safety I am assuming that, though we should validate it carefully..

          I'll double-check on thread safeness in the next iteration.

          Show
          subru Subru Krishnan added a comment - Thanks Carlo Curino for the clarifications and helping me validate getHeadroomWeighting . I only have a nit that we are using memory as proxy for Resource. Regarding "Use Math.round instead of (int) Math.ceil in splitIndividualAny." I don't think we can, as this might lead to less than what the user asked, e.g., I have 3 subclusters with equal weights/load and you ask for 1 container anywhere... I will end up rounding down the 0.333 containers for each subclusters and give you zero containers back. Good point. testStressTest was mostly to measure how much it takes to run a large number of big allocations. Do you think we should measure time and assert it should be < X? (Seems bad, though the timing information are generally useful as we develop/evolve this). I guess as it stands it only checks that large/many allocations don't end up throwing off anything. I am unsure about this. Regarding testStressPolicy , I think it's useful. So you should uncomment the validate call and add a sane timeout. Re: thread safety I am assuming that, though we should validate it carefully.. I'll double-check on thread safeness in the next iteration.
          Hide
          curino Carlo Curino added a comment -

          Hi Subru Krishnan, the uploaded .09 version of the patch address most of your asks above plus the discussion we had on having a fallback to "homesubcluster" in case the resolver cannot locate the subcluster owning a node/rack. We also log whether this happens. In case the homeSubCluster is not active, we do similarly to what yarn would do and silently ignore the ResourceRequest (please confirm this is ok for you).

          A few minor things were not changed:
          I fixed on AtomicLong that was redundandt, but the one in the HashMap are useful to avoid recreating objects at every update (or lengthy code).
          TreeMap is simply to have sorted output when printed (useful for debugging and with <20 sub-clusters and no data-structure churn this should never be a per issue).
          The system.outs in tests are useful for debugging the tests. I would leave them there if you are ok with it.
          Exceptions in tests were not ignored, they had to be handled by the code later on.
          Regarding multi invocation, the only case for which it makes sense is for "headroom" (as this is the only somewhat stateful side of the policy). I added a second invocation in the corresponding test, to validate.
          Regarding FederationPoliciesTestUtil::createResourceRequest there are few objects to create, and this would make the code in tests quite redundant I think.

          Re thread safeness, the switch to re-using AllocationBookkeeper I think made the code more fragile, so we should definitely double-check whether the intended use
          is single-threaded or protect the splitResourceRequest() with a lock.

          Show
          curino Carlo Curino added a comment - Hi Subru Krishnan , the uploaded .09 version of the patch address most of your asks above plus the discussion we had on having a fallback to "homesubcluster" in case the resolver cannot locate the subcluster owning a node/rack. We also log whether this happens. In case the homeSubCluster is not active, we do similarly to what yarn would do and silently ignore the ResourceRequest (please confirm this is ok for you). A few minor things were not changed: I fixed on AtomicLong that was redundandt, but the one in the HashMap are useful to avoid recreating objects at every update (or lengthy code). TreeMap is simply to have sorted output when printed (useful for debugging and with <20 sub-clusters and no data-structure churn this should never be a per issue). The system.outs in tests are useful for debugging the tests. I would leave them there if you are ok with it. Exceptions in tests were not ignored, they had to be handled by the code later on. Regarding multi invocation, the only case for which it makes sense is for "headroom" (as this is the only somewhat stateful side of the policy). I added a second invocation in the corresponding test, to validate. Regarding FederationPoliciesTestUtil::createResourceRequest there are few objects to create, and this would make the code in tests quite redundant I think. Re thread safeness, the switch to re-using AllocationBookkeeper I think made the code more fragile, so we should definitely double-check whether the intended use is single-threaded or protect the splitResourceRequest() with a lock.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 14s Docker mode activated.
          0 shelldocs 0m 0s Shelldocs was not available.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 5 new or modified test files.
          +1 mvninstall 7m 51s YARN-2915 passed
          +1 compile 0m 24s YARN-2915 passed
          +1 checkstyle 0m 16s YARN-2915 passed
          +1 mvnsite 0m 27s YARN-2915 passed
          +1 mvneclipse 0m 16s YARN-2915 passed
          +1 findbugs 0m 46s YARN-2915 passed
          +1 javadoc 0m 19s YARN-2915 passed
          +1 mvninstall 0m 21s the patch passed
          +1 compile 0m 20s the patch passed
          +1 javac 0m 20s the patch passed
          -1 checkstyle 0m 12s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common: The patch generated 12 new + 0 unchanged - 0 fixed = 12 total (was 0)
          +1 mvnsite 0m 25s the patch passed
          +1 mvneclipse 0m 12s the patch passed
          +1 shellcheck 0m 12s The patch generated 0 new + 74 unchanged - 1 fixed = 74 total (was 75)
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 51s the patch passed
          +1 javadoc 0m 15s the patch passed
          +1 unit 0m 53s hadoop-yarn-server-common in the patch passed.
          +1 asflicense 0m 19s The patch does not generate ASF License warnings.
          15m 21s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12832523/YARN-5325-YARN-2915.08.patch
          JIRA Issue YARN-5325
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle shellcheck shelldocs
          uname Linux 7d7fba61dc47 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 YARN-2915 / 0bf6bbb
          Default Java 1.8.0_101
          shellcheck v0.4.4
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13340/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13340/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/13340/console
          Powered by Apache Yetus 0.3.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 14s Docker mode activated. 0 shelldocs 0m 0s Shelldocs was not available. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 5 new or modified test files. +1 mvninstall 7m 51s YARN-2915 passed +1 compile 0m 24s YARN-2915 passed +1 checkstyle 0m 16s YARN-2915 passed +1 mvnsite 0m 27s YARN-2915 passed +1 mvneclipse 0m 16s YARN-2915 passed +1 findbugs 0m 46s YARN-2915 passed +1 javadoc 0m 19s YARN-2915 passed +1 mvninstall 0m 21s the patch passed +1 compile 0m 20s the patch passed +1 javac 0m 20s the patch passed -1 checkstyle 0m 12s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common: The patch generated 12 new + 0 unchanged - 0 fixed = 12 total (was 0) +1 mvnsite 0m 25s the patch passed +1 mvneclipse 0m 12s the patch passed +1 shellcheck 0m 12s The patch generated 0 new + 74 unchanged - 1 fixed = 74 total (was 75) +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 51s the patch passed +1 javadoc 0m 15s the patch passed +1 unit 0m 53s hadoop-yarn-server-common in the patch passed. +1 asflicense 0m 19s The patch does not generate ASF License warnings. 15m 21s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12832523/YARN-5325-YARN-2915.08.patch JIRA Issue YARN-5325 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle shellcheck shelldocs uname Linux 7d7fba61dc47 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 YARN-2915 / 0bf6bbb Default Java 1.8.0_101 shellcheck v0.4.4 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13340/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13340/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common Console output https://builds.apache.org/job/PreCommit-YARN-Build/13340/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 18s Docker mode activated.
          0 shelldocs 0m 1s Shelldocs was not available.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 5 new or modified test files.
          +1 mvninstall 10m 12s YARN-2915 passed
          +1 compile 0m 25s YARN-2915 passed
          +1 checkstyle 0m 16s YARN-2915 passed
          +1 mvnsite 0m 30s YARN-2915 passed
          +1 mvneclipse 0m 17s YARN-2915 passed
          +1 findbugs 0m 53s YARN-2915 passed
          +1 javadoc 0m 20s YARN-2915 passed
          +1 mvninstall 0m 22s the patch passed
          +1 compile 0m 20s the patch passed
          +1 javac 0m 20s the patch passed
          -1 checkstyle 0m 12s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common: The patch generated 12 new + 0 unchanged - 0 fixed = 12 total (was 0)
          +1 mvnsite 0m 28s the patch passed
          +1 mvneclipse 0m 13s the patch passed
          +1 shellcheck 0m 13s The patch generated 0 new + 74 unchanged - 1 fixed = 74 total (was 75)
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 1s the patch passed
          +1 javadoc 0m 17s the patch passed
          +1 unit 1m 2s hadoop-yarn-server-common in the patch passed.
          +1 asflicense 0m 19s The patch does not generate ASF License warnings.
          18m 29s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12832527/YARN-5325-YARN-2915.09.patch
          JIRA Issue YARN-5325
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle shellcheck shelldocs
          uname Linux a43ca7b9c81b 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 YARN-2915 / 0bf6bbb
          Default Java 1.8.0_101
          shellcheck v0.4.4
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13341/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13341/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/13341/console
          Powered by Apache Yetus 0.3.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 18s Docker mode activated. 0 shelldocs 0m 1s Shelldocs was not available. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 5 new or modified test files. +1 mvninstall 10m 12s YARN-2915 passed +1 compile 0m 25s YARN-2915 passed +1 checkstyle 0m 16s YARN-2915 passed +1 mvnsite 0m 30s YARN-2915 passed +1 mvneclipse 0m 17s YARN-2915 passed +1 findbugs 0m 53s YARN-2915 passed +1 javadoc 0m 20s YARN-2915 passed +1 mvninstall 0m 22s the patch passed +1 compile 0m 20s the patch passed +1 javac 0m 20s the patch passed -1 checkstyle 0m 12s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common: The patch generated 12 new + 0 unchanged - 0 fixed = 12 total (was 0) +1 mvnsite 0m 28s the patch passed +1 mvneclipse 0m 13s the patch passed +1 shellcheck 0m 13s The patch generated 0 new + 74 unchanged - 1 fixed = 74 total (was 75) +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 1s the patch passed +1 javadoc 0m 17s the patch passed +1 unit 1m 2s hadoop-yarn-server-common in the patch passed. +1 asflicense 0m 19s The patch does not generate ASF License warnings. 18m 29s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12832527/YARN-5325-YARN-2915.09.patch JIRA Issue YARN-5325 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle shellcheck shelldocs uname Linux a43ca7b9c81b 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 YARN-2915 / 0bf6bbb Default Java 1.8.0_101 shellcheck v0.4.4 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13341/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13341/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common Console output https://builds.apache.org/job/PreCommit-YARN-Build/13341/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 16s Docker mode activated.
          0 shelldocs 0m 0s Shelldocs was not available.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 5 new or modified test files.
          +1 mvninstall 8m 44s YARN-2915 passed
          +1 compile 0m 25s YARN-2915 passed
          +1 checkstyle 0m 17s YARN-2915 passed
          +1 mvnsite 0m 29s YARN-2915 passed
          +1 mvneclipse 0m 18s YARN-2915 passed
          +1 findbugs 0m 52s YARN-2915 passed
          +1 javadoc 0m 20s YARN-2915 passed
          +1 mvninstall 0m 28s the patch passed
          +1 compile 0m 23s the patch passed
          +1 javac 0m 23s the patch passed
          -1 checkstyle 0m 13s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
          +1 mvnsite 0m 28s the patch passed
          +1 mvneclipse 0m 13s the patch passed
          +1 shellcheck 0m 13s The patch generated 0 new + 74 unchanged - 1 fixed = 74 total (was 75)
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 3s the patch passed
          +1 javadoc 0m 16s the patch passed
          +1 unit 0m 56s hadoop-yarn-server-common in the patch passed.
          +1 asflicense 0m 21s The patch does not generate ASF License warnings.
          17m 4s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12832551/YARN-5325-YARN-2915.10.patch
          JIRA Issue YARN-5325
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle shellcheck shelldocs
          uname Linux d2b032bc8ea0 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 YARN-2915 / 0bf6bbb
          Default Java 1.8.0_101
          shellcheck v0.4.4
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13344/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13344/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/13344/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 16s Docker mode activated. 0 shelldocs 0m 0s Shelldocs was not available. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 5 new or modified test files. +1 mvninstall 8m 44s YARN-2915 passed +1 compile 0m 25s YARN-2915 passed +1 checkstyle 0m 17s YARN-2915 passed +1 mvnsite 0m 29s YARN-2915 passed +1 mvneclipse 0m 18s YARN-2915 passed +1 findbugs 0m 52s YARN-2915 passed +1 javadoc 0m 20s YARN-2915 passed +1 mvninstall 0m 28s the patch passed +1 compile 0m 23s the patch passed +1 javac 0m 23s the patch passed -1 checkstyle 0m 13s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) +1 mvnsite 0m 28s the patch passed +1 mvneclipse 0m 13s the patch passed +1 shellcheck 0m 13s The patch generated 0 new + 74 unchanged - 1 fixed = 74 total (was 75) +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 3s the patch passed +1 javadoc 0m 16s the patch passed +1 unit 0m 56s hadoop-yarn-server-common in the patch passed. +1 asflicense 0m 21s The patch does not generate ASF License warnings. 17m 4s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12832551/YARN-5325-YARN-2915.10.patch JIRA Issue YARN-5325 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle shellcheck shelldocs uname Linux d2b032bc8ea0 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 YARN-2915 / 0bf6bbb Default Java 1.8.0_101 shellcheck v0.4.4 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13344/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13344/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common Console output https://builds.apache.org/job/PreCommit-YARN-Build/13344/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          subru Subru Krishnan added a comment -

          Thanks Carlo Curino for addressing my comments. The patch looks close, have a few minor comments:

          • The AllocationBookkeeper is still being reinitialized (which is where the heavy lifting is) for every splitResourceRequests invocation. Instead we should do it as part of reinitialize of LocalityMulticastAMRMProxyPolicy as we already short-circuit based on dirty flag there. Additionally, we only need to update total headroom as part of notifyOfResponse.
          • Nit: we should log exceptions (at debug level?) from the SubClusterResolver.
          • We should do null check for targetId, could be done in isActiveAndEnabled.
          • We should use continues instead of multi-level nested if-else in splitResourceRequests for readability.
          • Can you please add code comments for expected numContainers in TestLocalityMulticastAMRMProxyFederationPolicy.

          The system.outs in tests are useful for debugging the tests. I would leave them there if you are ok with it.

          • In that case, can you replace the syso with log statements.

          Regarding FederationPoliciesTestUtil::createResourceRequest there are few objects to create, and this would make the code in tests quite redundant I think.

          • There seems to be only Priority and Resource both of which are easily inline-able but this is only a nit.
          Show
          subru Subru Krishnan added a comment - Thanks Carlo Curino for addressing my comments. The patch looks close, have a few minor comments: The AllocationBookkeeper is still being reinitialized (which is where the heavy lifting is) for every splitResourceRequests invocation. Instead we should do it as part of reinitialize of LocalityMulticastAMRMProxyPolicy as we already short-circuit based on dirty flag there. Additionally, we only need to update total headroom as part of notifyOfResponse . Nit: we should log exceptions (at debug level?) from the SubClusterResolver . We should do null check for targetId , could be done in isActiveAndEnabled . We should use continues instead of multi-level nested if-else in splitResourceRequests for readability. Can you please add code comments for expected numContainers in TestLocalityMulticastAMRMProxyFederationPolicy . The system.outs in tests are useful for debugging the tests. I would leave them there if you are ok with it. In that case, can you replace the syso with log statements. Regarding FederationPoliciesTestUtil::createResourceRequest there are few objects to create, and this would make the code in tests quite redundant I think. There seems to be only Priority and Resource both of which are easily inline-able but this is only a nit.
          Hide
          curino Carlo Curino added a comment -

          Thanks again for the suggestions.

          As per our offline discussion AllocationBookeeper cannot be reinit once and forall as the accumulation of weights it does in reinit depends on the (possibly changing) set of active subclusters.

          We do log at debug level if the SubClusterResolver throws, but I added a if(debug) to avoid costly string construction if we are not in debug mode per your advise.

          Using "continue" makes the code much more legible, thanks for the suggestion.

          The createResourceRequest is invoked tens of times in TestLocalityMulticastAMRMProxyFederationPolicy (now renamed to drop the federation), so I would prefer to leave it as is.

          Show
          curino Carlo Curino added a comment - Thanks again for the suggestions. As per our offline discussion AllocationBookeeper cannot be reinit once and forall as the accumulation of weights it does in reinit depends on the (possibly changing) set of active subclusters. We do log at debug level if the SubClusterResolver throws, but I added a if(debug) to avoid costly string construction if we are not in debug mode per your advise. Using "continue" makes the code much more legible, thanks for the suggestion. The createResourceRequest is invoked tens of times in TestLocalityMulticastAMRMProxyFederationPolicy (now renamed to drop the federation), so I would prefer to leave it as is.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 18s Docker mode activated.
          0 shelldocs 0m 1s Shelldocs was not available.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 5 new or modified test files.
          +1 mvninstall 7m 7s YARN-2915 passed
          +1 compile 0m 23s YARN-2915 passed
          +1 checkstyle 0m 16s YARN-2915 passed
          +1 mvnsite 0m 27s YARN-2915 passed
          +1 mvneclipse 0m 16s YARN-2915 passed
          +1 findbugs 0m 47s YARN-2915 passed
          +1 javadoc 0m 19s YARN-2915 passed
          +1 mvninstall 0m 22s the patch passed
          +1 compile 0m 20s the patch passed
          +1 javac 0m 20s the patch passed
          -1 checkstyle 0m 13s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common: The patch generated 2 new + 0 unchanged - 0 fixed = 2 total (was 0)
          +1 mvnsite 0m 25s the patch passed
          +1 mvneclipse 0m 12s the patch passed
          +1 shellcheck 0m 12s The patch generated 0 new + 74 unchanged - 1 fixed = 74 total (was 75)
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 53s the patch passed
          +1 javadoc 0m 16s the patch passed
          +1 unit 0m 55s hadoop-yarn-server-common in the patch passed.
          +1 asflicense 0m 17s The patch does not generate ASF License warnings.
          14m 42s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12832955/YARN-5325-YARN-2915.11.patch
          JIRA Issue YARN-5325
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle shellcheck shelldocs
          uname Linux cd1a9f4679a6 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 YARN-2915 / 0bf6bbb
          Default Java 1.8.0_101
          shellcheck v0.4.4
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13362/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13362/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/13362/console
          Powered by Apache Yetus 0.3.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 18s Docker mode activated. 0 shelldocs 0m 1s Shelldocs was not available. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 5 new or modified test files. +1 mvninstall 7m 7s YARN-2915 passed +1 compile 0m 23s YARN-2915 passed +1 checkstyle 0m 16s YARN-2915 passed +1 mvnsite 0m 27s YARN-2915 passed +1 mvneclipse 0m 16s YARN-2915 passed +1 findbugs 0m 47s YARN-2915 passed +1 javadoc 0m 19s YARN-2915 passed +1 mvninstall 0m 22s the patch passed +1 compile 0m 20s the patch passed +1 javac 0m 20s the patch passed -1 checkstyle 0m 13s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common: The patch generated 2 new + 0 unchanged - 0 fixed = 2 total (was 0) +1 mvnsite 0m 25s the patch passed +1 mvneclipse 0m 12s the patch passed +1 shellcheck 0m 12s The patch generated 0 new + 74 unchanged - 1 fixed = 74 total (was 75) +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 53s the patch passed +1 javadoc 0m 16s the patch passed +1 unit 0m 55s hadoop-yarn-server-common in the patch passed. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 14m 42s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12832955/YARN-5325-YARN-2915.11.patch JIRA Issue YARN-5325 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle shellcheck shelldocs uname Linux cd1a9f4679a6 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 YARN-2915 / 0bf6bbb Default Java 1.8.0_101 shellcheck v0.4.4 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13362/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13362/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common Console output https://builds.apache.org/job/PreCommit-YARN-Build/13362/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          subru Subru Krishnan added a comment -

          Thanks Carlo Curino for the updated patch. +1 from my side pending fix for checkstyle warnings.

          A couple of nits:

          • I think it'll help to have a simple private method to assert expected numConatiners for a subCluster in TestLocalityMulticastAMRMProxyPolicy.
          • Thanks for the code comments for tests, found it useful. Can you please add for testSplitBasedOnHeadroomAndWeights also.
          • If possible can you update to use slf4j for logging throughout.
          Show
          subru Subru Krishnan added a comment - Thanks Carlo Curino for the updated patch. +1 from my side pending fix for checkstyle warnings. A couple of nits: I think it'll help to have a simple private method to assert expected numConatiners for a subCluster in TestLocalityMulticastAMRMProxyPolicy . Thanks for the code comments for tests, found it useful. Can you please add for testSplitBasedOnHeadroomAndWeights also. If possible can you update to use slf4j for logging throughout.
          Hide
          curino Carlo Curino added a comment -

          Subru Krishnan I believe I address all your asks, let's wait for usual checkstyle (as I changed lots of stuff and IntelliJ doesn't enforce exactly what checkstyle wants), and then we are good to commit (if green).

          Show
          curino Carlo Curino added a comment - Subru Krishnan I believe I address all your asks, let's wait for usual checkstyle (as I changed lots of stuff and IntelliJ doesn't enforce exactly what checkstyle wants), and then we are good to commit (if green).
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 19s Docker mode activated.
          0 shelldocs 0m 0s Shelldocs was not available.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 5 new or modified test files.
          +1 mvninstall 6m 59s YARN-2915 passed
          +1 compile 0m 23s YARN-2915 passed
          +1 checkstyle 0m 15s YARN-2915 passed
          +1 mvnsite 0m 27s YARN-2915 passed
          +1 mvneclipse 0m 15s YARN-2915 passed
          +1 findbugs 0m 44s YARN-2915 passed
          +1 javadoc 0m 19s YARN-2915 passed
          +1 mvninstall 0m 20s the patch passed
          +1 compile 0m 20s the patch passed
          +1 javac 0m 20s the patch passed
          -1 checkstyle 0m 12s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
          +1 mvnsite 0m 23s the patch passed
          +1 mvneclipse 0m 11s the patch passed
          +1 shellcheck 0m 12s The patch generated 0 new + 74 unchanged - 1 fixed = 74 total (was 75)
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 49s the patch passed
          +1 javadoc 0m 15s the patch passed
          +1 unit 0m 49s hadoop-yarn-server-common in the patch passed.
          +1 asflicense 0m 17s The patch does not generate ASF License warnings.
          14m 14s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12833176/YARN-5325-YARN-2915.12.patch
          JIRA Issue YARN-5325
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle shellcheck shelldocs
          uname Linux bdfecc5d9ae2 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 YARN-2915 / 0bf6bbb
          Default Java 1.8.0_101
          shellcheck v0.4.4
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13379/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13379/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/13379/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. 0 shelldocs 0m 0s Shelldocs was not available. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 5 new or modified test files. +1 mvninstall 6m 59s YARN-2915 passed +1 compile 0m 23s YARN-2915 passed +1 checkstyle 0m 15s YARN-2915 passed +1 mvnsite 0m 27s YARN-2915 passed +1 mvneclipse 0m 15s YARN-2915 passed +1 findbugs 0m 44s YARN-2915 passed +1 javadoc 0m 19s YARN-2915 passed +1 mvninstall 0m 20s the patch passed +1 compile 0m 20s the patch passed +1 javac 0m 20s the patch passed -1 checkstyle 0m 12s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) +1 mvnsite 0m 23s the patch passed +1 mvneclipse 0m 11s the patch passed +1 shellcheck 0m 12s The patch generated 0 new + 74 unchanged - 1 fixed = 74 total (was 75) +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 49s the patch passed +1 javadoc 0m 15s the patch passed +1 unit 0m 49s hadoop-yarn-server-common in the patch passed. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 14m 14s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12833176/YARN-5325-YARN-2915.12.patch JIRA Issue YARN-5325 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle shellcheck shelldocs uname Linux bdfecc5d9ae2 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 YARN-2915 / 0bf6bbb Default Java 1.8.0_101 shellcheck v0.4.4 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13379/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13379/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common Console output https://builds.apache.org/job/PreCommit-YARN-Build/13379/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          subru Subru Krishnan added a comment -

          Attaching patch that fixes checkstyle and formatting for Yetus check before committing.

          Show
          subru Subru Krishnan added a comment - Attaching patch that fixes checkstyle and formatting for Yetus check before committing.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 21s Docker mode activated.
          0 shelldocs 0m 1s Shelldocs was not available.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 9 new or modified test files.
          +1 mvninstall 8m 31s YARN-2915 passed
          +1 compile 0m 36s YARN-2915 passed
          +1 checkstyle 0m 17s YARN-2915 passed
          +1 mvnsite 0m 32s YARN-2915 passed
          +1 mvneclipse 0m 17s YARN-2915 passed
          +1 findbugs 0m 54s YARN-2915 passed
          +1 javadoc 0m 22s YARN-2915 passed
          +1 mvninstall 0m 24s the patch passed
          +1 compile 0m 24s the patch passed
          +1 javac 0m 24s the patch passed
          -1 checkstyle 0m 14s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
          +1 mvnsite 0m 29s the patch passed
          +1 mvneclipse 0m 13s the patch passed
          +1 shellcheck 0m 13s The patch generated 0 new + 74 unchanged - 1 fixed = 74 total (was 75)
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 0s the patch passed
          +1 javadoc 0m 17s the patch passed
          +1 unit 0m 56s hadoop-yarn-server-common in the patch passed.
          +1 asflicense 0m 19s The patch does not generate ASF License warnings.
          17m 11s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12833241/YARN-5325-YARN-2915.13.patch
          JIRA Issue YARN-5325
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle shellcheck shelldocs
          uname Linux ffe2f11c8184 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 YARN-2915 / 0bf6bbb
          Default Java 1.8.0_101
          shellcheck v0.4.4
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13386/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13386/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/13386/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 21s Docker mode activated. 0 shelldocs 0m 1s Shelldocs was not available. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 9 new or modified test files. +1 mvninstall 8m 31s YARN-2915 passed +1 compile 0m 36s YARN-2915 passed +1 checkstyle 0m 17s YARN-2915 passed +1 mvnsite 0m 32s YARN-2915 passed +1 mvneclipse 0m 17s YARN-2915 passed +1 findbugs 0m 54s YARN-2915 passed +1 javadoc 0m 22s YARN-2915 passed +1 mvninstall 0m 24s the patch passed +1 compile 0m 24s the patch passed +1 javac 0m 24s the patch passed -1 checkstyle 0m 14s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) +1 mvnsite 0m 29s the patch passed +1 mvneclipse 0m 13s the patch passed +1 shellcheck 0m 13s The patch generated 0 new + 74 unchanged - 1 fixed = 74 total (was 75) +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 0s the patch passed +1 javadoc 0m 17s the patch passed +1 unit 0m 56s hadoop-yarn-server-common in the patch passed. +1 asflicense 0m 19s The patch does not generate ASF License warnings. 17m 11s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12833241/YARN-5325-YARN-2915.13.patch JIRA Issue YARN-5325 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle shellcheck shelldocs uname Linux ffe2f11c8184 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 YARN-2915 / 0bf6bbb Default Java 1.8.0_101 shellcheck v0.4.4 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13386/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13386/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common Console output https://builds.apache.org/job/PreCommit-YARN-Build/13386/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          subru Subru Krishnan added a comment -

          Missed one checkstyle issue, so updated patch that fixes it.

          Show
          subru Subru Krishnan added a comment - Missed one checkstyle issue, so updated patch that fixes it.
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 16s Docker mode activated.
          0 shelldocs 0m 0s Shelldocs was not available.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 9 new or modified test files.
          +1 mvninstall 6m 57s YARN-2915 passed
          +1 compile 0m 24s YARN-2915 passed
          +1 checkstyle 0m 16s YARN-2915 passed
          +1 mvnsite 0m 28s YARN-2915 passed
          +1 mvneclipse 0m 15s YARN-2915 passed
          +1 findbugs 0m 45s YARN-2915 passed
          +1 javadoc 0m 20s YARN-2915 passed
          +1 mvninstall 0m 21s the patch passed
          +1 compile 0m 25s the patch passed
          +1 javac 0m 25s the patch passed
          +1 checkstyle 0m 12s the patch passed
          +1 mvnsite 0m 25s the patch passed
          +1 mvneclipse 0m 11s the patch passed
          +1 shellcheck 0m 11s The patch generated 0 new + 74 unchanged - 1 fixed = 74 total (was 75)
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 53s the patch passed
          +1 javadoc 0m 15s the patch passed
          +1 unit 0m 50s hadoop-yarn-server-common in the patch passed.
          +1 asflicense 0m 16s The patch does not generate ASF License warnings.
          14m 28s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12833257/YARN-5325-YARN-2915.14.patch
          JIRA Issue YARN-5325
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle shellcheck shelldocs
          uname Linux f8f5ab23e989 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 YARN-2915 / 0bf6bbb
          Default Java 1.8.0_101
          shellcheck v0.4.4
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13388/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/13388/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 16s Docker mode activated. 0 shelldocs 0m 0s Shelldocs was not available. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 9 new or modified test files. +1 mvninstall 6m 57s YARN-2915 passed +1 compile 0m 24s YARN-2915 passed +1 checkstyle 0m 16s YARN-2915 passed +1 mvnsite 0m 28s YARN-2915 passed +1 mvneclipse 0m 15s YARN-2915 passed +1 findbugs 0m 45s YARN-2915 passed +1 javadoc 0m 20s YARN-2915 passed +1 mvninstall 0m 21s the patch passed +1 compile 0m 25s the patch passed +1 javac 0m 25s the patch passed +1 checkstyle 0m 12s the patch passed +1 mvnsite 0m 25s the patch passed +1 mvneclipse 0m 11s the patch passed +1 shellcheck 0m 11s The patch generated 0 new + 74 unchanged - 1 fixed = 74 total (was 75) +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 53s the patch passed +1 javadoc 0m 15s the patch passed +1 unit 0m 50s hadoop-yarn-server-common in the patch passed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 14m 28s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12833257/YARN-5325-YARN-2915.14.patch JIRA Issue YARN-5325 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle shellcheck shelldocs uname Linux f8f5ab23e989 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 YARN-2915 / 0bf6bbb Default Java 1.8.0_101 shellcheck v0.4.4 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13388/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common Console output https://builds.apache.org/job/PreCommit-YARN-Build/13388/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          subru Subru Krishnan added a comment -

          Thanks Carlo Curino for the contribution. I just committed this to branch YARN-2915.

          Show
          subru Subru Krishnan added a comment - Thanks Carlo Curino for the contribution. I just committed this to branch YARN-2915 .
          Hide
          curino Carlo Curino added a comment -

          Thanks Subru Krishnan for reviewing and checkstyle fixes.

          Show
          curino Carlo Curino added a comment - Thanks Subru Krishnan for reviewing and checkstyle fixes.

            People

            • Assignee:
              curino Carlo Curino
              Reporter:
              curino Carlo Curino
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development