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

Support user-specific minimum user limit percentage in Capacity Scheduler

    Details

    • Hadoop Flags:
      Reviewed

      Description

      Currently, in the capacity scheduler, the minimum-user-limit-percent property is per queue. A cluster admin should be able to set the minimum user limit percent on a per-user basis within the queue.

      This functionality is needed so that when intra-queue preemption is enabled (YARN-4945 / YARN-2113), some users can be deemed as more important than other users, and resources from VIP users won't be as likely to be preempted.

      For example, if the getstuffdone queue has a MULP of 25 percent, but user jane is a power user of queue getstuffdone and needs to be guaranteed 75 percent, the properties for getstuffdone and jane would look like this:

        <property>
          <name>yarn.scheduler.capacity.root.getstuffdone.minimum-user-limit-percent</name>
          <value>25</value>
        </property>
      
        <property>
          <name>yarn.scheduler.capacity.root.getstuffdone.jane.minimum-user-limit-percent</name>
          <value>75</value>
        </property>
      
      1. Active users highlighted.jpg
        485 kB
        Eric Payne
      2. YARN-5892.001.patch
        34 kB
        Eric Payne
      3. YARN-5892.002.patch
        34 kB
        Eric Payne
      4. YARN-5892.003.patch
        38 kB
        Eric Payne
      5. YARN-5892.004.patch
        40 kB
        Eric Payne
      6. YARN-5892.005.patch
        42 kB
        Eric Payne
      7. YARN-5892.006.patch
        30 kB
        Eric Payne
      8. YARN-5892.007.patch
        36 kB
        Eric Payne
      9. YARN-5892.008.patch
        36 kB
        Eric Payne
      10. YARN-5892.009.patch
        36 kB
        Eric Payne
      11. YARN-5892.010.patch
        45 kB
        Eric Payne
      12. YARN-5892.012.patch
        45 kB
        Eric Payne
      13. YARN-5892.013.patch
        45 kB
        Eric Payne
      14. YARN-5892.014.patch
        43 kB
        Eric Payne
      15. YARN-5892.015.patch
        42 kB
        Eric Payne
      16. YARN-5892.branch-2.015.patch
        38 kB
        Eric Payne
      17. YARN-5892.branch-2.016.patch
        42 kB
        Eric Payne
      18. YARN-5892.branch-2.8.016.patch
        39 kB
        Eric Payne
      19. YARN-5892.branch-2.8.017.patch
        39 kB
        Eric Payne
      20. YARN-5892.branch-2.8.018.patch
        39 kB
        Eric Payne

        Issue Links

          Activity

          Hide
          leftnoteasy Wangda Tan added a comment -

          Linked this JIRA to YARN-5889, I think this is more like a special case of YARN-5889, but we need to make API to be extensible and easier to use.

          + Sunil G

          Show
          leftnoteasy Wangda Tan added a comment - Linked this JIRA to YARN-5889 , I think this is more like a special case of YARN-5889 , but we need to make API to be extensible and easier to use. + Sunil G
          Hide
          eepayne Eric Payne added a comment -

          Attaching first draft of the patch. I still need to make the user-specific-minimum-user-limit-percent refreshable, but I wanted to get this out there for review sooner rather than later.

          Show
          eepayne Eric Payne added a comment - Attaching first draft of the patch. I still need to make the user-specific-minimum-user-limit-percent refreshable, but I wanted to get this out there for review sooner rather than later.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 21s 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 13m 46s trunk passed
          +1 compile 0m 38s trunk passed
          +1 checkstyle 0m 30s trunk passed
          +1 mvnsite 0m 37s trunk passed
          +1 mvneclipse 0m 17s trunk passed
          +1 findbugs 1m 10s trunk passed
          +1 javadoc 0m 21s trunk passed
          +1 mvninstall 0m 34s the patch passed
          +1 compile 0m 32s the patch passed
          +1 javac 0m 32s the patch passed
          +1 checkstyle 0m 25s the patch passed
          +1 mvnsite 0m 35s the patch passed
          +1 mvneclipse 0m 12s the patch passed
          -1 whitespace 0m 0s The patch has 16 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
          -1 findbugs 1m 23s 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 17s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 1 new + 908 unchanged - 0 fixed = 909 total (was 908)
          -1 unit 43m 56s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 asflicense 0m 18s The patch does not generate ASF License warnings.
          67m 23s



          Reason Tests
          FindBugs module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
            org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration.getUserSpecificUserLimit(String, String) concatenates strings using + in a loop At CapacitySchedulerConfiguration.java:+ in a loop At CapacitySchedulerConfiguration.java:[line 1418]
          Failed junit tests hadoop.yarn.server.resourcemanager.TestRMRestart



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue YARN-5892
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12854099/YARN-5892.001.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux f23c54255b79 3.13.0-108-generic #155-Ubuntu SMP Wed Jan 11 16:58:52 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / b10e962
          Default Java 1.8.0_121
          findbugs v3.0.0
          whitespace https://builds.apache.org/job/PreCommit-YARN-Build/15051/artifact/patchprocess/whitespace-eol.txt
          findbugs https://builds.apache.org/job/PreCommit-YARN-Build/15051/artifact/patchprocess/new-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.html
          javadoc https://builds.apache.org/job/PreCommit-YARN-Build/15051/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/15051/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/15051/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/15051/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT 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. +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 13m 46s trunk passed +1 compile 0m 38s trunk passed +1 checkstyle 0m 30s trunk passed +1 mvnsite 0m 37s trunk passed +1 mvneclipse 0m 17s trunk passed +1 findbugs 1m 10s trunk passed +1 javadoc 0m 21s trunk passed +1 mvninstall 0m 34s the patch passed +1 compile 0m 32s the patch passed +1 javac 0m 32s the patch passed +1 checkstyle 0m 25s the patch passed +1 mvnsite 0m 35s the patch passed +1 mvneclipse 0m 12s the patch passed -1 whitespace 0m 0s The patch has 16 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply -1 findbugs 1m 23s 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 17s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 1 new + 908 unchanged - 0 fixed = 909 total (was 908) -1 unit 43m 56s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 18s The patch does not generate ASF License warnings. 67m 23s Reason Tests FindBugs module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager   org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration.getUserSpecificUserLimit(String, String) concatenates strings using + in a loop At CapacitySchedulerConfiguration.java:+ in a loop At CapacitySchedulerConfiguration.java: [line 1418] Failed junit tests hadoop.yarn.server.resourcemanager.TestRMRestart Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue YARN-5892 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12854099/YARN-5892.001.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux f23c54255b79 3.13.0-108-generic #155-Ubuntu SMP Wed Jan 11 16:58:52 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / b10e962 Default Java 1.8.0_121 findbugs v3.0.0 whitespace https://builds.apache.org/job/PreCommit-YARN-Build/15051/artifact/patchprocess/whitespace-eol.txt findbugs https://builds.apache.org/job/PreCommit-YARN-Build/15051/artifact/patchprocess/new-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.html javadoc https://builds.apache.org/job/PreCommit-YARN-Build/15051/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/15051/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/15051/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/15051/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          eepayne Eric Payne added a comment -

          Uploading new patch to address the javadoc and findbug warnings. I also modified this patch to be refreshable.

          The unit test (TestRMRestart) is failing intermittently on trunk with and without this patch.

          Show
          eepayne Eric Payne added a comment - Uploading new patch to address the javadoc and findbug warnings. I also modified this patch to be refreshable. The unit test ( TestRMRestart ) is failing intermittently on trunk with and without this patch.
          Hide
          leftnoteasy Wangda Tan added a comment -

          Hi Eric Payne,

          Apologize for my late responses, finally get some time to look at this.

          I'm a little concerned about semantics of this feature. (I've to admit that existing user-limit semantics is not perfect first) For existing MULP, to me it has two guarantees:
          #1, if there're N (N <= 100 / MULP) users are consuming resource in a queue, each of them can get at least MULP / 100 * queue-configured-capacity.
          #2, when doing preemption, if an user uses <= MULP / 100 * queue-configured-capacity, we will not preempt resource from such users.

          If we introduced this feature, per my understanding, #1 will be break. For example, a queue with MULP = 33, and user=admin has MULP = 75. Assume we have 3 users: Jack, Alice, Admin are running jobs in the queue, there's no guarantee that any user can use at much as their MULP.

          To make a more clear semantics, in my mind there're some alternative solutions:
          a. Create queue just for such vip users, and we can plan resources, (for example root.users.admin = 30%, and root.users.others = 50%).
          b. In addition to that, special queue mapping rules can be implemented based on YARN-3635, which can do more flexible queue mappings for use cases like vip users.

          Please let me know you thoughts.

          Show
          leftnoteasy Wangda Tan added a comment - Hi Eric Payne , Apologize for my late responses, finally get some time to look at this. I'm a little concerned about semantics of this feature. (I've to admit that existing user-limit semantics is not perfect first) For existing MULP, to me it has two guarantees: #1, if there're N (N <= 100 / MULP) users are consuming resource in a queue, each of them can get at least MULP / 100 * queue-configured-capacity. #2, when doing preemption, if an user uses <= MULP / 100 * queue-configured-capacity, we will not preempt resource from such users. If we introduced this feature, per my understanding, #1 will be break. For example, a queue with MULP = 33, and user=admin has MULP = 75. Assume we have 3 users: Jack, Alice, Admin are running jobs in the queue, there's no guarantee that any user can use at much as their MULP. To make a more clear semantics, in my mind there're some alternative solutions: a. Create queue just for such vip users, and we can plan resources, (for example root.users.admin = 30%, and root.users.others = 50%). b. In addition to that, special queue mapping rules can be implemented based on YARN-3635 , which can do more flexible queue mappings for use cases like vip users. Please let me know you thoughts.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 14s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 14m 51s trunk passed
          +1 compile 0m 33s trunk passed
          +1 checkstyle 0m 25s trunk passed
          +1 mvnsite 0m 41s trunk passed
          +1 mvneclipse 0m 17s trunk passed
          +1 findbugs 1m 12s trunk passed
          +1 javadoc 0m 23s trunk passed
          +1 mvninstall 0m 30s the patch passed
          +1 compile 0m 28s the patch passed
          +1 javac 0m 28s the patch passed
          +1 checkstyle 0m 43s the patch passed
          +1 mvnsite 0m 38s the patch passed
          +1 mvneclipse 0m 12s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 3s the patch passed
          +1 javadoc 0m 19s the patch passed
          -1 unit 40m 31s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 asflicense 0m 20s The patch does not generate ASF License warnings.
          64m 47s



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



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue YARN-5892
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12854316/YARN-5892.002.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux fce68500d71f 3.13.0-107-generic #154-Ubuntu SMP Tue Dec 20 09:57:27 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 694e680
          Default Java 1.8.0_121
          findbugs v3.0.0
          unit https://builds.apache.org/job/PreCommit-YARN-Build/15066/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/15066/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/15066/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 14s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 14m 51s trunk passed +1 compile 0m 33s trunk passed +1 checkstyle 0m 25s trunk passed +1 mvnsite 0m 41s trunk passed +1 mvneclipse 0m 17s trunk passed +1 findbugs 1m 12s trunk passed +1 javadoc 0m 23s trunk passed +1 mvninstall 0m 30s the patch passed +1 compile 0m 28s the patch passed +1 javac 0m 28s the patch passed +1 checkstyle 0m 43s the patch passed +1 mvnsite 0m 38s the patch passed +1 mvneclipse 0m 12s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 3s the patch passed +1 javadoc 0m 19s the patch passed -1 unit 40m 31s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 20s The patch does not generate ASF License warnings. 64m 47s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.fair.TestFairScheduler Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue YARN-5892 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12854316/YARN-5892.002.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux fce68500d71f 3.13.0-107-generic #154-Ubuntu SMP Tue Dec 20 09:57:27 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 694e680 Default Java 1.8.0_121 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-YARN-Build/15066/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/15066/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/15066/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          eepayne Eric Payne added a comment -

          Thanks, Wangda Tan, for your feedback. I really value your input.

          in my mind there're some alternative solutions:
          a. Create queue just for such vip users

          In our multi-tenant clusters, we have several users (sometimes dozens) needing to use the same queue. Setting up separate queues for each of them based on weighted importance is more complicated than giving each users their own weight.

          #1, if there're N (N <= 100 / MULP) users are consuming resource in a queue, each of them can get at least MULP / 100 * queue-configured-capacity.

          Even today, we can have N > 100/MULP. If I think of these VIP users being the weighted as multiple users, then we have a similar situation. In your example above, Jack and Alice would be weighted as 1 user, but Admin would be 2.5 users.

          Show
          eepayne Eric Payne added a comment - Thanks, Wangda Tan , for your feedback. I really value your input. in my mind there're some alternative solutions: a. Create queue just for such vip users In our multi-tenant clusters, we have several users (sometimes dozens) needing to use the same queue. Setting up separate queues for each of them based on weighted importance is more complicated than giving each users their own weight. #1, if there're N (N <= 100 / MULP) users are consuming resource in a queue, each of them can get at least MULP / 100 * queue-configured-capacity. Even today, we can have N > 100/MULP. If I think of these VIP users being the weighted as multiple users, then we have a similar situation. In your example above, Jack and Alice would be weighted as 1 user, but Admin would be 2.5 users.
          Hide
          leftnoteasy Wangda Tan added a comment -

          Thanks Eric Payne for the elaborations,

          I have a discussion with Jason Lowe/Carlo Curino during community sync up and have a separate offline discussion with Sunil G.

          I was thinking solutions like dynamic creation of user-queue and assign quotas to these queues, etc. But it seems doesn't fit well for FIFO + user-quota. Also mentioned by Jason Lowe, dynamic queue exposes implementation details to end user.

          Carlo Curino proposed to use reservation system to solve the problem, but I'm not sure how, Carlo could you please add your thoughts?

          In my mind, the MULP (minimum-user-limit-percentage) is a quota applied to active users, which gives enough resource for app (or user which the app belongs to) to run. If there're more active users than (100 / MULP), scheduler tries to give resource to first (100 / MULP) users. If #active_users < (100 / MULP), available resource will be equally divided to these users.

          So I preferred to keep the semantic more similar to existing one, I propose to introduce a weight of users instead of overriding the MULP: scheduler will continue assign MULP% shares to each "unit users", but different user can have different weight to adjust quota based on share of "unit users". Also, the weights of users can be used independent from MULP: because in the future we may want to replace concept of user limit by different ones. (Like setting quota for each user, give weighted fair share to users, etc.)

          Also, regarding to config, there's a collision for <queue-name>.<user-name>.MULP when queue's name equals to <user-name>. How about adding one more layer to specify settings of users, like:

          ... <leaf-queue-name>.user-settings.<user-name>.weight = x
          ... <leaf-queue-name>.user-settings.<user-name>.weight = y
          ...
          

          ?

          Show
          leftnoteasy Wangda Tan added a comment - Thanks Eric Payne for the elaborations, I have a discussion with Jason Lowe / Carlo Curino during community sync up and have a separate offline discussion with Sunil G . I was thinking solutions like dynamic creation of user-queue and assign quotas to these queues, etc. But it seems doesn't fit well for FIFO + user-quota. Also mentioned by Jason Lowe , dynamic queue exposes implementation details to end user. Carlo Curino proposed to use reservation system to solve the problem, but I'm not sure how, Carlo could you please add your thoughts? In my mind, the MULP (minimum-user-limit-percentage) is a quota applied to active users, which gives enough resource for app (or user which the app belongs to) to run. If there're more active users than (100 / MULP), scheduler tries to give resource to first (100 / MULP) users. If #active_users < (100 / MULP), available resource will be equally divided to these users. So I preferred to keep the semantic more similar to existing one, I propose to introduce a weight of users instead of overriding the MULP: scheduler will continue assign MULP% shares to each "unit users", but different user can have different weight to adjust quota based on share of "unit users". Also, the weights of users can be used independent from MULP: because in the future we may want to replace concept of user limit by different ones. (Like setting quota for each user, give weighted fair share to users, etc.) Also, regarding to config, there's a collision for <queue-name>.<user-name>.MULP when queue's name equals to <user-name> . How about adding one more layer to specify settings of users, like: ... <leaf-queue-name>.user-settings.<user-name>.weight = x ... <leaf-queue-name>.user-settings.<user-name>.weight = y ... ?
          Hide
          eepayne Eric Payne added a comment -

          So I preferred to keep the semantic more similar to existing one, I propose to introduce a weight of users instead of overriding the MULP: scheduler will continue assign MULP% shares to each "unit users", but different user can have different weight to adjust quota based on share of "unit users". Also, the weights of users can be used independent from MULP: because in the future we may want to replace concept of user limit by different ones. (Like setting quota for each user, give weighted fair share to users, etc.)

          Thanks Wangda Tan for your review.

          In my mind, overriding queue's MULP with user-specific MULP is equivalent to adding weights to special users, and would be implemented in a similar way. If I understand correctly, you are saying that the weighted approach gives more flexibility for future features like user quota, weighted user fair share, etc. Is that correct?

          Show
          eepayne Eric Payne added a comment - So I preferred to keep the semantic more similar to existing one, I propose to introduce a weight of users instead of overriding the MULP: scheduler will continue assign MULP% shares to each "unit users", but different user can have different weight to adjust quota based on share of "unit users". Also, the weights of users can be used independent from MULP: because in the future we may want to replace concept of user limit by different ones. (Like setting quota for each user, give weighted fair share to users, etc.) Thanks Wangda Tan for your review. In my mind, overriding queue's MULP with user-specific MULP is equivalent to adding weights to special users, and would be implemented in a similar way. If I understand correctly, you are saying that the weighted approach gives more flexibility for future features like user quota, weighted user fair share, etc. Is that correct?
          Hide
          leftnoteasy Wangda Tan added a comment -

          In my mind, overriding queue's MULP with user-specific MULP is equivalent to adding weights to special users, and would be implemented in a similar way.

          Yeah it is similar, and actual it's inspired by what you mentioned above, weight=x means x unit users. Thus we will only have a identical MULP in the queue, but have different weights. Which in my mind is easier to be understood than overwriting MULP.

          If I understand correctly, you are saying that the weighted approach gives more flexibility for future features like user quota, weighted user fair share, etc. Is that correct?

          Yes because we may not want to continue MULP in the future, but we can continue use weight of users.

          Show
          leftnoteasy Wangda Tan added a comment - In my mind, overriding queue's MULP with user-specific MULP is equivalent to adding weights to special users, and would be implemented in a similar way. Yeah it is similar, and actual it's inspired by what you mentioned above, weight=x means x unit users. Thus we will only have a identical MULP in the queue, but have different weights. Which in my mind is easier to be understood than overwriting MULP. If I understand correctly, you are saying that the weighted approach gives more flexibility for future features like user quota, weighted user fair share, etc. Is that correct? Yes because we may not want to continue MULP in the future, but we can continue use weight of users.
          Hide
          eepayne Eric Payne added a comment -

          Thanks Wangda Tan and Sunil G for the reviews and valuable feedback. Attaching a new patch and would very much appreciate your reviews.

          I have refactored the feature to implement user-specific weights. Since each user has individual weights that could affect UL calculations for other users, I made the following user-specific:

          • private Map<String, Map<SchedulingMode, Long>> localVersionOfActiveUsersState became User#activeUsersState
          • private Map<String, Map<SchedulingMode, Long>> localVersionOfAllUsersState{{ became {{User#allUsersState
          • Map<String, Map<SchedulingMode, Resource>> preComputedActiveUserLimit became activeUserLimit
          • Map<String, Map<SchedulingMode, Resource>> preComputedAllUserLimit became allUserLimit

          I have added a weight column to the =Active User Info= section of each queue. In addition, I have added highlighting to users that have apps actively asking for resources. Please see the attached screenshot.

          Show
          eepayne Eric Payne added a comment - Thanks Wangda Tan and Sunil G for the reviews and valuable feedback. Attaching a new patch and would very much appreciate your reviews. I have refactored the feature to implement user-specific weights. Since each user has individual weights that could affect UL calculations for other users, I made the following user-specific: private Map<String, Map<SchedulingMode, Long>> localVersionOfActiveUsersState became User#activeUsersState private Map<String, Map<SchedulingMode, Long>> localVersionOfAllUsersState{{ became {{User#allUsersState Map<String, Map<SchedulingMode, Resource>> preComputedActiveUserLimit became activeUserLimit Map<String, Map<SchedulingMode, Resource>> preComputedAllUserLimit became allUserLimit I have added a weight column to the =Active User Info= section of each queue. In addition, I have added highlighting to users that have apps actively asking for resources. Please see the attached screenshot.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 23s 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.
          0 mvndep 0m 10s Maven dependency ordering for branch
          +1 mvninstall 15m 1s trunk passed
          -1 compile 5m 33s hadoop-yarn in trunk failed.
          +1 checkstyle 1m 2s trunk passed
          +1 mvnsite 1m 12s trunk passed
          +1 mvneclipse 0m 46s trunk passed
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
          +1 findbugs 1m 11s trunk passed
          +1 javadoc 0m 53s trunk passed
          0 mvndep 0m 9s Maven dependency ordering for patch
          +1 mvninstall 0m 40s the patch passed
          -1 compile 4m 32s hadoop-yarn in the patch failed.
          -1 javac 4m 32s hadoop-yarn in the patch failed.
          -0 checkstyle 1m 1s hadoop-yarn-project/hadoop-yarn: The patch generated 22 new + 545 unchanged - 8 fixed = 567 total (was 553)
          -1 mvnsite 0m 21s hadoop-yarn-site in the patch failed.
          +1 mvneclipse 0m 44s the patch passed
          +1 whitespace 0m 1s The patch has no whitespace issues.
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
          -1 findbugs 1m 21s 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 30s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 4 new + 897 unchanged - 0 fixed = 901 total (was 897)
          +1 unit 40m 4s hadoop-yarn-server-resourcemanager in the patch passed.
          +1 unit 0m 21s hadoop-yarn-site in the patch passed.
          +1 asflicense 0m 42s The patch does not generate ASF License warnings.
          86m 15s



          Reason Tests
          FindBugs module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
            Integral division result cast to double or float in org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.UsersManager.computeUserLimit(String, Resource, String, SchedulingMode, boolean) At UsersManager.java:double or float in org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.UsersManager.computeUserLimit(String, Resource, String, SchedulingMode, boolean) At UsersManager.java:[line 783]



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue YARN-5892
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12857425/YARN-5892.003.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux ebda1497c43f 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 229c7c9
          Default Java 1.8.0_121
          compile https://builds.apache.org/job/PreCommit-YARN-Build/15235/artifact/patchprocess/branch-compile-hadoop-yarn-project_hadoop-yarn.txt
          findbugs v3.0.0
          compile https://builds.apache.org/job/PreCommit-YARN-Build/15235/artifact/patchprocess/patch-compile-hadoop-yarn-project_hadoop-yarn.txt
          javac https://builds.apache.org/job/PreCommit-YARN-Build/15235/artifact/patchprocess/patch-compile-hadoop-yarn-project_hadoop-yarn.txt
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/15235/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
          mvnsite https://builds.apache.org/job/PreCommit-YARN-Build/15235/artifact/patchprocess/patch-mvnsite-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-site.txt
          findbugs https://builds.apache.org/job/PreCommit-YARN-Build/15235/artifact/patchprocess/new-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.html
          javadoc https://builds.apache.org/job/PreCommit-YARN-Build/15235/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/15235/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/15235/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT 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 23s 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. 0 mvndep 0m 10s Maven dependency ordering for branch +1 mvninstall 15m 1s trunk passed -1 compile 5m 33s hadoop-yarn in trunk failed. +1 checkstyle 1m 2s trunk passed +1 mvnsite 1m 12s trunk passed +1 mvneclipse 0m 46s trunk passed 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site +1 findbugs 1m 11s trunk passed +1 javadoc 0m 53s trunk passed 0 mvndep 0m 9s Maven dependency ordering for patch +1 mvninstall 0m 40s the patch passed -1 compile 4m 32s hadoop-yarn in the patch failed. -1 javac 4m 32s hadoop-yarn in the patch failed. -0 checkstyle 1m 1s hadoop-yarn-project/hadoop-yarn: The patch generated 22 new + 545 unchanged - 8 fixed = 567 total (was 553) -1 mvnsite 0m 21s hadoop-yarn-site in the patch failed. +1 mvneclipse 0m 44s the patch passed +1 whitespace 0m 1s The patch has no whitespace issues. 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site -1 findbugs 1m 21s 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 30s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 4 new + 897 unchanged - 0 fixed = 901 total (was 897) +1 unit 40m 4s hadoop-yarn-server-resourcemanager in the patch passed. +1 unit 0m 21s hadoop-yarn-site in the patch passed. +1 asflicense 0m 42s The patch does not generate ASF License warnings. 86m 15s Reason Tests FindBugs module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager   Integral division result cast to double or float in org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.UsersManager.computeUserLimit(String, Resource, String, SchedulingMode, boolean) At UsersManager.java:double or float in org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.UsersManager.computeUserLimit(String, Resource, String, SchedulingMode, boolean) At UsersManager.java: [line 783] Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue YARN-5892 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12857425/YARN-5892.003.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux ebda1497c43f 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 229c7c9 Default Java 1.8.0_121 compile https://builds.apache.org/job/PreCommit-YARN-Build/15235/artifact/patchprocess/branch-compile-hadoop-yarn-project_hadoop-yarn.txt findbugs v3.0.0 compile https://builds.apache.org/job/PreCommit-YARN-Build/15235/artifact/patchprocess/patch-compile-hadoop-yarn-project_hadoop-yarn.txt javac https://builds.apache.org/job/PreCommit-YARN-Build/15235/artifact/patchprocess/patch-compile-hadoop-yarn-project_hadoop-yarn.txt checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/15235/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt mvnsite https://builds.apache.org/job/PreCommit-YARN-Build/15235/artifact/patchprocess/patch-mvnsite-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-site.txt findbugs https://builds.apache.org/job/PreCommit-YARN-Build/15235/artifact/patchprocess/new-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.html javadoc https://builds.apache.org/job/PreCommit-YARN-Build/15235/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/15235/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/15235/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          leftnoteasy Wangda Tan added a comment -

          Thanks Eric Payne for updating the patch, haven't looked into details of the patch, a couple of high level questions/comments:

          1) Can this patch properly handle user limit updates when queue refreshes? I couldn't find related changes.
          2) In the existing patch it still changes user limit, and do normalize of user limit, calculate local user limit, etc. I'm not sure if following logic suggested can simplify the code:

          Since we have weights of users, scheduler can think number of active users (for active user limit) and number of total users (for total user limit) will be updated considering weights.
          For example, we have 3 users, u1 (weight=2.5), u2 (weight=1), u3 (weight=3). And u1/u2 are active users. Can we say there're 3.5 active users and 6.5 total users? With this we may not need to normalize user limit during computation.

          3) Can this patch properly handle cached per-user user-limit?
          Logics of YARN-5889 assumes user limit is identical across users, so it only needs to store pre-computed resource/version for partition/scheduling-mode. This patch may need to change this logic since different user can have different user-limit resource.
          Another option is to cache the "unit user"'s user-limit, with that we don't have to cache user-limit for different users. Instead, we have to multiply weight of user by computed user limit for "unit user" (when weight != 1).

          Show
          leftnoteasy Wangda Tan added a comment - Thanks Eric Payne for updating the patch, haven't looked into details of the patch, a couple of high level questions/comments: 1) Can this patch properly handle user limit updates when queue refreshes? I couldn't find related changes. 2) In the existing patch it still changes user limit, and do normalize of user limit, calculate local user limit, etc. I'm not sure if following logic suggested can simplify the code: Since we have weights of users, scheduler can think number of active users (for active user limit) and number of total users (for total user limit) will be updated considering weights. For example, we have 3 users, u1 (weight=2.5), u2 (weight=1), u3 (weight=3). And u1/u2 are active users. Can we say there're 3.5 active users and 6.5 total users? With this we may not need to normalize user limit during computation. 3) Can this patch properly handle cached per-user user-limit? Logics of YARN-5889 assumes user limit is identical across users, so it only needs to store pre-computed resource/version for partition/scheduling-mode. This patch may need to change this logic since different user can have different user-limit resource. Another option is to cache the "unit user"'s user-limit, with that we don't have to cache user-limit for different users. Instead, we have to multiply weight of user by computed user limit for "unit user" (when weight != 1).
          Hide
          eepayne Eric Payne added a comment -

          Thanks a lot, Wangda Tan, for your comments.

          1) Can this patch properly handle user limit updates when queue refreshes? I couldn't find related changes.

          I have done brief testing of refreshQueues, and this works for the use cases I've tested. My reasoning is that UsersManager#addUser always retrieves a fresh configuration from the CS Context:

          +    CapacitySchedulerConfiguration conf = csContext.getConfiguration();
          

          2) ... I'm not sure if following logic suggested can simplify the code:

          Thanks! I'm still thinking this through.

          3) Can this patch properly handle cached per-user user-limit?

          This patch has pushed the caching down to each User object.

          Show
          eepayne Eric Payne added a comment - Thanks a lot, Wangda Tan , for your comments. 1) Can this patch properly handle user limit updates when queue refreshes? I couldn't find related changes. I have done brief testing of refreshQueues , and this works for the use cases I've tested. My reasoning is that UsersManager#addUser always retrieves a fresh configuration from the CS Context: + CapacitySchedulerConfiguration conf = csContext.getConfiguration(); 2) ... I'm not sure if following logic suggested can simplify the code: Thanks! I'm still thinking this through. 3) Can this patch properly handle cached per-user user-limit? This patch has pushed the caching down to each User object.
          Hide
          leftnoteasy Wangda Tan added a comment -

          My reasoning is that UsersManager#addUser always retrieves a fresh configuration from the CS Context

          I'm not sure if this is enough, will it update existing users?

          Show
          leftnoteasy Wangda Tan added a comment - My reasoning is that UsersManager#addUser always retrieves a fresh configuration from the CS Context I'm not sure if this is enough, will it update existing users?
          Hide
          eepayne Eric Payne added a comment -

          Attaching new patch to address findbugs/checkstyle/compile/mvnsite issues. There's not much I can do about the javadoc issues, since they all address '_' used as an identifier.

          I'm not sure if this is enough, will it update existing users?

          Wangda Tan, that's a good point. I'll double check.

          Show
          eepayne Eric Payne added a comment - Attaching new patch to address findbugs/checkstyle/compile/mvnsite issues. There's not much I can do about the javadoc issues, since they all address '_' used as an identifier . I'm not sure if this is enough, will it update existing users? Wangda Tan , that's a good point. I'll double check.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 21s 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.
          0 mvndep 0m 12s Maven dependency ordering for branch
          +1 mvninstall 13m 48s trunk passed
          +1 compile 6m 38s trunk passed
          +1 checkstyle 1m 8s trunk passed
          +1 mvnsite 1m 24s trunk passed
          +1 mvneclipse 0m 50s trunk passed
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
          +1 findbugs 1m 23s trunk passed
          +1 javadoc 0m 58s trunk passed
          0 mvndep 0m 11s Maven dependency ordering for patch
          +1 mvninstall 0m 45s the patch passed
          +1 compile 7m 4s the patch passed
          +1 javac 7m 4s the patch passed
          -0 checkstyle 1m 9s hadoop-yarn-project/hadoop-yarn: The patch generated 13 new + 545 unchanged - 8 fixed = 558 total (was 553)
          +1 mvnsite 1m 24s the patch passed
          +1 mvneclipse 0m 47s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
          +1 findbugs 1m 45s the patch passed
          -1 javadoc 0m 36s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 4 new + 897 unchanged - 0 fixed = 901 total (was 897)
          -1 unit 42m 10s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 unit 0m 18s hadoop-yarn-site in the patch passed.
          +1 asflicense 0m 43s The patch does not generate ASF License warnings.
          92m 38s



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



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue YARN-5892
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12858890/YARN-5892.004.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 7f6babfb82af 3.13.0-105-generic #152-Ubuntu SMP Fri Dec 2 15:37:11 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / bb6a214
          Default Java 1.8.0_121
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/15288/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
          javadoc https://builds.apache.org/job/PreCommit-YARN-Build/15288/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/15288/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/15288/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/15288/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT 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. +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. 0 mvndep 0m 12s Maven dependency ordering for branch +1 mvninstall 13m 48s trunk passed +1 compile 6m 38s trunk passed +1 checkstyle 1m 8s trunk passed +1 mvnsite 1m 24s trunk passed +1 mvneclipse 0m 50s trunk passed 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site +1 findbugs 1m 23s trunk passed +1 javadoc 0m 58s trunk passed 0 mvndep 0m 11s Maven dependency ordering for patch +1 mvninstall 0m 45s the patch passed +1 compile 7m 4s the patch passed +1 javac 7m 4s the patch passed -0 checkstyle 1m 9s hadoop-yarn-project/hadoop-yarn: The patch generated 13 new + 545 unchanged - 8 fixed = 558 total (was 553) +1 mvnsite 1m 24s the patch passed +1 mvneclipse 0m 47s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site +1 findbugs 1m 45s the patch passed -1 javadoc 0m 36s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 4 new + 897 unchanged - 0 fixed = 901 total (was 897) -1 unit 42m 10s hadoop-yarn-server-resourcemanager in the patch failed. +1 unit 0m 18s hadoop-yarn-site in the patch passed. +1 asflicense 0m 43s The patch does not generate ASF License warnings. 92m 38s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.TestRMRestart Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue YARN-5892 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12858890/YARN-5892.004.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 7f6babfb82af 3.13.0-105-generic #152-Ubuntu SMP Fri Dec 2 15:37:11 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / bb6a214 Default Java 1.8.0_121 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/15288/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt javadoc https://builds.apache.org/job/PreCommit-YARN-Build/15288/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/15288/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/15288/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/15288/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          eepayne Eric Payne added a comment -

          The previous patch (004) only updated users' weights when each user was added. This patch (005) will update users' weights when the queues are refreshed.

          2) I'm not sure if following logic suggested can simplify the code:

          Wangda Tan, I have not forgotten your suggestion. I am investigating.

          Show
          eepayne Eric Payne added a comment - The previous patch ( 004 ) only updated users' weights when each user was added. This patch ( 005 ) will update users' weights when the queues are refreshed. 2) I'm not sure if following logic suggested can simplify the code: Wangda Tan , I have not forgotten your suggestion. I am investigating.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 17s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          0 mvndep 0m 49s Maven dependency ordering for branch
          +1 mvninstall 12m 34s trunk passed
          +1 compile 5m 46s trunk passed
          +1 checkstyle 1m 3s trunk passed
          +1 mvnsite 1m 11s trunk passed
          +1 mvneclipse 0m 47s trunk passed
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
          +1 findbugs 1m 10s trunk passed
          +1 javadoc 0m 53s trunk passed
          0 mvndep 0m 9s Maven dependency ordering for patch
          +1 mvninstall 0m 39s the patch passed
          +1 compile 6m 21s the patch passed
          +1 javac 6m 21s the patch passed
          -0 checkstyle 1m 3s hadoop-yarn-project/hadoop-yarn: The patch generated 16 new + 546 unchanged - 8 fixed = 562 total (was 554)
          +1 mvnsite 1m 9s the patch passed
          +1 mvneclipse 0m 44s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
          +1 findbugs 1m 28s the patch passed
          -1 javadoc 0m 34s hadoop-yarn-server-resourcemanager in the patch failed.
          -1 unit 40m 28s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 unit 0m 18s hadoop-yarn-site in the patch passed.
          +1 asflicense 0m 39s The patch does not generate ASF License warnings.
          87m 3s



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



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue YARN-5892
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12859309/YARN-5892.005.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux ea1fbcfeaef8 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 7536815
          Default Java 1.8.0_121
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/15311/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
          javadoc https://builds.apache.org/job/PreCommit-YARN-Build/15311/artifact/patchprocess/patch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/15311/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/15311/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/15311/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT 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 17s 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. 0 mvndep 0m 49s Maven dependency ordering for branch +1 mvninstall 12m 34s trunk passed +1 compile 5m 46s trunk passed +1 checkstyle 1m 3s trunk passed +1 mvnsite 1m 11s trunk passed +1 mvneclipse 0m 47s trunk passed 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site +1 findbugs 1m 10s trunk passed +1 javadoc 0m 53s trunk passed 0 mvndep 0m 9s Maven dependency ordering for patch +1 mvninstall 0m 39s the patch passed +1 compile 6m 21s the patch passed +1 javac 6m 21s the patch passed -0 checkstyle 1m 3s hadoop-yarn-project/hadoop-yarn: The patch generated 16 new + 546 unchanged - 8 fixed = 562 total (was 554) +1 mvnsite 1m 9s the patch passed +1 mvneclipse 0m 44s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site +1 findbugs 1m 28s the patch passed -1 javadoc 0m 34s hadoop-yarn-server-resourcemanager in the patch failed. -1 unit 40m 28s hadoop-yarn-server-resourcemanager in the patch failed. +1 unit 0m 18s hadoop-yarn-site in the patch passed. +1 asflicense 0m 39s The patch does not generate ASF License warnings. 87m 3s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.TestRMRestart Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue YARN-5892 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12859309/YARN-5892.005.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux ea1fbcfeaef8 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 7536815 Default Java 1.8.0_121 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/15311/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt javadoc https://builds.apache.org/job/PreCommit-YARN-Build/15311/artifact/patchprocess/patch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/15311/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/15311/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/15311/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          sunilg Sunil G added a comment -

          Sorry for late entry. I am also reviewing and will share my thoughts at earliest.

          Show
          sunilg Sunil G added a comment - Sorry for late entry. I am also reviewing and will share my thoughts at earliest.
          Hide
          sunilg Sunil G added a comment -

          Hi Eric Payne
          Thanks for working on this patch.

          I have some doubts which are inline with Wangdas comments as well.

          1. Do we need to store localVersionOfUsersState per user level. Currently we invalidate all user's limit if there is a semantic change (queue refresh, cluster resource change etc). Currently we invalidate per user level as per this patch, please correct me if I got that wrong. So if we invalidate per user level, how about other users in case of resource change/ config change etc.
          2. I could see that compureUserLimit method internally consider the user's weight. Do we need to do this way? Is it possible to keep existing computation as it is, and then from external caller side, we can multiply with specific user's weight with computed user-limit.

          Show
          sunilg Sunil G added a comment - Hi Eric Payne Thanks for working on this patch. I have some doubts which are inline with Wangdas comments as well. 1. Do we need to store localVersionOfUsersState per user level. Currently we invalidate all user's limit if there is a semantic change (queue refresh, cluster resource change etc). Currently we invalidate per user level as per this patch, please correct me if I got that wrong. So if we invalidate per user level, how about other users in case of resource change/ config change etc. 2. I could see that compureUserLimit method internally consider the user's weight. Do we need to do this way? Is it possible to keep existing computation as it is, and then from external caller side, we can multiply with specific user's weight with computed user-limit.
          Hide
          eepayne Eric Payne added a comment -

          Thanks for your comments, Sunil G.

          Is it possible to keep existing computation as it is, and then from external caller side, we can multiply with specific user's weight with computed user-limit.

          I think this was also Wangda Tan's point above:

          2) I'm not sure if following logic suggested can simplify the code

          I am investigating this and hope to have a patch up soon.

          Show
          eepayne Eric Payne added a comment - Thanks for your comments, Sunil G . Is it possible to keep existing computation as it is, and then from external caller side, we can multiply with specific user's weight with computed user-limit. I think this was also Wangda Tan 's point above: 2) I'm not sure if following logic suggested can simplify the code I am investigating this and hope to have a patch up soon.
          Hide
          eepayne Eric Payne added a comment -

          Thanks for your helpful comments Wangda Tan and Sunil G. I am attaching a new patch where the user limit computations are done as before but then the weight is applied afterwards. It is simpler.
          Thanks.

          Show
          eepayne Eric Payne added a comment - Thanks for your helpful comments Wangda Tan and Sunil G . I am attaching a new patch where the user limit computations are done as before but then the weight is applied afterwards. It is simpler. Thanks.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 18s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          0 mvndep 0m 8s Maven dependency ordering for branch
          +1 mvninstall 12m 47s trunk passed
          +1 compile 10m 6s trunk passed
          +1 checkstyle 0m 58s trunk passed
          +1 mvnsite 1m 0s trunk passed
          +1 mvneclipse 0m 36s trunk passed
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
          +1 findbugs 1m 3s trunk passed
          +1 javadoc 0m 43s trunk passed
          0 mvndep 0m 10s Maven dependency ordering for patch
          +1 mvninstall 0m 39s the patch passed
          +1 compile 9m 26s the patch passed
          +1 javac 9m 26s the patch passed
          -0 checkstyle 0m 58s hadoop-yarn-project/hadoop-yarn: The patch generated 16 new + 553 unchanged - 1 fixed = 569 total (was 554)
          +1 mvnsite 1m 1s the patch passed
          +1 mvneclipse 0m 33s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
          +1 findbugs 1m 23s the patch passed
          -1 javadoc 0m 27s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 4 new + 882 unchanged - 0 fixed = 886 total (was 882)
          +1 unit 40m 8s hadoop-yarn-server-resourcemanager in the patch passed.
          +1 unit 0m 13s hadoop-yarn-site in the patch passed.
          +1 asflicense 0m 32s The patch does not generate ASF License warnings.
          91m 41s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue YARN-5892
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12860457/YARN-5892.006.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 6935e8b416fc 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 1f66524
          Default Java 1.8.0_121
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/15382/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
          javadoc https://builds.apache.org/job/PreCommit-YARN-Build/15382/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/15382/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/15382/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 18s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 8s Maven dependency ordering for branch +1 mvninstall 12m 47s trunk passed +1 compile 10m 6s trunk passed +1 checkstyle 0m 58s trunk passed +1 mvnsite 1m 0s trunk passed +1 mvneclipse 0m 36s trunk passed 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site +1 findbugs 1m 3s trunk passed +1 javadoc 0m 43s trunk passed 0 mvndep 0m 10s Maven dependency ordering for patch +1 mvninstall 0m 39s the patch passed +1 compile 9m 26s the patch passed +1 javac 9m 26s the patch passed -0 checkstyle 0m 58s hadoop-yarn-project/hadoop-yarn: The patch generated 16 new + 553 unchanged - 1 fixed = 569 total (was 554) +1 mvnsite 1m 1s the patch passed +1 mvneclipse 0m 33s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site +1 findbugs 1m 23s the patch passed -1 javadoc 0m 27s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 4 new + 882 unchanged - 0 fixed = 886 total (was 882) +1 unit 40m 8s hadoop-yarn-server-resourcemanager in the patch passed. +1 unit 0m 13s hadoop-yarn-site in the patch passed. +1 asflicense 0m 32s The patch does not generate ASF License warnings. 91m 41s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue YARN-5892 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12860457/YARN-5892.006.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 6935e8b416fc 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 1f66524 Default Java 1.8.0_121 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/15382/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt javadoc https://builds.apache.org/job/PreCommit-YARN-Build/15382/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/15382/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/15382/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          leftnoteasy Wangda Tan added a comment -

          Thanks Eric Payne to update the patch.

          Several comments:

          1) For User weight:

          • I prefer to only allow specify it in the leaf queue level. Inheriting from parent causes confusing in other cases. (For example should we allow overwriting in leaf, etc.)
          • I think we should limit the user weight to be: User_weight <= 100 / MULP when validating the config. Otherwise if MULP = 100, and user X's weight = 3, by definition it can get guaranteed resource of 3 * queue configured resource, which is not good.

          2) LeafQueue:

          • getUserAMResourceLimitPerPartition(String) is not used by anyone, can be removed.

          3) UsersManager:
          I think logics in the computeUserLimit should be updated:

              int usersCount = getNumActiveUsers();
              Resource resourceUsed = totalResUsageForActiveUsers.getUsed(nodePartition);
          
              // For non-activeUser calculation, consider all users count.
              if (!activeUser) {
                resourceUsed = currentCapacity;
                usersCount = users.size();
              }
          

          Users_count should be a float and impacted by weight according to definition of weight (weight=x means equivalent to x users). For example, if a two users in the queue, one has weight=2, one has weight = 0.5, the users count should be 2.5. And we need get separately users_count_by_weight for all users and active users.

          4) updateUsersWeights:
          Should we move userLimitNeedsRecompute into the weight lock, otherwise it could cause a race condition.

          5) Could you update doc with more detailed example?

          Show
          leftnoteasy Wangda Tan added a comment - Thanks Eric Payne to update the patch. Several comments: 1) For User weight: I prefer to only allow specify it in the leaf queue level. Inheriting from parent causes confusing in other cases. (For example should we allow overwriting in leaf, etc.) I think we should limit the user weight to be: User_weight <= 100 / MULP when validating the config. Otherwise if MULP = 100, and user X's weight = 3, by definition it can get guaranteed resource of 3 * queue configured resource, which is not good. 2) LeafQueue: getUserAMResourceLimitPerPartition(String) is not used by anyone, can be removed. 3) UsersManager: I think logics in the computeUserLimit should be updated: int usersCount = getNumActiveUsers(); Resource resourceUsed = totalResUsageForActiveUsers.getUsed(nodePartition); // For non-activeUser calculation, consider all users count. if (!activeUser) { resourceUsed = currentCapacity; usersCount = users.size(); } Users_count should be a float and impacted by weight according to definition of weight (weight=x means equivalent to x users). For example, if a two users in the queue, one has weight=2, one has weight = 0.5, the users count should be 2.5. And we need get separately users_count_by_weight for all users and active users. 4) updateUsersWeights: Should we move userLimitNeedsRecompute into the weight lock, otherwise it could cause a race condition. 5) Could you update doc with more detailed example?
          Hide
          eepayne Eric Payne added a comment -

          Thanks Wangda Tan for your detailed review. I really appreciate it.

          I have a couple of clarifications:

          1) For User weight:I prefer to only allow specify it in the leaf queue level.

          I think it's good to allow setting user weights at parent-queue levels. This allows cluster admins to set it once per user per cluster instead of on multiple leaf queues.

          should we allow overwriting in leaf

          Yes, with the current implementing, settings in leaf queues would override those in parent queues.

          getUserAMResourceLimitPerPartition(String) is not used by anyone, can be removed.

          Since this was implemented as a public interface, I wanted to keep this API here for backwards compatibility, in case something outside Hadoop was depending on it. If this is not necessary, we can remove it.

          Should we move userLimitNeedsRecompute into the weight lock

          Did you mean "write lock"?

          Show
          eepayne Eric Payne added a comment - Thanks Wangda Tan for your detailed review. I really appreciate it. I have a couple of clarifications: 1) For User weight:I prefer to only allow specify it in the leaf queue level. I think it's good to allow setting user weights at parent-queue levels. This allows cluster admins to set it once per user per cluster instead of on multiple leaf queues. should we allow overwriting in leaf Yes, with the current implementing, settings in leaf queues would override those in parent queues. getUserAMResourceLimitPerPartition(String) is not used by anyone, can be removed. Since this was implemented as a public interface, I wanted to keep this API here for backwards compatibility, in case something outside Hadoop was depending on it. If this is not necessary, we can remove it. Should we move userLimitNeedsRecompute into the weight lock Did you mean " write lock"?
          Hide
          leftnoteasy Wangda Tan added a comment -

          Eric Payne,

          I think it's good to allow setting user weights at parent-queue levels. This allows cluster admins to set it once per user per cluster instead of on multiple leaf queues.

          And:

          Yes, with the current implementing, settings in leaf queues would override those in parent queues.

          Make sense to me

          Since this was implemented as a public interface, I wanted to keep this API here for backwards compatibility, in case something outside Hadoop was depending on it. If this is not necessary, we can remove it.

          Since the entire LeafQueue is marked by @private. I think we should remove it.

          Did you mean "write lock"?

          Yes

          Show
          leftnoteasy Wangda Tan added a comment - Eric Payne , I think it's good to allow setting user weights at parent-queue levels. This allows cluster admins to set it once per user per cluster instead of on multiple leaf queues. And: Yes, with the current implementing, settings in leaf queues would override those in parent queues. Make sense to me Since this was implemented as a public interface, I wanted to keep this API here for backwards compatibility, in case something outside Hadoop was depending on it. If this is not necessary, we can remove it. Since the entire LeafQueue is marked by @private . I think we should remove it. Did you mean "write lock"? Yes
          Hide
          eepayne Eric Payne added a comment -

          Wangda Tan,

          Attaching a YARN-5892.007.patch, that addresses all of the above comments.

          I have a couple of concerns, however.
          1) I took out the code that allows the weight property to be inherited from parent queues. I felt that it was inefficient (and maybe risky) because it parsed through the weight conf property string several times for each user. This patch requires the weight property to be set at the leaf queue only. If it's all the same to you, I would like to add the inheritance feature in a separate JIRA.
          2) The other concern is larger, I think. Currently, the weight is applied to the user limit in getComputedResourceLimitForActiveUsers. However, the logic that prevents the user limit from falling below the minimum user limit percent is done down at the computeUserLimit level. Which means that if a user's weight is < 1, the user's user limit will fall below the minimum user limit percent.

          I can think of a couple of ways to get around this, but I would like to get your opinion first. Also, CC-ing Jason Lowe

          Show
          eepayne Eric Payne added a comment - Wangda Tan , Attaching a YARN-5892 .007.patch, that addresses all of the above comments. I have a couple of concerns, however. 1) I took out the code that allows the weight property to be inherited from parent queues. I felt that it was inefficient (and maybe risky) because it parsed through the weight conf property string several times for each user. This patch requires the weight property to be set at the leaf queue only. If it's all the same to you, I would like to add the inheritance feature in a separate JIRA. 2) The other concern is larger, I think. Currently, the weight is applied to the user limit in getComputedResourceLimitForActiveUsers . However, the logic that prevents the user limit from falling below the minimum user limit percent is done down at the computeUserLimit level. Which means that if a user's weight is < 1, the user's user limit will fall below the minimum user limit percent. I can think of a couple of ways to get around this, but I would like to get your opinion first. Also, CC-ing Jason Lowe
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 23s 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.
          0 mvndep 0m 12s Maven dependency ordering for branch
          +1 mvninstall 16m 6s trunk passed
          -1 compile 3m 47s hadoop-yarn in trunk failed.
          +1 checkstyle 1m 8s trunk passed
          +1 mvnsite 1m 50s trunk passed
          +1 mvneclipse 0m 44s trunk passed
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
          +1 findbugs 2m 30s trunk passed
          +1 javadoc 1m 17s trunk passed
          0 mvndep 0m 12s Maven dependency ordering for patch
          +1 mvninstall 1m 35s the patch passed
          -1 compile 3m 41s hadoop-yarn in the patch failed.
          -1 javac 3m 41s hadoop-yarn in the patch failed.
          -0 checkstyle 1m 2s hadoop-yarn-project/hadoop-yarn: The patch generated 15 new + 572 unchanged - 1 fixed = 587 total (was 573)
          +1 mvnsite 1m 37s the patch passed
          +1 mvneclipse 0m 48s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
          +1 findbugs 2m 50s the patch passed
          -1 javadoc 0m 31s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 4 new + 880 unchanged - 0 fixed = 884 total (was 880)
          +1 unit 2m 51s hadoop-yarn-common in the patch passed.
          +1 unit 41m 33s hadoop-yarn-server-resourcemanager in the patch passed.
          +1 unit 0m 9s hadoop-yarn-site in the patch passed.
          +1 asflicense 0m 20s The patch does not generate ASF License warnings.
          94m 8s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue YARN-5892
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12861980/YARN-5892.007.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux bd81d395e33f 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 3fdae0a
          Default Java 1.8.0_121
          compile https://builds.apache.org/job/PreCommit-YARN-Build/15517/artifact/patchprocess/branch-compile-hadoop-yarn-project_hadoop-yarn.txt
          findbugs v3.0.0
          compile https://builds.apache.org/job/PreCommit-YARN-Build/15517/artifact/patchprocess/patch-compile-hadoop-yarn-project_hadoop-yarn.txt
          javac https://builds.apache.org/job/PreCommit-YARN-Build/15517/artifact/patchprocess/patch-compile-hadoop-yarn-project_hadoop-yarn.txt
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/15517/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
          javadoc https://builds.apache.org/job/PreCommit-YARN-Build/15517/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/15517/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/15517/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT 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 23s 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. 0 mvndep 0m 12s Maven dependency ordering for branch +1 mvninstall 16m 6s trunk passed -1 compile 3m 47s hadoop-yarn in trunk failed. +1 checkstyle 1m 8s trunk passed +1 mvnsite 1m 50s trunk passed +1 mvneclipse 0m 44s trunk passed 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site +1 findbugs 2m 30s trunk passed +1 javadoc 1m 17s trunk passed 0 mvndep 0m 12s Maven dependency ordering for patch +1 mvninstall 1m 35s the patch passed -1 compile 3m 41s hadoop-yarn in the patch failed. -1 javac 3m 41s hadoop-yarn in the patch failed. -0 checkstyle 1m 2s hadoop-yarn-project/hadoop-yarn: The patch generated 15 new + 572 unchanged - 1 fixed = 587 total (was 573) +1 mvnsite 1m 37s the patch passed +1 mvneclipse 0m 48s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site +1 findbugs 2m 50s the patch passed -1 javadoc 0m 31s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 4 new + 880 unchanged - 0 fixed = 884 total (was 880) +1 unit 2m 51s hadoop-yarn-common in the patch passed. +1 unit 41m 33s hadoop-yarn-server-resourcemanager in the patch passed. +1 unit 0m 9s hadoop-yarn-site in the patch passed. +1 asflicense 0m 20s The patch does not generate ASF License warnings. 94m 8s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue YARN-5892 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12861980/YARN-5892.007.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux bd81d395e33f 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 3fdae0a Default Java 1.8.0_121 compile https://builds.apache.org/job/PreCommit-YARN-Build/15517/artifact/patchprocess/branch-compile-hadoop-yarn-project_hadoop-yarn.txt findbugs v3.0.0 compile https://builds.apache.org/job/PreCommit-YARN-Build/15517/artifact/patchprocess/patch-compile-hadoop-yarn-project_hadoop-yarn.txt javac https://builds.apache.org/job/PreCommit-YARN-Build/15517/artifact/patchprocess/patch-compile-hadoop-yarn-project_hadoop-yarn.txt checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/15517/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt javadoc https://builds.apache.org/job/PreCommit-YARN-Build/15517/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/15517/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/15517/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          leftnoteasy Wangda Tan added a comment -

          Thanks Eric Payne,

          Haven't reviewed your latest patch, regarding to your previous comment:

          I would like to add the inheritance feature in a separate JIRA.

          Agree

          Which means that if a user's weight is < 1, the user's user limit will fall below the minimum user limit percent.

          To me, YARN can ensure minimum weight of each user to >= 1. I agree with you that too small user limit can cause many issues like only one container can be allocated, etc. If limiting minimum weight >= 1 is an option to you, let's limit that for now.

          Show
          leftnoteasy Wangda Tan added a comment - Thanks Eric Payne , Haven't reviewed your latest patch, regarding to your previous comment: I would like to add the inheritance feature in a separate JIRA. Agree Which means that if a user's weight is < 1, the user's user limit will fall below the minimum user limit percent. To me, YARN can ensure minimum weight of each user to >= 1. I agree with you that too small user limit can cause many issues like only one container can be allocated, etc. If limiting minimum weight >= 1 is an option to you, let's limit that for now.
          Hide
          eepayne Eric Payne added a comment -

          Build failure seems unrelated:

          [ERROR] Failed to execute goal com.github.eirslett:frontend-maven-plugin:0.0.22:install-node-and-npm (install node and npm) on project hadoop-yarn-ui: Could not download Node.js: Got error code 522 from the server. -> [Help 1]
          

          I re-kicked the YARN precommit build.

          Which means that if a user's weight is < 1, the user's user limit will fall below the minimum user limit percent.

          To me, YARN can ensure minimum weight of each user to >= 1.

          My concern about this approach is that if we really do have a user that should have a weight that is 0.5 compared to all of the other users in the queue, we would have to set all of the other users' weights to 2, and then remember to set new users' weights whenever they are added to that queue. We may have a use case for this, actually.

          I discussed this with Jason Lowe offline. I think that if a user is really supposed to be less than the other users, it's acceptable for that user's user limit to be less than the MULP as long as that does not ever prevent that user from using the whole queue if they are the only user. I will investigate further.

          Show
          eepayne Eric Payne added a comment - Build failure seems unrelated: [ERROR] Failed to execute goal com.github.eirslett:frontend-maven-plugin:0.0.22:install-node-and-npm (install node and npm) on project hadoop-yarn-ui: Could not download Node.js: Got error code 522 from the server. -> [Help 1] I re-kicked the YARN precommit build. Which means that if a user's weight is < 1, the user's user limit will fall below the minimum user limit percent. To me, YARN can ensure minimum weight of each user to >= 1. My concern about this approach is that if we really do have a user that should have a weight that is 0.5 compared to all of the other users in the queue, we would have to set all of the other users' weights to 2, and then remember to set new users' weights whenever they are added to that queue. We may have a use case for this, actually. I discussed this with Jason Lowe offline. I think that if a user is really supposed to be less than the other users, it's acceptable for that user's user limit to be less than the MULP as long as that does not ever prevent that user from using the whole queue if they are the only user. I will investigate further.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 21s 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.
          0 mvndep 0m 11s Maven dependency ordering for branch
          +1 mvninstall 15m 45s trunk passed
          +1 compile 12m 1s trunk passed
          +1 checkstyle 1m 4s trunk passed
          +1 mvnsite 1m 49s trunk passed
          +1 mvneclipse 1m 2s trunk passed
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
          +1 findbugs 2m 42s trunk passed
          +1 javadoc 1m 31s trunk passed
          0 mvndep 0m 12s Maven dependency ordering for patch
          +1 mvninstall 1m 31s the patch passed
          +1 compile 10m 8s the patch passed
          +1 javac 10m 8s the patch passed
          -0 checkstyle 1m 7s hadoop-yarn-project/hadoop-yarn: The patch generated 15 new + 572 unchanged - 1 fixed = 587 total (was 573)
          +1 mvnsite 1m 59s the patch passed
          +1 mvneclipse 0m 58s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
          +1 findbugs 3m 16s the patch passed
          -1 javadoc 0m 30s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 4 new + 880 unchanged - 0 fixed = 884 total (was 880)
          +1 unit 2m 36s hadoop-yarn-common in the patch passed.
          -1 unit 40m 16s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 unit 0m 14s hadoop-yarn-site in the patch passed.
          +1 asflicense 0m 32s The patch does not generate ASF License warnings.
          109m 13s



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



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue YARN-5892
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12861980/YARN-5892.007.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux b3944c8b3c35 3.13.0-105-generic #152-Ubuntu SMP Fri Dec 2 15:37:11 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 34ab8e7
          Default Java 1.8.0_121
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/15526/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
          javadoc https://builds.apache.org/job/PreCommit-YARN-Build/15526/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/15526/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/15526/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/15526/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT 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. +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. 0 mvndep 0m 11s Maven dependency ordering for branch +1 mvninstall 15m 45s trunk passed +1 compile 12m 1s trunk passed +1 checkstyle 1m 4s trunk passed +1 mvnsite 1m 49s trunk passed +1 mvneclipse 1m 2s trunk passed 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site +1 findbugs 2m 42s trunk passed +1 javadoc 1m 31s trunk passed 0 mvndep 0m 12s Maven dependency ordering for patch +1 mvninstall 1m 31s the patch passed +1 compile 10m 8s the patch passed +1 javac 10m 8s the patch passed -0 checkstyle 1m 7s hadoop-yarn-project/hadoop-yarn: The patch generated 15 new + 572 unchanged - 1 fixed = 587 total (was 573) +1 mvnsite 1m 59s the patch passed +1 mvneclipse 0m 58s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site +1 findbugs 3m 16s the patch passed -1 javadoc 0m 30s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 4 new + 880 unchanged - 0 fixed = 884 total (was 880) +1 unit 2m 36s hadoop-yarn-common in the patch passed. -1 unit 40m 16s hadoop-yarn-server-resourcemanager in the patch failed. +1 unit 0m 14s hadoop-yarn-site in the patch passed. +1 asflicense 0m 32s The patch does not generate ASF License warnings. 109m 13s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.TestRMRestart Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue YARN-5892 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12861980/YARN-5892.007.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux b3944c8b3c35 3.13.0-105-generic #152-Ubuntu SMP Fri Dec 2 15:37:11 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 34ab8e7 Default Java 1.8.0_121 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/15526/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt javadoc https://builds.apache.org/job/PreCommit-YARN-Build/15526/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/15526/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/15526/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/15526/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          eepayne Eric Payne added a comment -

          There are a couple of notable things about this patch:

          • A user's weight can be less than 1.0. When a user's weight is < 1.0, Max Resources looks as expected on the scheduler GUI when all users are requesting resources. However, when user(s) with weithg < 1 are asking and others are not, the Max Resource value is not accurate for the non-active users. For example, in a queue with 20GB, User2 has a weight of 0.2 and User5 has a weight of 1.0. When both are active, the weights and used resources for both are part of the Max Resources calculations, as below:
            User Name Max Resource Weight Used Resource
            User2 <memory:1536, vCores:1> 0.2 <memory:7168, vCores:14>
            User5 <memory:7680, vCores:1> 1.0 <memory:2048, vCores:4>

            However, when User5 stops requsting resources, its Max Resources becomes a factor of User2's Max Resources:

            User Name Max Resource Weight Used Resource
            User2 <memory:7168, vCores:1> 0.2 <memory:7168, vCores:14>
            User5 <memory:35840, vCores:1> 1.0 <memory:2560, vCores:5>

            Note that the queue only has 20GB. This is odd, but I don't view this as a problem since it is evident which users are asking for resources (see attached screenshot) and the Max Resources is only applicable when a user is active.

          • User Limit Factor is also affected by user weights. If User1 has a weight of 0.5 and a queue's User Limit Factor is 1.0, User1 can only use half of the queue.
          • A user weighted at 0 will be granted the AM container, but no more.
          • We have an addition use case that this feature doesn't cover, but it will need to wait for another JIRA. We need to allow a user to be weighted very small but that will not be limited by the weighted user limit factor so that it can use the whole queue.
          Show
          eepayne Eric Payne added a comment - There are a couple of notable things about this patch: A user's weight can be less than 1.0. When a user's weight is < 1.0, Max Resources looks as expected on the scheduler GUI when all users are requesting resources. However, when user(s) with weithg < 1 are asking and others are not, the Max Resource value is not accurate for the non-active users. For example, in a queue with 20GB, User2 has a weight of 0.2 and User5 has a weight of 1.0. When both are active, the weights and used resources for both are part of the Max Resources calculations, as below: User Name Max Resource Weight Used Resource User2 <memory:1536, vCores:1> 0.2 <memory:7168, vCores:14> User5 <memory:7680, vCores:1> 1.0 <memory:2048, vCores:4> However, when User5 stops requsting resources, its Max Resources becomes a factor of User2 's Max Resources : User Name Max Resource Weight Used Resource User2 <memory:7168, vCores:1> 0.2 <memory:7168, vCores:14> User5 <memory:35840, vCores:1> 1.0 <memory:2560, vCores:5> Note that the queue only has 20GB. This is odd, but I don't view this as a problem since it is evident which users are asking for resources (see attached screenshot) and the Max Resources is only applicable when a user is active. User Limit Factor is also affected by user weights. If User1 has a weight of 0.5 and a queue's User Limit Factor is 1.0, User1 can only use half of the queue. A user weighted at 0 will be granted the AM container, but no more. We have an addition use case that this feature doesn't cover, but it will need to wait for another JIRA. We need to allow a user to be weighted very small but that will not be limited by the weighted user limit factor so that it can use the whole queue.
          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.
          0 mvndep 0m 10s Maven dependency ordering for branch
          +1 mvninstall 13m 19s trunk passed
          +1 compile 10m 31s trunk passed
          +1 checkstyle 0m 57s trunk passed
          +1 mvnsite 1m 38s trunk passed
          +1 mvneclipse 1m 1s trunk passed
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
          +1 findbugs 2m 8s trunk passed
          +1 javadoc 1m 19s trunk passed
          0 mvndep 0m 9s Maven dependency ordering for patch
          +1 mvninstall 1m 8s the patch passed
          +1 compile 9m 19s the patch passed
          +1 javac 9m 19s the patch passed
          -0 checkstyle 0m 57s hadoop-yarn-project/hadoop-yarn: The patch generated 16 new + 572 unchanged - 1 fixed = 588 total (was 573)
          +1 mvnsite 1m 37s the patch passed
          +1 mvneclipse 0m 59s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
          +1 findbugs 2m 22s the patch passed
          -1 javadoc 0m 27s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 4 new + 880 unchanged - 0 fixed = 884 total (was 880)
          +1 unit 2m 29s hadoop-yarn-common in the patch passed.
          -1 unit 38m 30s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 unit 0m 15s hadoop-yarn-site in the patch passed.
          +1 asflicense 0m 32s The patch does not generate ASF License warnings.
          99m 37s



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



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:612578f
          JIRA Issue YARN-5892
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12862596/YARN-5892.008.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux fcbde4bf5f54 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 5d38504
          Default Java 1.8.0_121
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/15563/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
          javadoc https://builds.apache.org/job/PreCommit-YARN-Build/15563/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/15563/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/15563/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/15563/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT 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. 0 mvndep 0m 10s Maven dependency ordering for branch +1 mvninstall 13m 19s trunk passed +1 compile 10m 31s trunk passed +1 checkstyle 0m 57s trunk passed +1 mvnsite 1m 38s trunk passed +1 mvneclipse 1m 1s trunk passed 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site +1 findbugs 2m 8s trunk passed +1 javadoc 1m 19s trunk passed 0 mvndep 0m 9s Maven dependency ordering for patch +1 mvninstall 1m 8s the patch passed +1 compile 9m 19s the patch passed +1 javac 9m 19s the patch passed -0 checkstyle 0m 57s hadoop-yarn-project/hadoop-yarn: The patch generated 16 new + 572 unchanged - 1 fixed = 588 total (was 573) +1 mvnsite 1m 37s the patch passed +1 mvneclipse 0m 59s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site +1 findbugs 2m 22s the patch passed -1 javadoc 0m 27s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 4 new + 880 unchanged - 0 fixed = 884 total (was 880) +1 unit 2m 29s hadoop-yarn-common in the patch passed. -1 unit 38m 30s hadoop-yarn-server-resourcemanager in the patch failed. +1 unit 0m 15s hadoop-yarn-site in the patch passed. +1 asflicense 0m 32s The patch does not generate ASF License warnings. 99m 37s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.TestRMRestart Subsystem Report/Notes Docker Image:yetus/hadoop:612578f JIRA Issue YARN-5892 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12862596/YARN-5892.008.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux fcbde4bf5f54 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 5d38504 Default Java 1.8.0_121 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/15563/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt javadoc https://builds.apache.org/job/PreCommit-YARN-Build/15563/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/15563/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/15563/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/15563/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          eepayne Eric Payne added a comment -
          User Name Max Resource Weight Used Resource
          User2 <memory:7168, vCores:1> 0.2 <memory:7168, vCores:14>
          User5 <memory:35840, vCores:1> 1.0 <memory:2560, vCores:5>

          Note that the queue only has 20GB. This is odd, but I don't view this as a problem...

          I think I know why it does this. It may be because of something I tried to "fix".

          Show
          eepayne Eric Payne added a comment - User Name Max Resource Weight Used Resource User2 <memory:7168, vCores:1> 0.2 <memory:7168, vCores:14> User5 <memory:35840, vCores:1> 1.0 <memory:2560, vCores:5> Note that the queue only has 20GB. This is odd, but I don't view this as a problem... I think I know why it does this. It may be because of something I tried to "fix".
          Hide
          sunilg Sunil G added a comment -

          Thanks Eric Payne.

          Few comments from end:

          1. getNumActive/AllWeightedUsers is iterating on all users now. It could be optimized by considering only weighted users.
          2. Given we have only one user in cluster, and we gave UL < 1 for that user. This will be increase the defined UserLimit for that user, because getNumActive/AllWeightedUsers will give a count < 1 in this case. Correct?
          3. I think user_weight=0 is tough to go in with existing nomenclatures or usages. If AM container is huge, still we may give resource for that user.
          Show
          sunilg Sunil G added a comment - Thanks Eric Payne . Few comments from end: getNumActive/AllWeightedUsers is iterating on all users now. It could be optimized by considering only weighted users. Given we have only one user in cluster, and we gave UL < 1 for that user. This will be increase the defined UserLimit for that user, because getNumActive/AllWeightedUsers will give a count < 1 in this case. Correct? I think user_weight=0 is tough to go in with existing nomenclatures or usages. If AM container is huge, still we may give resource for that user.
          Hide
          eepayne Eric Payne added a comment -

          Thanks Sunil G for your comments.

          1. getNumActive/AllWeightedUsers is iterating on all users now. It could be optimized by considering only weighted users.

          The method is probably named badly. This method really does need to iterate over all users to determine the sum of all the weights. There may be optimizations possible, but since these methods are only called from computeUserLimit, they are only recomputed when necessary. I renamed the methods to countActiveUsersTimesWeights and countAllUsersTimesWeights.

          2.Given we have only one user in cluster, and we gave UL < 1 for that user. This will be increase the defined UserLimit for that user, because getNumActive/AllWeightedUsers will give a count < 1 in this case. Correct?

          The return value of computeUserLimit is userLimitResource. If the sum of active user(s)'s weight(s) is < 1, then it is true that userLimitResource is larger than the actual number of resources used, sometimes much larger. However, this works out fine because the only methods that ever use the value of userLimitResource are getComputedResourceLimitFor[Active|All]Users. These methods then multiply the value of userLimitResource by the appropriate user weight(s), which will result in the correct value of userLimit for that specific user.

          3.I think user_weight=0 is tough to go in with existing nomenclatures or usages. If AM container is huge, still we may give resource for that user.

          Yes, the user weight of 0 case is a special one that we would like to explore, but certainly not in this JIRA. Weight of 0 would have a special meaning that although the user will only be given resources if no one else is asking, that user can still use the whole queue.

          Show
          eepayne Eric Payne added a comment - Thanks Sunil G for your comments. 1. getNumActive/AllWeightedUsers is iterating on all users now. It could be optimized by considering only weighted users. The method is probably named badly. This method really does need to iterate over all users to determine the sum of all the weights. There may be optimizations possible, but since these methods are only called from computeUserLimit , they are only recomputed when necessary. I renamed the methods to countActiveUsersTimesWeights and countAllUsersTimesWeights . 2.Given we have only one user in cluster, and we gave UL < 1 for that user. This will be increase the defined UserLimit for that user, because getNumActive/AllWeightedUsers will give a count < 1 in this case. Correct? The return value of computeUserLimit is userLimitResource . If the sum of active user(s)'s weight(s) is < 1, then it is true that userLimitResource is larger than the actual number of resources used, sometimes much larger. However, this works out fine because the only methods that ever use the value of userLimitResource are getComputedResourceLimitFor[Active|All]Users . These methods then multiply the value of userLimitResource by the appropriate user weight(s), which will result in the correct value of userLimit for that specific user. 3.I think user_weight=0 is tough to go in with existing nomenclatures or usages. If AM container is huge, still we may give resource for that user. Yes, the user weight of 0 case is a special one that we would like to explore, but certainly not in this JIRA. Weight of 0 would have a special meaning that although the user will only be given resources if no one else is asking, that user can still use the whole queue.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 16s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          0 mvndep 0m 9s Maven dependency ordering for branch
          +1 mvninstall 14m 2s trunk passed
          +1 compile 11m 31s trunk passed
          +1 checkstyle 1m 0s trunk passed
          +1 mvnsite 1m 45s trunk passed
          +1 mvneclipse 1m 1s trunk passed
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
          +1 findbugs 2m 22s trunk passed
          +1 javadoc 1m 16s trunk passed
          0 mvndep 0m 11s Maven dependency ordering for patch
          +1 mvninstall 1m 21s the patch passed
          +1 compile 9m 10s the patch passed
          +1 javac 9m 10s the patch passed
          -0 checkstyle 0m 58s hadoop-yarn-project/hadoop-yarn: The patch generated 15 new + 572 unchanged - 1 fixed = 587 total (was 573)
          +1 mvnsite 1m 48s the patch passed
          +1 mvneclipse 0m 58s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
          +1 findbugs 2m 29s the patch passed
          -1 javadoc 0m 27s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 4 new + 880 unchanged - 0 fixed = 884 total (was 880)
          +1 unit 2m 24s hadoop-yarn-common in the patch passed.
          +1 unit 41m 1s hadoop-yarn-server-resourcemanager in the patch passed.
          +1 unit 0m 15s hadoop-yarn-site in the patch passed.
          +1 asflicense 0m 37s The patch does not generate ASF License warnings.
          104m 23s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:612578f
          JIRA Issue YARN-5892
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12863083/YARN-5892.009.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 7255905f467d 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / a16ab2b
          Default Java 1.8.0_121
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/15606/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
          javadoc https://builds.apache.org/job/PreCommit-YARN-Build/15606/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/15606/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/15606/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT 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. +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. 0 mvndep 0m 9s Maven dependency ordering for branch +1 mvninstall 14m 2s trunk passed +1 compile 11m 31s trunk passed +1 checkstyle 1m 0s trunk passed +1 mvnsite 1m 45s trunk passed +1 mvneclipse 1m 1s trunk passed 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site +1 findbugs 2m 22s trunk passed +1 javadoc 1m 16s trunk passed 0 mvndep 0m 11s Maven dependency ordering for patch +1 mvninstall 1m 21s the patch passed +1 compile 9m 10s the patch passed +1 javac 9m 10s the patch passed -0 checkstyle 0m 58s hadoop-yarn-project/hadoop-yarn: The patch generated 15 new + 572 unchanged - 1 fixed = 587 total (was 573) +1 mvnsite 1m 48s the patch passed +1 mvneclipse 0m 58s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site +1 findbugs 2m 29s the patch passed -1 javadoc 0m 27s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 4 new + 880 unchanged - 0 fixed = 884 total (was 880) +1 unit 2m 24s hadoop-yarn-common in the patch passed. +1 unit 41m 1s hadoop-yarn-server-resourcemanager in the patch passed. +1 unit 0m 15s hadoop-yarn-site in the patch passed. +1 asflicense 0m 37s The patch does not generate ASF License warnings. 104m 23s Subsystem Report/Notes Docker Image:yetus/hadoop:612578f JIRA Issue YARN-5892 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12863083/YARN-5892.009.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 7255905f467d 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / a16ab2b Default Java 1.8.0_121 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/15606/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt javadoc https://builds.apache.org/job/PreCommit-YARN-Build/15606/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/15606/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/15606/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          eepayne Eric Payne added a comment - - edited

          By the way, the latest patch reverts the behavior of what is displayed in the Max Resource column in the Capacity Scheduler GUI to the previous behavior. The last active value is stored and displayed rather than being recalculated based on current active users and resources.

          • YARN-5892.008.patch (odd behavior) (User 2 is active):
            User Name Max Resource Weight Used Resource

            User2

            <memory:7168, vCores:1>

            0.2

            <memory:7168, vCores:14>

            User5 <memory:35840, vCores:1> 1.0 <memory:2560, vCores:5>
          • YARN-5892.009.patch (sane behavior) (user2 is active):
            User Name Max Resource Weight Used Resource

            User2

            <memory:7168, vCores:1>

            0.2

            <memory:7168, vCores:14>

            User5 <memory:7168, vCores:1> 1.0 <memory:2560, vCores:5>
          Show
          eepayne Eric Payne added a comment - - edited By the way, the latest patch reverts the behavior of what is displayed in the Max Resource column in the Capacity Scheduler GUI to the previous behavior. The last active value is stored and displayed rather than being recalculated based on current active users and resources. YARN-5892 .008.patch (odd behavior) (User 2 is active): User Name Max Resource Weight Used Resource User2 <memory:7168, vCores:1> 0.2 <memory:7168, vCores:14> User5 <memory:35840, vCores:1> 1.0 <memory:2560, vCores:5> YARN-5892 .009.patch (sane behavior) (user2 is active): User Name Max Resource Weight Used Resource User2 <memory:7168, vCores:1> 0.2 <memory:7168, vCores:14> User5 <memory:7168, vCores:1> 1.0 <memory:2560, vCores:5>
          Hide
          sunilg Sunil G added a comment -

          Yes, the user weight of 0 case is a special one that we would like to explore, but certainly not in this JIRA.

          Yes, we could discuss more about the definition in separate jira. For now, could we avoid taking 0 as a valid configuration.

          These methods then multiply the value of userLimitResource by the appropriate user weight(s), which will result in the correct value of userLimit for that specific user

          Sure. Since we multiply again later, we can get the same userlimit back. A value less than one as weight is going to be trickier but we are rounding off to minimumAllocation. So i think its fine given patch have logs and UI.

          495	    if (activeUsersSet.contains(userName)) {
          496	      user.setUserResourceLimit(userSpecificUserLimit);
          497      }
          

          If activeUsersSet has userName, i think user cant be null. Correct?

          Show
          sunilg Sunil G added a comment - Yes, the user weight of 0 case is a special one that we would like to explore, but certainly not in this JIRA. Yes, we could discuss more about the definition in separate jira. For now, could we avoid taking 0 as a valid configuration. These methods then multiply the value of userLimitResource by the appropriate user weight(s), which will result in the correct value of userLimit for that specific user Sure. Since we multiply again later, we can get the same userlimit back. A value less than one as weight is going to be trickier but we are rounding off to minimumAllocation. So i think its fine given patch have logs and UI. 495 if (activeUsersSet.contains(userName)) { 496 user.setUserResourceLimit(userSpecificUserLimit); 497 } If activeUsersSet has userName, i think user cant be null. Correct?
          Hide
          eepayne Eric Payne added a comment -

          Thanks a lot Sunil G for your review and comments.

          495	    if (activeUsersSet.contains(userName)) {
          496	      user.setUserResourceLimit(userSpecificUserLimit);
          497      }
          

          If activeUsersSet has userName, i think user cant be null. Correct?

          Correct. A check for null user could be added, just for safety, but I think it would be redundant.

          Show
          eepayne Eric Payne added a comment - Thanks a lot Sunil G for your review and comments. 495 if (activeUsersSet.contains(userName)) { 496 user.setUserResourceLimit(userSpecificUserLimit); 497 } If activeUsersSet has userName, i think user cant be null. Correct? Correct. A check for null user could be added, just for safety, but I think it would be redundant.
          Hide
          eepayne Eric Payne added a comment -

          Yes, we could discuss more about the definition in separate jira. For now, could we avoid taking 0 as a valid configuration.

          Having 0 as a user weight doesn't hurt anything except the user is never assigned resources. However, I can never think of a use case where <0 would be allowed. I can add a check for <=0, return value of 1.0 for the weight, and log a warning.

          Show
          eepayne Eric Payne added a comment - Yes, we could discuss more about the definition in separate jira. For now, could we avoid taking 0 as a valid configuration. Having 0 as a user weight doesn't hurt anything except the user is never assigned resources. However, I can never think of a use case where <0 would be allowed. I can add a check for <=0, return value of 1.0 for the weight, and log a warning.
          Hide
          eepayne Eric Payne added a comment -

          Having 0 as a user weight doesn't hurt anything except the user is never assigned resources. ... I can add a check for <=0, return value of 1.0 for the weight, and log a warning.

          After thinking more about it, allowing 0 weight seems consistent to me. If weight is 0, the capacity scheduler will assign 1 container to the user since it gives 1 more than the user limit.

          I would rather allow 0 and give a warning for <0.

          Sunil G, Wangda Tan, Jason Lowe, Nathan Roberts, thoughts?

          Show
          eepayne Eric Payne added a comment - Having 0 as a user weight doesn't hurt anything except the user is never assigned resources. ... I can add a check for <=0, return value of 1.0 for the weight, and log a warning. After thinking more about it, allowing 0 weight seems consistent to me. If weight is 0, the capacity scheduler will assign 1 container to the user since it gives 1 more than the user limit. I would rather allow 0 and give a warning for <0. Sunil G , Wangda Tan , Jason Lowe , Nathan Roberts , thoughts?
          Hide
          leftnoteasy Wangda Tan added a comment -

          Thanks Eric Payne for updating the patch, got some time to check the latest patch and caught up with discussions.

          There're several concerns regarding to user-limit < 1:
          1) When there're several active users with weight < 1. For example in a queue, UL = 1, and u1.weight=0.1, u2.weight=0.2. If there're only two apps, a1 from u1 and a2 from u2 running in the queue and both are requesting a lot of resource, ideally we should get u1 has 33% of the queue's guaranteed capacity and u2 has 66% of the queue's guaranteed capacity. However in this implementation (just from my understanding, haven't tested yet). a1 can get all queue's resource (because #active-user-applied-weights = 1/0.3) while a2 got starved.
          2) I would like to prevent setting user's weight to <= 0. It doesn't have clear semantics and could cause many other computation issues - printing a warning is not enough to me.

          Generally speaking, set user weight < 1 is a reasonable requirement however I don't think we're ready for that. It looks there're bunch of things we need to do to make #2 and related preemption logic works properly. I would suggest to not allow this for now and add a note in the doc saying that this could be supported in the future to avoid incompatible behavior changes. Thoughts?

          Beyond that, I suggest to make #active-users-times-weight can updated in O(1) for every changes to active users set or any active user's weight get updated. We saw in some extreme cases a queue can have several hundreds of users so change to active users set could be quite frequent.

          Also, weight of users applies to hard limit of user (user limit factor) as well. This is a gray area to me, since it may cause some issue of resource planning (one more factor apply to maximum resource of user). Would like to hear thoughts from Jason Lowe/Sunil G as well.

          Show
          leftnoteasy Wangda Tan added a comment - Thanks Eric Payne for updating the patch, got some time to check the latest patch and caught up with discussions. There're several concerns regarding to user-limit < 1: 1) When there're several active users with weight < 1. For example in a queue, UL = 1, and u1.weight=0.1, u2.weight=0.2. If there're only two apps, a1 from u1 and a2 from u2 running in the queue and both are requesting a lot of resource, ideally we should get u1 has 33% of the queue's guaranteed capacity and u2 has 66% of the queue's guaranteed capacity. However in this implementation (just from my understanding, haven't tested yet). a1 can get all queue's resource (because #active-user-applied-weights = 1/0.3) while a2 got starved. 2) I would like to prevent setting user's weight to <= 0. It doesn't have clear semantics and could cause many other computation issues - printing a warning is not enough to me. Generally speaking, set user weight < 1 is a reasonable requirement however I don't think we're ready for that. It looks there're bunch of things we need to do to make #2 and related preemption logic works properly. I would suggest to not allow this for now and add a note in the doc saying that this could be supported in the future to avoid incompatible behavior changes. Thoughts? Beyond that, I suggest to make #active-users-times-weight can updated in O(1) for every changes to active users set or any active user's weight get updated. We saw in some extreme cases a queue can have several hundreds of users so change to active users set could be quite frequent. Also, weight of users applies to hard limit of user (user limit factor) as well. This is a gray area to me, since it may cause some issue of resource planning (one more factor apply to maximum resource of user). Would like to hear thoughts from Jason Lowe / Sunil G as well.
          Hide
          eepayne Eric Payne added a comment -

          Wangda Tan, thank you very much for your in-depth review and comments.

          1) When there're several active users with [combined sum of] weights < 1. ... However in this implementation ... a1 can get all queue's resource (because #active-user-applied-weights = 1/0.3) while a2 got starved.

          No, that's not how it will work with this implementation.

          Sunil G had a similar question above. Having a combined sum of weights < 1 works because userLimitResource (the return value of computeUserLimit) is only ever used by getComputedResourceLimitFor[Active|All]Users, which multiplies the value of userLimitResource by the appropriate user weight(s). This will result in the correct value of userLimit for each specific user. If the sum of active user(s)'s weight(s) is < 1, then it is true that userLimitResource is larger than the actual user limit, and sometimes even larger than the actual number of resources used. However, this algorithm calculates userLimit correctly and consistently when getComputedResourceLimitFor[Active|All]Users multiplies it by each user's weight.

          2) I would like to prevent setting user's weight to <= 0.

          Instead of a warning, I will cause the parse of the CS config to fail if weight is < 0. I would like Jason Lowe's and Nathan Roberts's feedback on whether or not weight == 0 is reasonable and consistent.

          Generally speaking, set user weight < 1 is a reasonable requirement however I don't think we're ready for that. It looks there're bunch of things we need to do to make #2 and related preemption logic works properly.

          I am afraid that I disagree for reasons stated above. #2 can be addressed with a simple check that treats failure the same as other parsing issues. The one concern that remains in my mind is to ensure that this algorithm calculates allUserLimit correctly for preemption. I have not yet combined and tested this patch with the one for YARN-2113. I will do so and post my findings.

          Beyond that, I suggest to make #active-users-times-weight can updated in O(1) for every changes to active users set or any active user's weight get updated.

          Yes, good point. Although #active-users-times-weight and #users-times-weight are only calculated in computeUserLimit, and computeUserLimit is only called when a significant event happens, we could eliminate the need to calculate this for things like container allocate and container free events. I will modify the patch to do this.

          Also, weight of users applies to hard limit of user (user limit factor) as well. This is a gray area to me, since it may cause some issue of resource planning (one more factor apply to maximum resource of user). Would like to hear thoughts from Jason Lowe/Sunil G as well.

          I look forward to Jason Lowe's and Sunil G's comments

          Show
          eepayne Eric Payne added a comment - Wangda Tan , thank you very much for your in-depth review and comments. 1) When there're several active users with [combined sum of] weights < 1. ... However in this implementation ... a1 can get all queue's resource (because #active-user-applied-weights = 1/0.3) while a2 got starved. No, that's not how it will work with this implementation. Sunil G had a similar question above . Having a combined sum of weights < 1 works because userLimitResource (the return value of computeUserLimit ) is only ever used by getComputedResourceLimitFor[Active|All]Users , which multiplies the value of userLimitResource by the appropriate user weight(s). This will result in the correct value of userLimit for each specific user. If the sum of active user(s)'s weight(s) is < 1, then it is true that userLimitResource is larger than the actual user limit, and sometimes even larger than the actual number of resources used. However, this algorithm calculates userLimit correctly and consistently when getComputedResourceLimitFor[Active|All]Users multiplies it by each user's weight. 2) I would like to prevent setting user's weight to <= 0. Instead of a warning, I will cause the parse of the CS config to fail if weight is < 0. I would like Jason Lowe 's and Nathan Roberts 's feedback on whether or not weight == 0 is reasonable and consistent. Generally speaking, set user weight < 1 is a reasonable requirement however I don't think we're ready for that. It looks there're bunch of things we need to do to make #2 and related preemption logic works properly. I am afraid that I disagree for reasons stated above. #2 can be addressed with a simple check that treats failure the same as other parsing issues. The one concern that remains in my mind is to ensure that this algorithm calculates allUserLimit correctly for preemption. I have not yet combined and tested this patch with the one for YARN-2113 . I will do so and post my findings. Beyond that, I suggest to make #active-users-times-weight can updated in O(1) for every changes to active users set or any active user's weight get updated. Yes, good point. Although #active-users-times-weight and #users-times-weight are only calculated in computeUserLimit , and computeUserLimit is only called when a significant event happens, we could eliminate the need to calculate this for things like container allocate and container free events. I will modify the patch to do this. Also, weight of users applies to hard limit of user (user limit factor) as well. This is a gray area to me, since it may cause some issue of resource planning (one more factor apply to maximum resource of user). Would like to hear thoughts from Jason Lowe/Sunil G as well. I look forward to Jason Lowe 's and Sunil G 's comments
          Hide
          jlowe Jason Lowe added a comment -

          I'm +1 for weight == 0. As long as it doesn't break the code (e.g.: division by zero, etc.) and does something semantically consistent with weights then I don't see why we should disallow it.
          A practical use of this could be to essentially "pause" a user in a queue – it won't reject the user's app submissions like changing the queue ACLs would, but the user will get very little to no resources until the weight becomes non-zero.

          I'm also +1 for having the weight be less than 1. It looks like it works with the patch now, and I worry that the longer we keep support for it out of the codebase the more difficult it can become to introduce it later. People will see in the existing code that it cannot be less than 1 and end up assuming (explicitly or implicitly) that it never will.

          Show
          jlowe Jason Lowe added a comment - I'm +1 for weight == 0. As long as it doesn't break the code (e.g.: division by zero, etc.) and does something semantically consistent with weights then I don't see why we should disallow it. A practical use of this could be to essentially "pause" a user in a queue – it won't reject the user's app submissions like changing the queue ACLs would, but the user will get very little to no resources until the weight becomes non-zero. I'm also +1 for having the weight be less than 1. It looks like it works with the patch now, and I worry that the longer we keep support for it out of the codebase the more difficult it can become to introduce it later. People will see in the existing code that it cannot be less than 1 and end up assuming (explicitly or implicitly) that it never will.
          Hide
          jlowe Jason Lowe added a comment -

          Also, weight of users applies to hard limit of user (user limit factor) as well. This is a gray area to me, since it may cause some issue of resource planning (one more factor apply to maximum resource of user).

          I think the weight needs to apply to the user limit factor as well. Semantically a user with a weight of 2 should be equivalent to spreading that user's load across two "normal" users. That means a user of weight 2 should get twice the normal limit factor, since two users who both hit their ULF means twice the ULF load was allocated to the queue. If we don't apply the weight to the ULF as well then the math isn't consistent – the 2x user isn't exactly like having two users sharing a load.

          Show
          jlowe Jason Lowe added a comment - Also, weight of users applies to hard limit of user (user limit factor) as well. This is a gray area to me, since it may cause some issue of resource planning (one more factor apply to maximum resource of user). I think the weight needs to apply to the user limit factor as well. Semantically a user with a weight of 2 should be equivalent to spreading that user's load across two "normal" users. That means a user of weight 2 should get twice the normal limit factor, since two users who both hit their ULF means twice the ULF load was allocated to the queue. If we don't apply the weight to the ULF as well then the math isn't consistent – the 2x user isn't exactly like having two users sharing a load.
          Hide
          leftnoteasy Wangda Tan added a comment -

          Jason Lowe, thanks for your comments.

          I think the weight needs to apply to the user limit factor as well ...

          This quite make sense to me, and weight = 0 means the user cannot use any resource at all. Which is also consistent to your previous comment:

          A practical use of this could be to essentially "pause" a user in a queue

          Regarding to weights less than 1, I agree with this part:

          I worry that the longer we keep support for it out of the codebase the more difficult it can become to introduce it later.

          Also as mentioned by Eric Payne, I think this is not a problem as well:

          When there're several active users with weight < 1.

          So I'm OK with setting weight to any value >= 0.

          Show
          leftnoteasy Wangda Tan added a comment - Jason Lowe , thanks for your comments. I think the weight needs to apply to the user limit factor as well ... This quite make sense to me, and weight = 0 means the user cannot use any resource at all. Which is also consistent to your previous comment: A practical use of this could be to essentially "pause" a user in a queue Regarding to weights less than 1, I agree with this part: I worry that the longer we keep support for it out of the codebase the more difficult it can become to introduce it later. Also as mentioned by Eric Payne , I think this is not a problem as well: When there're several active users with weight < 1. So I'm OK with setting weight to any value >= 0.
          Hide
          leftnoteasy Wangda Tan added a comment -

          Thanks Eric Payne for your detailed explanations:

          No, that's not how it will work with this implementation.

          Yeah you're correct, it works with this implementation.

          Instead of a warning, I will cause the parse of the CS config to fail if weight is < 0

          Sounds good.

          I am afraid that I disagree for reasons stated above. #2 can be addressed with a simple check

          It was a typo in my previous comment, it should be #1 and as you commented, it is not an issue.

          Show
          leftnoteasy Wangda Tan added a comment - Thanks Eric Payne for your detailed explanations: No, that's not how it will work with this implementation. Yeah you're correct, it works with this implementation. Instead of a warning, I will cause the parse of the CS config to fail if weight is < 0 Sounds good. I am afraid that I disagree for reasons stated above. #2 can be addressed with a simple check It was a typo in my previous comment, it should be #1 and as you commented, it is not an issue.
          Hide
          eepayne Eric Payne added a comment -

          Thanks again Wangda Tan, Sunil G, and Jason Lowe.

          I am uploading a new patch (YARN-5892.010.patch) with the changes we discussed:

          • this patch causes the parsing of the CS config to fail if weight is <0 or > 100/mulp
          • I have combined and tested this patch with previous patches for YARN-2113, but not with the latest one which fixes the oscillation issues. I will do that soon.
          • the calculation of #active-users-times-weight and #users-times-weight only occurs when users are added or removed, or when the queue is reinitialized.
          Show
          eepayne Eric Payne added a comment - Thanks again Wangda Tan , Sunil G , and Jason Lowe . I am uploading a new patch ( YARN-5892 .010.patch) with the changes we discussed: this patch causes the parsing of the CS config to fail if weight is <0 or > 100/mulp I have combined and tested this patch with previous patches for YARN-2113 , but not with the latest one which fixes the oscillation issues. I will do that soon. the calculation of #active-users-times-weight and #users-times-weight only occurs when users are added or removed, or when the queue is reinitialized.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 18s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
          0 mvndep 0m 10s Maven dependency ordering for branch
          +1 mvninstall 13m 37s trunk passed
          +1 compile 11m 27s trunk passed
          +1 checkstyle 0m 59s trunk passed
          +1 mvnsite 1m 39s trunk passed
          +1 mvneclipse 1m 1s trunk passed
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
          -1 findbugs 1m 2s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common in trunk has 2 extant Findbugs warnings.
          -1 findbugs 1m 5s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager in trunk has 8 extant Findbugs warnings.
          +1 javadoc 1m 19s trunk passed
          0 mvndep 0m 9s Maven dependency ordering for patch
          +1 mvninstall 1m 10s the patch passed
          +1 compile 9m 24s the patch passed
          +1 javac 9m 24s the patch passed
          -0 checkstyle 0m 59s hadoop-yarn-project/hadoop-yarn: The patch generated 19 new + 682 unchanged - 1 fixed = 701 total (was 683)
          +1 mvnsite 1m 37s the patch passed
          +1 mvneclipse 0m 58s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
          +1 findbugs 2m 21s the patch passed
          -1 javadoc 0m 26s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 4 new + 880 unchanged - 0 fixed = 884 total (was 880)
          +1 unit 2m 30s hadoop-yarn-common in the patch passed.
          +1 unit 42m 7s hadoop-yarn-server-resourcemanager in the patch passed.
          +1 unit 0m 15s hadoop-yarn-site in the patch passed.
          +1 asflicense 0m 34s The patch does not generate ASF License warnings.
          104m 39s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ac17dc
          JIRA Issue YARN-5892
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12864638/YARN-5892.010.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 4c6c926796f1 3.13.0-108-generic #155-Ubuntu SMP Wed Jan 11 16:58:52 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / fda86ef
          Default Java 1.8.0_121
          findbugs v3.1.0-RC1
          findbugs https://builds.apache.org/job/PreCommit-YARN-Build/15714/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common-warnings.html
          findbugs https://builds.apache.org/job/PreCommit-YARN-Build/15714/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-warnings.html
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/15714/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
          javadoc https://builds.apache.org/job/PreCommit-YARN-Build/15714/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/15714/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/15714/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 18s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. 0 mvndep 0m 10s Maven dependency ordering for branch +1 mvninstall 13m 37s trunk passed +1 compile 11m 27s trunk passed +1 checkstyle 0m 59s trunk passed +1 mvnsite 1m 39s trunk passed +1 mvneclipse 1m 1s trunk passed 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site -1 findbugs 1m 2s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common in trunk has 2 extant Findbugs warnings. -1 findbugs 1m 5s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager in trunk has 8 extant Findbugs warnings. +1 javadoc 1m 19s trunk passed 0 mvndep 0m 9s Maven dependency ordering for patch +1 mvninstall 1m 10s the patch passed +1 compile 9m 24s the patch passed +1 javac 9m 24s the patch passed -0 checkstyle 0m 59s hadoop-yarn-project/hadoop-yarn: The patch generated 19 new + 682 unchanged - 1 fixed = 701 total (was 683) +1 mvnsite 1m 37s the patch passed +1 mvneclipse 0m 58s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site +1 findbugs 2m 21s the patch passed -1 javadoc 0m 26s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 4 new + 880 unchanged - 0 fixed = 884 total (was 880) +1 unit 2m 30s hadoop-yarn-common in the patch passed. +1 unit 42m 7s hadoop-yarn-server-resourcemanager in the patch passed. +1 unit 0m 15s hadoop-yarn-site in the patch passed. +1 asflicense 0m 34s The patch does not generate ASF License warnings. 104m 39s Subsystem Report/Notes Docker Image:yetus/hadoop:0ac17dc JIRA Issue YARN-5892 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12864638/YARN-5892.010.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 4c6c926796f1 3.13.0-108-generic #155-Ubuntu SMP Wed Jan 11 16:58:52 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / fda86ef Default Java 1.8.0_121 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-YARN-Build/15714/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common-warnings.html findbugs https://builds.apache.org/job/PreCommit-YARN-Build/15714/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-warnings.html checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/15714/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt javadoc https://builds.apache.org/job/PreCommit-YARN-Build/15714/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/15714/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/15714/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          jlowe Jason Lowe added a comment -

          I don't understand imposing a hard limit of weight < 100/MULP. For example, take a queue that has a max capacity far larger than normal capacity and the MULP is 100%. With that hard limit, nobody can have a weight > 1. However two or three separate users can all fit in that queue with their minimum limit due to queue growth beyond normal capacity, but we're not going to let a single, weighted user do the same? I don't see why there needs to be an upper limit on the weight – it's the same as that many separate users showing up. At some point, yes, the weight could be high enough that the queue could never meet the minimum limit of that user, but the same occurs when that many users show up at the same time as well. Some users do not get their minimum because there's too much demand.

          Show
          jlowe Jason Lowe added a comment - I don't understand imposing a hard limit of weight < 100/MULP. For example, take a queue that has a max capacity far larger than normal capacity and the MULP is 100%. With that hard limit, nobody can have a weight > 1. However two or three separate users can all fit in that queue with their minimum limit due to queue growth beyond normal capacity, but we're not going to let a single, weighted user do the same? I don't see why there needs to be an upper limit on the weight – it's the same as that many separate users showing up. At some point, yes, the weight could be high enough that the queue could never meet the minimum limit of that user, but the same occurs when that many users show up at the same time as well. Some users do not get their minimum because there's too much demand.
          Hide
          eepayne Eric Payne added a comment -

          I don't understand imposing a hard limit of weight < 100/MULP.

          Jason Lowe, this was in response to Wangda Tan's comment here. I would be interested to hear his thoughts regarding this.

          BTW, the findbugs issues from the last pre-commit build are all in code that was not changed by this patch.

          Show
          eepayne Eric Payne added a comment - I don't understand imposing a hard limit of weight < 100/MULP. Jason Lowe , this was in response to Wangda Tan 's comment here . I would be interested to hear his thoughts regarding this. BTW, the findbugs issues from the last pre-commit build are all in code that was not changed by this patch.
          Hide
          leftnoteasy Wangda Tan added a comment -

          Eric Payne, Jason Lowe, ideally MULP is guaranteed resource for users in the queue.

          From https://hadoop.apache.org/docs/current/hadoop-yarn/hadoop-yarn-site/CapacityScheduler.html

          Each queue enforces a limit on the percentage of resources allocated to a user at any given time, if there is demand for resources. The user limit can vary between a minimum and maximum value. The former (the minimum value) is set to this property value and the latter (the maximum value) depends on the number of users who have submitted applications.

          In reality there could be more than 100/MULP users run their application in the queue, so some users cannot get the minimum resource. But to me it doesn't make sense to say a user's minimum resource can be more than queue's configured capacity. Previously we didn't enforce MULP <= 100, it was a mistake to me, we should fix it.

          Show
          leftnoteasy Wangda Tan added a comment - Eric Payne , Jason Lowe , ideally MULP is guaranteed resource for users in the queue. From https://hadoop.apache.org/docs/current/hadoop-yarn/hadoop-yarn-site/CapacityScheduler.html Each queue enforces a limit on the percentage of resources allocated to a user at any given time, if there is demand for resources. The user limit can vary between a minimum and maximum value. The former (the minimum value) is set to this property value and the latter (the maximum value) depends on the number of users who have submitted applications. In reality there could be more than 100/MULP users run their application in the queue, so some users cannot get the minimum resource. But to me it doesn't make sense to say a user's minimum resource can be more than queue's configured capacity. Previously we didn't enforce MULP <= 100, it was a mistake to me, we should fix it.
          Hide
          jlowe Jason Lowe added a comment -

          I won't block this patch over the MULP*weight <= 100 check, but I still feel this isn't desirable. Again it comes back to my example above, where multiple users will act semantically different than a single user with equivalent weight if we put in this limit check. Anyway it's something we can revisit in another JIRA.

          Show
          jlowe Jason Lowe added a comment - I won't block this patch over the MULP*weight <= 100 check, but I still feel this isn't desirable. Again it comes back to my example above, where multiple users will act semantically different than a single user with equivalent weight if we put in this limit check. Anyway it's something we can revisit in another JIRA.
          Hide
          eepayne Eric Payne added a comment -

          I will need to upload a new patch at any rate, because the check for 0 < weight < 100/MULP doesn't happen if the weight is set at a parent level and inherited.

          This brings up another (very small) concern that I have regarding the way I implemented this part. I added the collection of user weights as part of AbstractCSQueue because that's where the basic queue initialization happens, and that's where we added this same functionality for preemption properties. However, in order to do the check for weight < 100/MULP, I had to do an instanceof LeafQueue check, which I was a little uncomfortable with because it's the only place that happens in AbstractCSQueue.

          Show
          eepayne Eric Payne added a comment - I will need to upload a new patch at any rate, because the check for 0 < weight < 100/MULP doesn't happen if the weight is set at a parent level and inherited. This brings up another (very small) concern that I have regarding the way I implemented this part. I added the collection of user weights as part of AbstractCSQueue because that's where the basic queue initialization happens, and that's where we added this same functionality for preemption properties. However, in order to do the check for weight < 100/MULP , I had to do an instanceof LeafQueue check, which I was a little uncomfortable with because it's the only place that happens in AbstractCSQueue .
          Hide
          leftnoteasy Wangda Tan added a comment -

          1) For UsersManager

          • updateUserWeights: Should we re-count countActiveUsersTimesWeights/countAllUsersTimesWeights as well?
          • count users weight is an O(n) (n = #users) operation which is not optimal. However I think it is safer than only update one single user in case we can miss some updates by mistake. I don't want this patch blocked by the comment, we can revisit it once we identify any perf issues.

          2) As you mentioned:

          This brings up another (very small) concern that I have regarding the way I implemented this part. I added the collection of user weights as part of AbstractCSQueue because that's where the basic queue initialization happens

          I suggest you can move the check user weight part which is unique to LeafQueue to LeafQueue#setupQueueConfigs. It will be called during reinitialization as well.

          Show
          leftnoteasy Wangda Tan added a comment - 1) For UsersManager updateUserWeights: Should we re-count countActiveUsersTimesWeights/countAllUsersTimesWeights as well? count users weight is an O(n) (n = #users) operation which is not optimal. However I think it is safer than only update one single user in case we can miss some updates by mistake. I don't want this patch blocked by the comment, we can revisit it once we identify any perf issues. 2) As you mentioned: This brings up another (very small) concern that I have regarding the way I implemented this part. I added the collection of user weights as part of AbstractCSQueue because that's where the basic queue initialization happens I suggest you can move the check user weight part which is unique to LeafQueue to LeafQueue#setupQueueConfigs . It will be called during reinitialization as well.
          Hide
          eepayne Eric Payne added a comment -

          Wangda Tan

          updateUserWeights: Should we re-count

          Yes. Thanks for catching this.

          count users weight is an O ... I don't want this patch blocked by the comment, we can revisit it once we identify any perf issues.

          OK

          I suggest you can move the check ... to LeafQueue#setupQueueConfigs.

          Done. Good idea. Thanks.

          Show
          eepayne Eric Payne added a comment - Wangda Tan updateUserWeights: Should we re-count Yes. Thanks for catching this. count users weight is an O ... I don't want this patch blocked by the comment, we can revisit it once we identify any perf issues. OK I suggest you can move the check ... to LeafQueue#setupQueueConfigs. Done. Good idea. Thanks.
          Hide
          sunilg Sunil G added a comment -

          Eric Payne
          Thanks for the work here.

          I have few doubts related to YARN-2113 and this patch together. I remember you tested these two patches together.

          1. Give user-weight is 0, we can get user-limit for certain users as 0. Hence intra-queue preemption module could preempt all containers of user with 0 weight leaving its AM containers behind.
          2. Similar behavior could occur if there are few users in the cluster whose weights are below 1 (and together the sum of weights is less than 1). These edge cases could force more preemption.

          Could you please help to confirm the same.

          Show
          sunilg Sunil G added a comment - Eric Payne Thanks for the work here. I have few doubts related to YARN-2113 and this patch together. I remember you tested these two patches together. Give user-weight is 0, we can get user-limit for certain users as 0. Hence intra-queue preemption module could preempt all containers of user with 0 weight leaving its AM containers behind. Similar behavior could occur if there are few users in the cluster whose weights are below 1 (and together the sum of weights is less than 1). These edge cases could force more preemption. Could you please help to confirm the same.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 18s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
          0 mvndep 0m 45s Maven dependency ordering for branch
          +1 mvninstall 16m 0s trunk passed
          +1 compile 14m 35s trunk passed
          +1 checkstyle 1m 13s trunk passed
          +1 mvnsite 1m 56s trunk passed
          +1 mvneclipse 1m 13s trunk passed
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
          -1 findbugs 1m 12s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common in trunk has 2 extant Findbugs warnings.
          -1 findbugs 1m 14s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager in trunk has 8 extant Findbugs warnings.
          +1 javadoc 1m 36s trunk passed
          0 mvndep 0m 14s Maven dependency ordering for patch
          +1 mvninstall 1m 25s the patch passed
          +1 compile 10m 15s the patch passed
          +1 javac 10m 15s the patch passed
          -0 checkstyle 1m 1s hadoop-yarn-project/hadoop-yarn: The patch generated 19 new + 681 unchanged - 1 fixed = 700 total (was 682)
          +1 mvnsite 1m 38s the patch passed
          +1 mvneclipse 1m 0s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
          +1 findbugs 2m 24s the patch passed
          -1 javadoc 0m 27s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 4 new + 880 unchanged - 0 fixed = 884 total (was 880)
          +1 unit 2m 34s hadoop-yarn-common in the patch passed.
          -1 unit 43m 22s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 unit 0m 17s hadoop-yarn-site in the patch passed.
          +1 asflicense 0m 36s The patch does not generate ASF License warnings.
          115m 0s



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



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ac17dc
          JIRA Issue YARN-5892
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12865233/YARN-5892.012.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 0eb787fcbc20 3.13.0-110-generic #157-Ubuntu SMP Mon Feb 20 11:54:05 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 8b5f2c3
          Default Java 1.8.0_131
          findbugs v3.1.0-RC1
          findbugs https://builds.apache.org/job/PreCommit-YARN-Build/15748/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common-warnings.html
          findbugs https://builds.apache.org/job/PreCommit-YARN-Build/15748/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-warnings.html
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/15748/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
          javadoc https://builds.apache.org/job/PreCommit-YARN-Build/15748/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/15748/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/15748/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/15748/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 18s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. 0 mvndep 0m 45s Maven dependency ordering for branch +1 mvninstall 16m 0s trunk passed +1 compile 14m 35s trunk passed +1 checkstyle 1m 13s trunk passed +1 mvnsite 1m 56s trunk passed +1 mvneclipse 1m 13s trunk passed 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site -1 findbugs 1m 12s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common in trunk has 2 extant Findbugs warnings. -1 findbugs 1m 14s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager in trunk has 8 extant Findbugs warnings. +1 javadoc 1m 36s trunk passed 0 mvndep 0m 14s Maven dependency ordering for patch +1 mvninstall 1m 25s the patch passed +1 compile 10m 15s the patch passed +1 javac 10m 15s the patch passed -0 checkstyle 1m 1s hadoop-yarn-project/hadoop-yarn: The patch generated 19 new + 681 unchanged - 1 fixed = 700 total (was 682) +1 mvnsite 1m 38s the patch passed +1 mvneclipse 1m 0s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site +1 findbugs 2m 24s the patch passed -1 javadoc 0m 27s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 4 new + 880 unchanged - 0 fixed = 884 total (was 880) +1 unit 2m 34s hadoop-yarn-common in the patch passed. -1 unit 43m 22s hadoop-yarn-server-resourcemanager in the patch failed. +1 unit 0m 17s hadoop-yarn-site in the patch passed. +1 asflicense 0m 36s The patch does not generate ASF License warnings. 115m 0s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.TestRMRestart Subsystem Report/Notes Docker Image:yetus/hadoop:0ac17dc JIRA Issue YARN-5892 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12865233/YARN-5892.012.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 0eb787fcbc20 3.13.0-110-generic #157-Ubuntu SMP Mon Feb 20 11:54:05 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 8b5f2c3 Default Java 1.8.0_131 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-YARN-Build/15748/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common-warnings.html findbugs https://builds.apache.org/job/PreCommit-YARN-Build/15748/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-warnings.html checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/15748/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt javadoc https://builds.apache.org/job/PreCommit-YARN-Build/15748/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/15748/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/15748/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/15748/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          eepayne Eric Payne added a comment -

          Sorry Sunil G. I missed your comment.

          I have few doubts related to YARN-2113 and this patch together.
          1. Give user-weight is 0, we can get user-limit for certain users as 0. Hence intra-queue preemption module could preempt all containers of user with 0 weight leaving its AM containers behind.

          If a user starts out with a weight of 0, their app will only get the AM and then never be given any more containers because the weight is also multiplied by the user limit factor. If a user has a weight of >0 and the user's apps have running containers, and then a cluster admin "pauses" the user on a preemptable queue, then all non-AM containers will be preempted. I think this is consistent behavior.

          2. Similar behavior could occur if there are few users in the cluster whose weights are below 1 (and together the sum of weights is less than 1).

          The fact that a user's weight is < 1.0 (or even the sum of multiple users' weights < 1.0) won't cause a problem. If user1 has a weight of 0.5 and its app is consuming the whole queue, and user2 with weight 2.0 submits an app, preemption occurs until user2's app has 4 times the resources of user1. It's the same as if user1 had weight of 1 and user2 had weight of 4.

          Show
          eepayne Eric Payne added a comment - Sorry Sunil G . I missed your comment. I have few doubts related to YARN-2113 and this patch together. 1. Give user-weight is 0, we can get user-limit for certain users as 0. Hence intra-queue preemption module could preempt all containers of user with 0 weight leaving its AM containers behind. If a user starts out with a weight of 0, their app will only get the AM and then never be given any more containers because the weight is also multiplied by the user limit factor. If a user has a weight of >0 and the user's apps have running containers, and then a cluster admin "pauses" the user on a preemptable queue, then all non-AM containers will be preempted. I think this is consistent behavior. 2. Similar behavior could occur if there are few users in the cluster whose weights are below 1 (and together the sum of weights is less than 1). The fact that a user's weight is < 1.0 (or even the sum of multiple users' weights < 1.0) won't cause a problem. If user1 has a weight of 0.5 and its app is consuming the whole queue, and user2 with weight 2.0 submits an app, preemption occurs until user2's app has 4 times the resources of user1. It's the same as if user1 had weight of 1 and user2 had weight of 4.
          Hide
          eepayne Eric Payne added a comment -

          Attaching patch YARN-5892.013.patch. Previous patch had a rounding error.

          Show
          eepayne Eric Payne added a comment - Attaching patch YARN-5892 .013.patch. Previous patch had a rounding error.
          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 2 new or modified test files.
          0 mvndep 0m 15s Maven dependency ordering for branch
          +1 mvninstall 14m 21s trunk passed
          +1 compile 12m 27s trunk passed
          +1 checkstyle 1m 1s trunk passed
          +1 mvnsite 1m 44s trunk passed
          +1 mvneclipse 1m 2s trunk passed
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
          -1 findbugs 1m 8s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common in trunk has 1 extant Findbugs warnings.
          +1 javadoc 1m 19s trunk passed
          0 mvndep 0m 10s Maven dependency ordering for patch
          +1 mvninstall 1m 12s the patch passed
          +1 compile 9m 40s the patch passed
          +1 javac 9m 40s the patch passed
          -0 checkstyle 1m 0s hadoop-yarn-project/hadoop-yarn: The patch generated 19 new + 682 unchanged - 1 fixed = 701 total (was 683)
          +1 mvnsite 1m 37s the patch passed
          +1 mvneclipse 0m 59s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
          +1 findbugs 2m 22s the patch passed
          -1 javadoc 0m 27s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 4 new + 874 unchanged - 0 fixed = 878 total (was 874)
          +1 unit 2m 26s hadoop-yarn-common in the patch passed.
          +1 unit 39m 25s hadoop-yarn-server-resourcemanager in the patch passed.
          +1 unit 0m 14s hadoop-yarn-site in the patch passed.
          +1 asflicense 0m 32s The patch does not generate ASF License warnings.
          104m 17s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:14b5c93
          JIRA Issue YARN-5892
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12867610/YARN-5892.013.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 23522cb60157 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 51b671e
          Default Java 1.8.0_121
          findbugs v3.1.0-RC1
          findbugs https://builds.apache.org/job/PreCommit-YARN-Build/15906/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common-warnings.html
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/15906/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
          javadoc https://builds.apache.org/job/PreCommit-YARN-Build/15906/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/15906/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/15906/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT 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 2 new or modified test files. 0 mvndep 0m 15s Maven dependency ordering for branch +1 mvninstall 14m 21s trunk passed +1 compile 12m 27s trunk passed +1 checkstyle 1m 1s trunk passed +1 mvnsite 1m 44s trunk passed +1 mvneclipse 1m 2s trunk passed 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site -1 findbugs 1m 8s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common in trunk has 1 extant Findbugs warnings. +1 javadoc 1m 19s trunk passed 0 mvndep 0m 10s Maven dependency ordering for patch +1 mvninstall 1m 12s the patch passed +1 compile 9m 40s the patch passed +1 javac 9m 40s the patch passed -0 checkstyle 1m 0s hadoop-yarn-project/hadoop-yarn: The patch generated 19 new + 682 unchanged - 1 fixed = 701 total (was 683) +1 mvnsite 1m 37s the patch passed +1 mvneclipse 0m 59s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site +1 findbugs 2m 22s the patch passed -1 javadoc 0m 27s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 4 new + 874 unchanged - 0 fixed = 878 total (was 874) +1 unit 2m 26s hadoop-yarn-common in the patch passed. +1 unit 39m 25s hadoop-yarn-server-resourcemanager in the patch passed. +1 unit 0m 14s hadoop-yarn-site in the patch passed. +1 asflicense 0m 32s The patch does not generate ASF License warnings. 104m 17s Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue YARN-5892 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12867610/YARN-5892.013.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 23522cb60157 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 51b671e Default Java 1.8.0_121 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-YARN-Build/15906/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common-warnings.html checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/15906/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt javadoc https://builds.apache.org/job/PreCommit-YARN-Build/15906/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/15906/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/15906/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          eepayne Eric Payne added a comment -

          Jason Lowe, Wangda Tan, Sunil G, just checking to see if you have had any progress reviewing this patch. Thanks.

          Show
          eepayne Eric Payne added a comment - Jason Lowe , Wangda Tan , Sunil G , just checking to see if you have had any progress reviewing this patch. Thanks.
          Hide
          sunilg Sunil G added a comment -

          Yes Eric Payne. I will do a round of review and some tests today and share my experience. Thank You.

          Show
          sunilg Sunil G added a comment - Yes Eric Payne . I will do a round of review and some tests today and share my experience. Thank You.
          Hide
          leftnoteasy Wangda Tan added a comment -

          Thanks Eric Payne, one minor comment:

          1) Could you move CapacitySchedulerQueueManager#updateUserWeights to LeafQueue#setupQueueConfigs.

          Show
          leftnoteasy Wangda Tan added a comment - Thanks Eric Payne , one minor comment: 1) Could you move CapacitySchedulerQueueManager#updateUserWeights to LeafQueue#setupQueueConfigs.
          Hide
          sunilg Sunil G added a comment -

          Eric Payne

          Few more doubts on patch.

          1.

          	    // For non-activeUser calculation, consider all users count.
          	    if (!activeUser) {
                        resourceUsed = currentCapacity;	
          	      usersSummedByWeight = allUsersTimesWeights;
                     }
          

          In this case, do we need to also consider a case where allUsersTimesWeights need to be more than 1. For eg, i have user1 as weight 0.4 and user2 as 0.5. Now if these 2 users are only users in my cluster, allUsersTimesWeights will be less than 1. I think in this case UL value is higher.

          2. In UserManager, do we also need to lock while updating "activeUsersTimesWeights". Because this value could be updated when there is queue refresh, or when user move to active from no-active etc.

          Show
          sunilg Sunil G added a comment - Eric Payne Few more doubts on patch. 1. // For non-activeUser calculation, consider all users count. if (!activeUser) { resourceUsed = currentCapacity; usersSummedByWeight = allUsersTimesWeights; } In this case, do we need to also consider a case where allUsersTimesWeights need to be more than 1. For eg, i have user1 as weight 0.4 and user2 as 0.5. Now if these 2 users are only users in my cluster, allUsersTimesWeights will be less than 1. I think in this case UL value is higher. 2. In UserManager, do we also need to lock while updating "activeUsersTimesWeights". Because this value could be updated when there is queue refresh, or when user move to active from no-active etc.
          Hide
          eepayne Eric Payne added a comment -

          Thank you for the reviews.

          allUsersTimesWeights will be less than 1. I think in this case UL value is higher

          Sunil G, I think this is similar to the question I answered above, but I'll restate it here for the sake of clarity.

          Having a combined sum of weights < 1 does not cause UL to be too large. This is because userLimitResource (the return value of computeUserLimit) is only ever used by getComputedResourceLimitFor[Active|All]Users, which then multiplies the value of userLimitResource by the appropriate user's weight before returning it. This will result in the correct value of userLimit for each specific user. When the sum of active user(s)'s weight(s) is < 1, then it is true that userLimitResource is larger than the actual number of resources used. However, userLimitResource is just an intermediate value.

          In UserManager, do we also need to lock while updating "activeUsersTimesWeights".

          Can you please clarify where you see it being read or written outside the lock? I think the code is within locks everwhere it is used.

          1) Could you move CapacitySchedulerQueueManager#updateUserWeights to LeafQueue#setupQueueConfigs.

          Wangda Tan, good optimization. I will make this change, do testing and await Sunil G's response before submitting a new patch.

          Show
          eepayne Eric Payne added a comment - Thank you for the reviews. allUsersTimesWeights will be less than 1. I think in this case UL value is higher Sunil G , I think this is similar to the question I answered above , but I'll restate it here for the sake of clarity. Having a combined sum of weights < 1 does not cause UL to be too large. This is because userLimitResource (the return value of computeUserLimit ) is only ever used by getComputedResourceLimitFor[Active|All]Users , which then multiplies the value of userLimitResource by the appropriate user's weight before returning it. This will result in the correct value of userLimit for each specific user. When the sum of active user(s)'s weight(s) is < 1, then it is true that userLimitResource is larger than the actual number of resources used. However, userLimitResource is just an intermediate value. In UserManager, do we also need to lock while updating "activeUsersTimesWeights". Can you please clarify where you see it being read or written outside the lock? I think the code is within locks everwhere it is used. 1) Could you move CapacitySchedulerQueueManager#updateUserWeights to LeafQueue#setupQueueConfigs. Wangda Tan , good optimization. I will make this change, do testing and await Sunil G 's response before submitting a new patch.
          Hide
          sunilg Sunil G added a comment -

          Thanks Eric Payne

          which then multiplies the value of userLimitResource by the appropriate user's weight before returning it

          I think I am fine here as we multiply with real weight of user, this will help to bring UL to correct value. Thanks for explaining in detail. I also have one more doubt now.

          Resource userSpecificUserLimit =
          	        Resources.multiplyAndNormalizeUp(resourceCalculator,
          	            userLimitResource, weight, lQueue.getMinimumAllocation());
          

          I think we could use multiplyAndNormalizeDown here. I have 2 reasons for this.
          1) Ideally we allow atleast one container (extra) for a user given UL is lesser. So it might be fine to use multiplyAndNormalizeDown given we are not breaking a valid use case.
          We do a > check here, not >=

          LeafQueue#canAssignToUser
                if (Resources.greaterThan(resourceCalculator, clusterResource,
                    user.getUsed(nodePartition), limit)) {
          ...
          

          2) weight_user1=0.1, weight_user2=0.1. Now consider userLimitResource is some how 10GB and minimumAllocation is 4GB. In this case, both user1 and 2 will get UL as 4GB. This will help each user to get 2 containers each. I assume we have queue elasticity and other queue has some more resources. In this case, I feel we do not need to award a user with 2 containers, correct.?

          Please correct me if I am wrong.

          I think the code is within locks everwhere it is used.

          Yes. I did check the code detail. We are fine here, below code was not having lock which confused me, but its caller has correct lock.
          UsersManager.addUser(String userName, User user)

          Show
          sunilg Sunil G added a comment - Thanks Eric Payne which then multiplies the value of userLimitResource by the appropriate user's weight before returning it I think I am fine here as we multiply with real weight of user, this will help to bring UL to correct value. Thanks for explaining in detail. I also have one more doubt now. Resource userSpecificUserLimit = Resources.multiplyAndNormalizeUp(resourceCalculator, userLimitResource, weight, lQueue.getMinimumAllocation()); I think we could use multiplyAndNormalizeDown here. I have 2 reasons for this. 1) Ideally we allow atleast one container (extra) for a user given UL is lesser. So it might be fine to use multiplyAndNormalizeDown given we are not breaking a valid use case. We do a > check here, not >= LeafQueue#canAssignToUser if (Resources.greaterThan(resourceCalculator, clusterResource, user.getUsed(nodePartition), limit)) { ... 2) weight_user1=0.1, weight_user2=0.1. Now consider userLimitResource is some how 10GB and minimumAllocation is 4GB. In this case, both user1 and 2 will get UL as 4GB. This will help each user to get 2 containers each. I assume we have queue elasticity and other queue has some more resources. In this case, I feel we do not need to award a user with 2 containers, correct.? Please correct me if I am wrong. I think the code is within locks everwhere it is used. Yes. I did check the code detail. We are fine here, below code was not having lock which confused me, but its caller has correct lock. UsersManager.addUser(String userName, User user)
          Hide
          eepayne Eric Payne added a comment -

          Uploading YARN-5892.014.patch.

          1) Ideally we allow atleast one container (extra) for a user given UL is lesser. So it might be fine to use multiplyAndNormalizeDown

          I feel we do not need to award a user with 2 containers, correct.?

          Sunil G, thanks. That makes sense. This patch uses multiplyAndNormalizeDown

          1) Could you move CapacitySchedulerQueueManager#updateUserWeights to LeafQueue#setupQueueConfigs.

          Wangda Tan, thanks. This patch contains that change.

          Show
          eepayne Eric Payne added a comment - Uploading YARN-5892 .014.patch. 1) Ideally we allow atleast one container (extra) for a user given UL is lesser. So it might be fine to use multiplyAndNormalizeDown I feel we do not need to award a user with 2 containers, correct.? Sunil G , thanks. That makes sense. This patch uses multiplyAndNormalizeDown 1) Could you move CapacitySchedulerQueueManager#updateUserWeights to LeafQueue#setupQueueConfigs. Wangda Tan , thanks. This patch contains that change.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 17s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
          0 mvndep 0m 44s Maven dependency ordering for branch
          +1 mvninstall 14m 18s trunk passed
          +1 compile 8m 36s trunk passed
          +1 checkstyle 1m 0s trunk passed
          +1 mvnsite 1m 40s trunk passed
          +1 mvneclipse 1m 2s trunk passed
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
          -1 findbugs 1m 5s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common in trunk has 1 extant Findbugs warnings.
          +1 javadoc 1m 20s trunk passed
          0 mvndep 0m 10s Maven dependency ordering for patch
          +1 mvninstall 1m 11s the patch passed
          +1 compile 7m 45s the patch passed
          +1 javac 7m 45s the patch passed
          -0 checkstyle 1m 0s hadoop-yarn-project/hadoop-yarn: The patch generated 18 new + 679 unchanged - 1 fixed = 697 total (was 680)
          +1 mvnsite 1m 36s the patch passed
          +1 mvneclipse 0m 58s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
          +1 findbugs 2m 21s the patch passed
          -1 javadoc 0m 27s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 4 new + 874 unchanged - 0 fixed = 878 total (was 874)
          +1 unit 2m 25s hadoop-yarn-common in the patch passed.
          +1 unit 39m 24s hadoop-yarn-server-resourcemanager in the patch passed.
          +1 unit 0m 15s hadoop-yarn-site in the patch passed.
          +1 asflicense 0m 35s The patch does not generate ASF License warnings.
          98m 38s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:14b5c93
          JIRA Issue YARN-5892
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12870408/YARN-5892.014.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux bb78fec54a26 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / af03c33
          Default Java 1.8.0_131
          findbugs v3.1.0-RC1
          findbugs https://builds.apache.org/job/PreCommit-YARN-Build/16045/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common-warnings.html
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16045/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
          javadoc https://builds.apache.org/job/PreCommit-YARN-Build/16045/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/16045/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/16045/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT 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 17s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. 0 mvndep 0m 44s Maven dependency ordering for branch +1 mvninstall 14m 18s trunk passed +1 compile 8m 36s trunk passed +1 checkstyle 1m 0s trunk passed +1 mvnsite 1m 40s trunk passed +1 mvneclipse 1m 2s trunk passed 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site -1 findbugs 1m 5s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common in trunk has 1 extant Findbugs warnings. +1 javadoc 1m 20s trunk passed 0 mvndep 0m 10s Maven dependency ordering for patch +1 mvninstall 1m 11s the patch passed +1 compile 7m 45s the patch passed +1 javac 7m 45s the patch passed -0 checkstyle 1m 0s hadoop-yarn-project/hadoop-yarn: The patch generated 18 new + 679 unchanged - 1 fixed = 697 total (was 680) +1 mvnsite 1m 36s the patch passed +1 mvneclipse 0m 58s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site +1 findbugs 2m 21s the patch passed -1 javadoc 0m 27s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 4 new + 874 unchanged - 0 fixed = 878 total (was 874) +1 unit 2m 25s hadoop-yarn-common in the patch passed. +1 unit 39m 24s hadoop-yarn-server-resourcemanager in the patch passed. +1 unit 0m 15s hadoop-yarn-site in the patch passed. +1 asflicense 0m 35s The patch does not generate ASF License warnings. 98m 38s Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue YARN-5892 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12870408/YARN-5892.014.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux bb78fec54a26 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / af03c33 Default Java 1.8.0_131 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-YARN-Build/16045/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common-warnings.html checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16045/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt javadoc https://builds.apache.org/job/PreCommit-YARN-Build/16045/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/16045/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/16045/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          eepayne Eric Payne added a comment -

          The findbugs warning was in code that was not changed by this patch. The javadoc warnings are unavoidable due to the naming conventions of the rendering code: '_' used as an identifier.

          Show
          eepayne Eric Payne added a comment - The findbugs warning was in code that was not changed by this patch. The javadoc warnings are unavoidable due to the naming conventions of the rendering code: '_' used as an identifier .
          Hide
          sunilg Sunil G added a comment -

          Latest patch seems fine for me. Wangda Tan, could you also please take a final look.

          Show
          sunilg Sunil G added a comment - Latest patch seems fine for me. Wangda Tan , could you also please take a final look.
          Hide
          eepayne Eric Payne added a comment -

          Sunil G / Wangda Tan / Jason Lowe, I'm loading a new patch. I forgot to make the NormalizeDown modifications to the calculations for all users. In the previous patch, it was just for the active users' calculations.

          Show
          eepayne Eric Payne added a comment - Sunil G / Wangda Tan / Jason Lowe , I'm loading a new patch. I forgot to make the NormalizeDown modifications to the calculations for all users. In the previous patch, it was just for the active users' calculations.
          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.
          0 mvndep 1m 2s Maven dependency ordering for branch
          +1 mvninstall 13m 50s trunk passed
          +1 compile 8m 48s trunk passed
          +1 checkstyle 0m 59s trunk passed
          +1 mvnsite 1m 39s trunk passed
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
          -1 findbugs 1m 4s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common in trunk has 1 extant Findbugs warnings.
          +1 javadoc 1m 20s trunk passed
          0 mvndep 0m 10s Maven dependency ordering for patch
          +1 mvninstall 1m 7s the patch passed
          +1 compile 5m 8s the patch passed
          +1 javac 5m 8s the patch passed
          -0 checkstyle 0m 58s hadoop-yarn-project/hadoop-yarn: The patch generated 16 new + 654 unchanged - 1 fixed = 670 total (was 655)
          +1 mvnsite 1m 36s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
          +1 findbugs 2m 19s the patch passed
          -1 javadoc 0m 27s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 4 new + 853 unchanged - 0 fixed = 857 total (was 853)
          +1 unit 2m 25s hadoop-yarn-common in the patch passed.
          -1 unit 39m 18s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 unit 0m 15s hadoop-yarn-site in the patch passed.
          +1 asflicense 0m 34s The patch does not generate ASF License warnings.
          92m 50s



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



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:14b5c93
          JIRA Issue YARN-5892
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12872802/YARN-5892.015.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 7d1d3a782bbe 3.13.0-107-generic #154-Ubuntu SMP Tue Dec 20 09:57:27 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 8633ef8
          Default Java 1.8.0_131
          findbugs v3.1.0-RC1
          findbugs https://builds.apache.org/job/PreCommit-YARN-Build/16182/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common-warnings.html
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16182/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
          javadoc https://builds.apache.org/job/PreCommit-YARN-Build/16182/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/16182/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/16182/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/16182/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT 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. 0 mvndep 1m 2s Maven dependency ordering for branch +1 mvninstall 13m 50s trunk passed +1 compile 8m 48s trunk passed +1 checkstyle 0m 59s trunk passed +1 mvnsite 1m 39s trunk passed 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site -1 findbugs 1m 4s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common in trunk has 1 extant Findbugs warnings. +1 javadoc 1m 20s trunk passed 0 mvndep 0m 10s Maven dependency ordering for patch +1 mvninstall 1m 7s the patch passed +1 compile 5m 8s the patch passed +1 javac 5m 8s the patch passed -0 checkstyle 0m 58s hadoop-yarn-project/hadoop-yarn: The patch generated 16 new + 654 unchanged - 1 fixed = 670 total (was 655) +1 mvnsite 1m 36s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site +1 findbugs 2m 19s the patch passed -1 javadoc 0m 27s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 4 new + 853 unchanged - 0 fixed = 857 total (was 853) +1 unit 2m 25s hadoop-yarn-common in the patch passed. -1 unit 39m 18s hadoop-yarn-server-resourcemanager in the patch failed. +1 unit 0m 15s hadoop-yarn-site in the patch passed. +1 asflicense 0m 34s The patch does not generate ASF License warnings. 92m 50s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.TestRMRestart Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue YARN-5892 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12872802/YARN-5892.015.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 7d1d3a782bbe 3.13.0-107-generic #154-Ubuntu SMP Tue Dec 20 09:57:27 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 8633ef8 Default Java 1.8.0_131 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-YARN-Build/16182/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common-warnings.html checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16182/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt javadoc https://builds.apache.org/job/PreCommit-YARN-Build/16182/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/16182/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/16182/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/16182/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          leftnoteasy Wangda Tan added a comment -

          Thanks Eric Payne and reviews from Sunil G, patch looks good. Sunil G please commit the patch if you think it's good.

          Show
          leftnoteasy Wangda Tan added a comment - Thanks Eric Payne and reviews from Sunil G , patch looks good. Sunil G please commit the patch if you think it's good.
          Hide
          sunilg Sunil G added a comment -

          Thanks Wangda Tan and Eric Payne
          Committing it shortly.

          Show
          sunilg Sunil G added a comment - Thanks Wangda Tan and Eric Payne Committing it shortly.
          Hide
          sunilg Sunil G added a comment -

          Committed to trunk. However it is not getting applied to branch-2. Eric Payne, could you please help to get branch-2 patch.

          Show
          sunilg Sunil G added a comment - Committed to trunk. However it is not getting applied to branch-2. Eric Payne , could you please help to get branch-2 patch.
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11914 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11914/)
          YARN-5892. Support user-specific minimum user limit percentage in (sunilg: rev ca13b224b2feb9c44de861da9cbba8dd2a12cb35)

          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/CapacitySchedulerConfiguration.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/UserInfo.java
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/resource/DefaultResourceCalculator.java
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/resource/DominantResourceCalculator.java
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/common/fica/FiCaSchedulerApp.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/AbstractCSQueue.java
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site/src/site/markdown/CapacityScheduler.md
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/LeafQueue.java
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/CapacitySchedulerPage.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/CSQueue.java
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/resource/Resources.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/UsersManager.java
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/TestLeafQueue.java
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/resource/ResourceCalculator.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11914 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11914/ ) YARN-5892 . Support user-specific minimum user limit percentage in (sunilg: rev ca13b224b2feb9c44de861da9cbba8dd2a12cb35) (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/CapacitySchedulerConfiguration.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/UserInfo.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/resource/DefaultResourceCalculator.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/resource/DominantResourceCalculator.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/common/fica/FiCaSchedulerApp.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/AbstractCSQueue.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site/src/site/markdown/CapacityScheduler.md (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/LeafQueue.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/CapacitySchedulerPage.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/CSQueue.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/resource/Resources.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/UsersManager.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/TestLeafQueue.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/resource/ResourceCalculator.java
          Hide
          eepayne Eric Payne added a comment -

          Thanks for everything, Sunil G.

          Committed to trunk. However it is not getting applied to branch-2. Eric Payne, could you please help to get branch-2 patch.

          I have a backport for branch-2 for this JIRA. However, it depends on the backport patches for branch-2 from YARN-2113. Can you please take a look at the backport patches that I uploaded for YARN-2113?

          Show
          eepayne Eric Payne added a comment - Thanks for everything, Sunil G . Committed to trunk. However it is not getting applied to branch-2. Eric Payne, could you please help to get branch-2 patch. I have a backport for branch-2 for this JIRA. However, it depends on the backport patches for branch-2 from YARN-2113 . Can you please take a look at the backport patches that I uploaded for YARN-2113 ?
          Hide
          sunilg Sunil G added a comment -

          Sure. I will definitely take a look in to that.

          Show
          sunilg Sunil G added a comment - Sure. I will definitely take a look in to that.
          Hide
          andrew.wang Andrew Wang added a comment -

          I'm going to resolve this for now so I can roll a release. Please re-open if you need a Jenkins run on your backport.

          Show
          andrew.wang Andrew Wang added a comment - I'm going to resolve this for now so I can roll a release. Please re-open if you need a Jenkins run on your backport.
          Hide
          eepayne Eric Payne added a comment -

          Re-opening in order to backport to branch-2 and 2.8.

          Show
          eepayne Eric Payne added a comment - Re-opening in order to backport to branch-2 and 2.8.
          Hide
          eepayne Eric Payne added a comment - - edited

          Sunil G, Wangda Tan, Jason Lowe:
          Since branch-2 and 2.8 are somewhat different than trunk, it was necessary to make some design decisions that I would like you to be aware of when reviewing this backport:

          • As noted here, I did not backport YARN-5889 because it depends on locking changes from YARN-3140 and other locking JIRAs.
          • In trunk, a change was made in YARN-5889 that changed the way computeUserLimit calculates user limit. In branch-2 and branch-2.8, userLimitResource = (all used resources in queue) / (num active users in queue). In trunk after YARN-5889, userLimitResource = (all used resources by active users in queue) / (num active users).
            • Since branch-2 and 2.8 use all used resources in queue instead of all used resources by active users in queue, it is not necessary to modify LeafQueue to update used resource when users are activated and deactivated like was done in UsersManager in trunk.
            • However, I did add the activeUsersSet to LeafQueue and all the places it is modified so it can be used to sum active users times weight.
            • Therefore, it wasn't necessary to create a separate UsersManager class as was done in YARN-5889. Instead, I added a small amount of code in ActiveUsersManager to keep track of active users and to indicate when users are either activated or deactivated.
          • LeafQueue#sumActiveUsersTimesWeights should not do anything that synchronizes or locks. This is to avoid deadlocks because it is called by getHeadRoom (indirectly), which is called by FiCaSchedulerApp.
              float sumActiveUsersTimesWeights() {
                float count = 0.0f;
                for (String userName : activeUsersSet) {
                  User user = users.get(userName);
                  count += (user != null) ? user.getWeight() : 1.0f;
                }
                return count;
              }
            
            • This opens up a race condition for when a user is added or removed from activeUsersSet while sumActiveUsersTimesWeights is iterating over the set.
              • I'm not an expert in Java syncronization. Does this expose LeafQueue to concurrent modification exceptions?
              • There is no ConcurrentHashSet so should I make activeUsersSet a ConcurrentHashMap?
          Show
          eepayne Eric Payne added a comment - - edited Sunil G , Wangda Tan , Jason Lowe : Since branch-2 and 2.8 are somewhat different than trunk, it was necessary to make some design decisions that I would like you to be aware of when reviewing this backport: As noted here , I did not backport YARN-5889 because it depends on locking changes from YARN-3140 and other locking JIRAs. In trunk, a change was made in YARN-5889 that changed the way computeUserLimit calculates user limit. In branch-2 and branch-2.8, userLimitResource = (all used resources in queue) / (num active users in queue) . In trunk after YARN-5889 , userLimitResource = (all used resources by active users in queue) / (num active users) . Since branch-2 and 2.8 use all used resources in queue instead of all used resources by active users in queue , it is not necessary to modify LeafQueue to update used resource when users are activated and deactivated like was done in UsersManager in trunk. However, I did add the activeUsersSet to LeafQueue and all the places it is modified so it can be used to sum active users times weight. Therefore, it wasn't necessary to create a separate UsersManager class as was done in YARN-5889 . Instead, I added a small amount of code in ActiveUsersManager to keep track of active users and to indicate when users are either activated or deactivated. LeafQueue#sumActiveUsersTimesWeights should not do anything that synchronizes or locks. This is to avoid deadlocks because it is called by getHeadRoom (indirectly), which is called by FiCaSchedulerApp . float sumActiveUsersTimesWeights() { float count = 0.0f; for ( String userName : activeUsersSet) { User user = users.get(userName); count += (user != null ) ? user.getWeight() : 1.0f; } return count; } This opens up a race condition for when a user is added or removed from activeUsersSet while sumActiveUsersTimesWeights is iterating over the set. I'm not an expert in Java syncronization. Does this expose LeafQueue to concurrent modification exceptions? There is no ConcurrentHashSet so should I make activeUsersSet a ConcurrentHashMap ?
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 12m 30s 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.
          0 mvndep 2m 27s Maven dependency ordering for branch
          1 mvninstall 7m 18s branch-2 passed
          1 compile 1m 53s branch-2 passed with JDK v1.8.0_131
          1 compile 2m 19s branch-2 passed with JDK v1.7.0_131
          1 checkstyle 0m 49s branch-2 passed
          1 mvnsite 1m 33s branch-2 passed
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
          1 findbugs 2m 29s branch-2 passed
          1 javadoc 0m 58s branch-2 passed with JDK v1.8.0_131
          1 javadoc 1m 8s branch-2 passed with JDK v1.7.0_131
          0 mvndep 0m 11s Maven dependency ordering for patch
          1 mvninstall 1m 9s the patch passed
          1 compile 1m 49s the patch passed with JDK v1.8.0_131
          1 javac 1m 49s the patch passed
          1 compile 2m 13s the patch passed with JDK v1.7.0_131
          1 javac 2m 13s the patch passed
          -0 checkstyle 0m 48s hadoop-yarn-project/hadoop-yarn: The patch generated 20 new 661 unchanged - 1 fixed = 681 total (was 662)
          1 mvnsite 1m 25s the patch passed
          1 whitespace 0m 0s The patch has no whitespace issues.
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
          1 findbugs 2m 41s the patch passed
          -1 javadoc 0m 20s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.8.0_131 with JDK v1.8.0_131 generated 5 new 877 unchanged - 0 fixed = 882 total (was 877)
          1 javadoc 1m 4s the patch passed with JDK v1.7.0_131
          1 unit 2m 22s hadoop-yarn-common in the patch passed with JDK v1.7.0_131.
          -1 unit 43m 28s hadoop-yarn-server-resourcemanager in the patch failed with JDK v1.7.0_131.
          1 unit 0m 12s hadoop-yarn-site in the patch passed with JDK v1.7.0_131.
          1 asflicense 0m 20s The patch does not generate ASF License warnings.
          144m 41s



          Reason Tests
          JDK v1.8.0_131 Failed junit tests hadoop.yarn.server.resourcemanager.TestRMRestart
            hadoop.yarn.server.resourcemanager.security.TestDelegationTokenRenewer
          JDK v1.7.0_131 Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.capacity.TestCapacityScheduler
            hadoop.yarn.server.resourcemanager.TestRMRestart



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:5e40efe
          JIRA Issue YARN-5892
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12875987/YARN-5892.branch-2.015.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux e2fbc99ba4a7 3.13.0-116-generic #163-Ubuntu SMP Fri Mar 31 14:13:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision branch-2 / eda4ac0
          Default Java 1.7.0_131
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_131 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_131
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16316/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
          javadoc https://builds.apache.org/job/PreCommit-YARN-Build/16316/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.8.0_131.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/16316/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.7.0_131.txt
          JDK v1.7.0_131 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/16316/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/16316/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT 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 30s 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. 0 mvndep 2m 27s Maven dependency ordering for branch 1 mvninstall 7m 18s branch-2 passed 1 compile 1m 53s branch-2 passed with JDK v1.8.0_131 1 compile 2m 19s branch-2 passed with JDK v1.7.0_131 1 checkstyle 0m 49s branch-2 passed 1 mvnsite 1m 33s branch-2 passed 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site 1 findbugs 2m 29s branch-2 passed 1 javadoc 0m 58s branch-2 passed with JDK v1.8.0_131 1 javadoc 1m 8s branch-2 passed with JDK v1.7.0_131 0 mvndep 0m 11s Maven dependency ordering for patch 1 mvninstall 1m 9s the patch passed 1 compile 1m 49s the patch passed with JDK v1.8.0_131 1 javac 1m 49s the patch passed 1 compile 2m 13s the patch passed with JDK v1.7.0_131 1 javac 2m 13s the patch passed -0 checkstyle 0m 48s hadoop-yarn-project/hadoop-yarn: The patch generated 20 new 661 unchanged - 1 fixed = 681 total (was 662) 1 mvnsite 1m 25s the patch passed 1 whitespace 0m 0s The patch has no whitespace issues. 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site 1 findbugs 2m 41s the patch passed -1 javadoc 0m 20s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.8.0_131 with JDK v1.8.0_131 generated 5 new 877 unchanged - 0 fixed = 882 total (was 877) 1 javadoc 1m 4s the patch passed with JDK v1.7.0_131 1 unit 2m 22s hadoop-yarn-common in the patch passed with JDK v1.7.0_131. -1 unit 43m 28s hadoop-yarn-server-resourcemanager in the patch failed with JDK v1.7.0_131. 1 unit 0m 12s hadoop-yarn-site in the patch passed with JDK v1.7.0_131. 1 asflicense 0m 20s The patch does not generate ASF License warnings. 144m 41s Reason Tests JDK v1.8.0_131 Failed junit tests hadoop.yarn.server.resourcemanager.TestRMRestart   hadoop.yarn.server.resourcemanager.security.TestDelegationTokenRenewer JDK v1.7.0_131 Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.capacity.TestCapacityScheduler   hadoop.yarn.server.resourcemanager.TestRMRestart Subsystem Report/Notes Docker Image:yetus/hadoop:5e40efe JIRA Issue YARN-5892 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12875987/YARN-5892.branch-2.015.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux e2fbc99ba4a7 3.13.0-116-generic #163-Ubuntu SMP Fri Mar 31 14:13:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision branch-2 / eda4ac0 Default Java 1.7.0_131 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_131 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_131 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16316/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt javadoc https://builds.apache.org/job/PreCommit-YARN-Build/16316/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.8.0_131.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/16316/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.7.0_131.txt JDK v1.7.0_131 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/16316/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/16316/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          sunilg Sunil G added a comment -

          Thanks Eric Payne for the effort.

          YARN-3140 is currently available in trunk. Hence will it be better to get YARN-5889 to branch-2 atleast. Yes, as you mentioned the delta between branch-2 and branch-2.8 is more due the absence of YARN-3140 and its related jiras. I think in a long run we need to make a call whether those changes are really needed to be pulled to branch-2.8

          If this is the case, could we have branch-2 patch of this ticket to be dependent on YARN-5889. And branch-2.8 patch as the way you attached already here? Thoughts.

          Regarding activeUsersSet, we are kind of updating this reference under readLock also. I think it might need to be put it under writeLock for better safety.

          Show
          sunilg Sunil G added a comment - Thanks Eric Payne for the effort. YARN-3140 is currently available in trunk. Hence will it be better to get YARN-5889 to branch-2 atleast. Yes, as you mentioned the delta between branch-2 and branch-2.8 is more due the absence of YARN-3140 and its related jiras. I think in a long run we need to make a call whether those changes are really needed to be pulled to branch-2.8 If this is the case, could we have branch-2 patch of this ticket to be dependent on YARN-5889 . And branch-2.8 patch as the way you attached already here? Thoughts. Regarding activeUsersSet , we are kind of updating this reference under readLock also. I think it might need to be put it under writeLock for better safety.
          Hide
          eepayne Eric Payne added a comment -

          Andrew Wang, this JIRA is already in 3.0.0.alpha4 (revision ca13b224b2feb9c44de861da9cbba8dd2a12cb35). I reopened this JIRA so that I could implement the backport to branches 2 and 2.8.

          Sunil G, yes, if we could get YARN-3140 backported to branch-2, that would indeed be the best. However, I am worried about the time it will take to do that. I don't think I have the bandwidth to do that right now. Are you volunteering to do the backport to branch-2?

          Regarding locking for activeUsersSet, I don't think it is necessary if we can overcome the concurrent modification exception problem. In fact, simple synchronization on this method will cause deadlocks, and I think read/write locks will have the same problem, if on a smaller scale.

          I don't think locking is necessary. If the sum is inaccurate, it will be recomputed during the next round because the list of active users has changed.

          On a related note (if no locking is implemented), I think the following logic is incorrect:

              for (String userName : activeUsersSet) {
                // Do the following instead of calling getUser so locking is not needed.
                User user = users.get(userName);
                count += (user != null) ? user.getWeight() : 1.0f;
              }
          

          I think that if the user was in activeUsersSet when the for loop started but was later removed from the users map, the user weight should be 0.0f instead of 1.0f

          Show
          eepayne Eric Payne added a comment - Andrew Wang , this JIRA is already in 3.0.0.alpha4 (revision ca13b224b2feb9c44de861da9cbba8dd2a12cb35). I reopened this JIRA so that I could implement the backport to branches 2 and 2.8. Sunil G , yes, if we could get YARN-3140 backported to branch-2, that would indeed be the best. However, I am worried about the time it will take to do that. I don't think I have the bandwidth to do that right now. Are you volunteering to do the backport to branch-2? Regarding locking for activeUsersSet , I don't think it is necessary if we can overcome the concurrent modification exception problem. In fact, simple synchronization on this method will cause deadlocks, and I think read/write locks will have the same problem, if on a smaller scale. I don't think locking is necessary. If the sum is inaccurate, it will be recomputed during the next round because the list of active users has changed. On a related note (if no locking is implemented), I think the following logic is incorrect: for ( String userName : activeUsersSet) { // Do the following instead of calling getUser so locking is not needed. User user = users.get(userName); count += (user != null ) ? user.getWeight() : 1.0f; } I think that if the user was in activeUsersSet when the for loop started but was later removed from the users map, the user weight should be 0.0f instead of 1.0f
          Hide
          sunilg Sunil G added a comment -

          Thanks Eric Payne
          YARN-3140 is something i would like to see in branch-2.8 as well. I ll pool some bandwidth for same, but i might need some more time (considering tests).

          I don't think locking is necessary.

          Its not a logical problem i think. I was mentioning about improving it. we already have a reentrant lock there.

          On a related note (if no locking is implemented), I think the following logic is incorrect

          May be we can pull this to another ticket and retrospect there. I think we need a second look in this in detail.

          Show
          sunilg Sunil G added a comment - Thanks Eric Payne YARN-3140 is something i would like to see in branch-2.8 as well. I ll pool some bandwidth for same, but i might need some more time (considering tests). I don't think locking is necessary. Its not a logical problem i think. I was mentioning about improving it. we already have a reentrant lock there. On a related note (if no locking is implemented), I think the following logic is incorrect May be we can pull this to another ticket and retrospect there. I think we need a second look in this in detail.
          Hide
          eepayne Eric Payne added a comment -

          Thanks Sunil G

          YARN-3140 is something i would like to see in branch-2.8 as well. I ll pool some bandwidth for same, but i might need some more time (considering tests).

          Sorry, but I don't think my statement about YARN-3140 was clear.

          • I tried backporting YARN-3140 to branch-2.8, but I had several deadlock problems.
          • I also tried backporting YARN-3140 to branch-2, and I also ran into deadlock problems, but not as many and I didn't really try to look into it as hard as branch-2.8
          • We are anxious to get the user weights feature backported soon.
            I don't think it is worth the effort to try to backport YARN-3140 to branch-2.8. However, for branch-2, I will make time to try the backport again.

            On a related note (if no locking is implemented), I think the following logic is incorrect

            May be we can pull this to another ticket and retrospect there. I think we need a second look in this in detail.

                  count += (user != null) ? user.getWeight() : 1.0f;
            

            It's just a matter of changing the 1 to 0. I would like to go ahead and do that in the next patch.

          Show
          eepayne Eric Payne added a comment - Thanks Sunil G YARN-3140 is something i would like to see in branch-2.8 as well. I ll pool some bandwidth for same, but i might need some more time (considering tests). Sorry, but I don't think my statement about YARN-3140 was clear. I tried backporting YARN-3140 to branch-2.8, but I had several deadlock problems. I also tried backporting YARN-3140 to branch-2, and I also ran into deadlock problems, but not as many and I didn't really try to look into it as hard as branch-2.8 We are anxious to get the user weights feature backported soon. I don't think it is worth the effort to try to backport YARN-3140 to branch-2.8. However, for branch-2, I will make time to try the backport again. On a related note (if no locking is implemented), I think the following logic is incorrect May be we can pull this to another ticket and retrospect there. I think we need a second look in this in detail. count += (user != null ) ? user.getWeight() : 1.0f; It's just a matter of changing the 1 to 0. I would like to go ahead and do that in the next patch.
          Hide
          sunilg Sunil G added a comment -

          I tried backporting YARN-3140 to branch-2.8, but I had several deadlock problems.

          Ok, Thats worrying. I could understand the urgency, so i am fine with approach suggested here. I ll help in review this patch soon

          Show
          sunilg Sunil G added a comment - I tried backporting YARN-3140 to branch-2.8, but I had several deadlock problems. Ok, Thats worrying. I could understand the urgency, so i am fine with approach suggested here. I ll help in review this patch soon
          Hide
          eepayne Eric Payne added a comment -

          Submitting patch with cherry-pick from trunk to branch-2 so that pre-commit build can run on it.

          Show
          eepayne Eric Payne added a comment - Submitting patch with cherry-pick from trunk to branch-2 so that pre-commit build can run on it.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 16m 45s Docker mode activated.
                Prechecks
          +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.
                branch-2 Compile Tests
          0 mvndep 0m 42s Maven dependency ordering for branch
          -1 mvninstall 6m 2s root in branch-2 failed.
          +1 compile 1m 37s branch-2 passed with JDK v1.8.0_131
          +1 compile 2m 2s branch-2 passed with JDK v1.7.0_131
          +1 checkstyle 0m 40s branch-2 passed
          +1 mvnsite 1m 14s branch-2 passed
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
          +1 findbugs 2m 5s branch-2 passed
          +1 javadoc 0m 48s branch-2 passed with JDK v1.8.0_131
          +1 javadoc 0m 58s branch-2 passed with JDK v1.7.0_131
                Patch Compile Tests
          0 mvndep 0m 9s Maven dependency ordering for patch
          +1 mvninstall 0m 59s the patch passed
          +1 compile 1m 36s the patch passed with JDK v1.8.0_131
          +1 javac 1m 36s the patch passed
          +1 compile 2m 0s the patch passed with JDK v1.7.0_131
          +1 javac 2m 0s the patch passed
          -0 checkstyle 0m 38s hadoop-yarn-project/hadoop-yarn: The patch generated 16 new + 663 unchanged - 1 fixed = 679 total (was 664)
          +1 mvnsite 1m 12s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
          +1 findbugs 2m 22s the patch passed
          -1 javadoc 0m 16s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.8.0_131 with JDK v1.8.0_131 generated 4 new + 877 unchanged - 0 fixed = 881 total (was 877)
          +1 javadoc 0m 52s the patch passed with JDK v1.7.0_131
                Other Tests
          +1 unit 2m 34s hadoop-yarn-common in the patch passed with JDK v1.7.0_131.
          -1 unit 46m 18s hadoop-yarn-server-resourcemanager in the patch failed with JDK v1.7.0_131.
          +1 unit 0m 11s hadoop-yarn-site in the patch passed with JDK v1.7.0_131.
          +1 asflicense 0m 21s The patch does not generate ASF License warnings.
          145m 37s



          Reason Tests
          JDK v1.8.0_131 Failed junit tests hadoop.yarn.server.resourcemanager.TestRMRestart
            hadoop.yarn.server.resourcemanager.security.TestDelegationTokenRenewer
          JDK v1.7.0_131 Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.capacity.TestCapacityScheduler
            hadoop.yarn.server.resourcemanager.TestRMRestart



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:5e40efe
          JIRA Issue YARN-5892
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12877115/YARN-5892.branch-2.016.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 4931ed3af6c9 4.4.0-43-generic #63-Ubuntu SMP Wed Oct 12 13:48:03 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision branch-2 / cbb5f60
          Default Java 1.7.0_131
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_131 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_131
          mvninstall https://builds.apache.org/job/PreCommit-YARN-Build/16422/artifact/patchprocess/branch-mvninstall-root.txt
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16422/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
          javadoc https://builds.apache.org/job/PreCommit-YARN-Build/16422/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.8.0_131.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/16422/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.7.0_131.txt
          JDK v1.7.0_131 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/16422/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/16422/console
          Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 16m 45s Docker mode activated.       Prechecks +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.       branch-2 Compile Tests 0 mvndep 0m 42s Maven dependency ordering for branch -1 mvninstall 6m 2s root in branch-2 failed. +1 compile 1m 37s branch-2 passed with JDK v1.8.0_131 +1 compile 2m 2s branch-2 passed with JDK v1.7.0_131 +1 checkstyle 0m 40s branch-2 passed +1 mvnsite 1m 14s branch-2 passed 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site +1 findbugs 2m 5s branch-2 passed +1 javadoc 0m 48s branch-2 passed with JDK v1.8.0_131 +1 javadoc 0m 58s branch-2 passed with JDK v1.7.0_131       Patch Compile Tests 0 mvndep 0m 9s Maven dependency ordering for patch +1 mvninstall 0m 59s the patch passed +1 compile 1m 36s the patch passed with JDK v1.8.0_131 +1 javac 1m 36s the patch passed +1 compile 2m 0s the patch passed with JDK v1.7.0_131 +1 javac 2m 0s the patch passed -0 checkstyle 0m 38s hadoop-yarn-project/hadoop-yarn: The patch generated 16 new + 663 unchanged - 1 fixed = 679 total (was 664) +1 mvnsite 1m 12s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site +1 findbugs 2m 22s the patch passed -1 javadoc 0m 16s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.8.0_131 with JDK v1.8.0_131 generated 4 new + 877 unchanged - 0 fixed = 881 total (was 877) +1 javadoc 0m 52s the patch passed with JDK v1.7.0_131       Other Tests +1 unit 2m 34s hadoop-yarn-common in the patch passed with JDK v1.7.0_131. -1 unit 46m 18s hadoop-yarn-server-resourcemanager in the patch failed with JDK v1.7.0_131. +1 unit 0m 11s hadoop-yarn-site in the patch passed with JDK v1.7.0_131. +1 asflicense 0m 21s The patch does not generate ASF License warnings. 145m 37s Reason Tests JDK v1.8.0_131 Failed junit tests hadoop.yarn.server.resourcemanager.TestRMRestart   hadoop.yarn.server.resourcemanager.security.TestDelegationTokenRenewer JDK v1.7.0_131 Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.capacity.TestCapacityScheduler   hadoop.yarn.server.resourcemanager.TestRMRestart Subsystem Report/Notes Docker Image:yetus/hadoop:5e40efe JIRA Issue YARN-5892 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12877115/YARN-5892.branch-2.016.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 4931ed3af6c9 4.4.0-43-generic #63-Ubuntu SMP Wed Oct 12 13:48:03 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision branch-2 / cbb5f60 Default Java 1.7.0_131 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_131 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_131 mvninstall https://builds.apache.org/job/PreCommit-YARN-Build/16422/artifact/patchprocess/branch-mvninstall-root.txt findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16422/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt javadoc https://builds.apache.org/job/PreCommit-YARN-Build/16422/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.8.0_131.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/16422/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.7.0_131.txt JDK v1.7.0_131 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/16422/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/16422/console Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          eepayne Eric Payne added a comment -

          The javadoc failures are because of the naming convention of the '_' display method:

          [WARNING] /testptch/hadoop/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/CapacitySchedulerPage.java:580: warning: '_' used as an identifier
          

          The TestRMRestart failure is the same as HADOOP-14637.
          resourcemanager.security.TestDelegationTokenRenewer and TestCapacityScheduler both pass for me in my local repo build.

          I will cherry-pick this commit from trunk into branch-2.

          Show
          eepayne Eric Payne added a comment - The javadoc failures are because of the naming convention of the '_' display method: [WARNING] /testptch/hadoop/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/CapacitySchedulerPage.java:580: warning: '_' used as an identifier The TestRMRestart failure is the same as HADOOP-14637 . resourcemanager.security.TestDelegationTokenRenewer and TestCapacityScheduler both pass for me in my local repo build. I will cherry-pick this commit from trunk into branch-2.
          Hide
          eepayne Eric Payne added a comment -

          I cherry-picked the trunk commit to branch-2 for this JIRA.

          I am now attaching YARN-5892.branch-2.8.016.patch, which had to be refactored from the branch-2 version, due to the differences in branch-2.8.

          Show
          eepayne Eric Payne added a comment - I cherry-picked the trunk commit to branch-2 for this JIRA. I am now attaching YARN-5892 .branch-2.8.016.patch, which had to be refactored from the branch-2 version, due to the differences in branch-2.8.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 21s Docker mode activated.
                Prechecks
          +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.
                branch-2.8 Compile Tests
          0 mvndep 0m 19s Maven dependency ordering for branch
          +1 mvninstall 6m 31s branch-2.8 passed
          +1 compile 1m 50s branch-2.8 passed with JDK v1.8.0_131
          +1 compile 2m 12s branch-2.8 passed with JDK v1.7.0_131
          +1 checkstyle 0m 36s branch-2.8 passed
          +1 mvnsite 1m 24s branch-2.8 passed
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
          +1 findbugs 2m 14s branch-2.8 passed
          +1 javadoc 0m 55s branch-2.8 passed with JDK v1.8.0_131
          +1 javadoc 1m 5s branch-2.8 passed with JDK v1.7.0_131
                Patch Compile Tests
          0 mvndep 0m 10s Maven dependency ordering for patch
          -1 mvninstall 0m 20s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 compile 1m 50s the patch passed with JDK v1.8.0_131
          +1 javac 1m 50s the patch passed
          -1 compile 1m 26s hadoop-yarn in the patch failed with JDK v1.7.0_131.
          -1 javac 1m 26s hadoop-yarn in the patch failed with JDK v1.7.0_131.
          -0 checkstyle 0m 37s hadoop-yarn-project/hadoop-yarn: The patch generated 8 new + 286 unchanged - 1 fixed = 294 total (was 287)
          -1 mvnsite 0m 22s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 whitespace 0m 0s The patch has no whitespace issues.
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
          -1 findbugs 0m 19s hadoop-yarn-server-resourcemanager in the patch failed.
          -1 javadoc 0m 19s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.8.0_131 with JDK v1.8.0_131 generated 4 new + 973 unchanged - 0 fixed = 977 total (was 973)
          +1 javadoc 1m 4s the patch passed with JDK v1.7.0_131
                Other Tests
          +1 unit 2m 28s hadoop-yarn-common in the patch passed with JDK v1.7.0_131.
          -1 unit 0m 23s hadoop-yarn-server-resourcemanager in the patch failed with JDK v1.7.0_131.
          +1 unit 0m 9s hadoop-yarn-site in the patch passed with JDK v1.7.0_131.
          +1 asflicense 0m 20s The patch does not generate ASF License warnings.
          115m 1s



          Reason Tests
          JDK v1.8.0_131 Failed junit tests hadoop.yarn.server.resourcemanager.TestAMAuthorization
            hadoop.yarn.server.resourcemanager.TestClientRMTokens



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:d946387
          JIRA Issue YARN-5892
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12877459/YARN-5892.branch-2.8.016.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 842ca0567ad2 3.13.0-119-generic #166-Ubuntu SMP Wed May 3 12:18:55 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision branch-2.8 / 6039a2a
          Default Java 1.7.0_131
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_131 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_131
          findbugs v3.0.0
          mvninstall https://builds.apache.org/job/PreCommit-YARN-Build/16457/artifact/patchprocess/patch-mvninstall-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          compile https://builds.apache.org/job/PreCommit-YARN-Build/16457/artifact/patchprocess/patch-compile-hadoop-yarn-project_hadoop-yarn-jdk1.7.0_131.txt
          javac https://builds.apache.org/job/PreCommit-YARN-Build/16457/artifact/patchprocess/patch-compile-hadoop-yarn-project_hadoop-yarn-jdk1.7.0_131.txt
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16457/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
          mvnsite https://builds.apache.org/job/PreCommit-YARN-Build/16457/artifact/patchprocess/patch-mvnsite-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          findbugs https://builds.apache.org/job/PreCommit-YARN-Build/16457/artifact/patchprocess/patch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          javadoc https://builds.apache.org/job/PreCommit-YARN-Build/16457/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.8.0_131.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/16457/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.7.0_131.txt
          JDK v1.7.0_131 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/16457/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/16457/console
          Powered by Apache Yetus 0.6.0-SNAPSHOT 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.       Prechecks +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.       branch-2.8 Compile Tests 0 mvndep 0m 19s Maven dependency ordering for branch +1 mvninstall 6m 31s branch-2.8 passed +1 compile 1m 50s branch-2.8 passed with JDK v1.8.0_131 +1 compile 2m 12s branch-2.8 passed with JDK v1.7.0_131 +1 checkstyle 0m 36s branch-2.8 passed +1 mvnsite 1m 24s branch-2.8 passed 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site +1 findbugs 2m 14s branch-2.8 passed +1 javadoc 0m 55s branch-2.8 passed with JDK v1.8.0_131 +1 javadoc 1m 5s branch-2.8 passed with JDK v1.7.0_131       Patch Compile Tests 0 mvndep 0m 10s Maven dependency ordering for patch -1 mvninstall 0m 20s hadoop-yarn-server-resourcemanager in the patch failed. +1 compile 1m 50s the patch passed with JDK v1.8.0_131 +1 javac 1m 50s the patch passed -1 compile 1m 26s hadoop-yarn in the patch failed with JDK v1.7.0_131. -1 javac 1m 26s hadoop-yarn in the patch failed with JDK v1.7.0_131. -0 checkstyle 0m 37s hadoop-yarn-project/hadoop-yarn: The patch generated 8 new + 286 unchanged - 1 fixed = 294 total (was 287) -1 mvnsite 0m 22s hadoop-yarn-server-resourcemanager in the patch failed. +1 whitespace 0m 0s The patch has no whitespace issues. 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site -1 findbugs 0m 19s hadoop-yarn-server-resourcemanager in the patch failed. -1 javadoc 0m 19s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.8.0_131 with JDK v1.8.0_131 generated 4 new + 973 unchanged - 0 fixed = 977 total (was 973) +1 javadoc 1m 4s the patch passed with JDK v1.7.0_131       Other Tests +1 unit 2m 28s hadoop-yarn-common in the patch passed with JDK v1.7.0_131. -1 unit 0m 23s hadoop-yarn-server-resourcemanager in the patch failed with JDK v1.7.0_131. +1 unit 0m 9s hadoop-yarn-site in the patch passed with JDK v1.7.0_131. +1 asflicense 0m 20s The patch does not generate ASF License warnings. 115m 1s Reason Tests JDK v1.8.0_131 Failed junit tests hadoop.yarn.server.resourcemanager.TestAMAuthorization   hadoop.yarn.server.resourcemanager.TestClientRMTokens Subsystem Report/Notes Docker Image:yetus/hadoop:d946387 JIRA Issue YARN-5892 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12877459/YARN-5892.branch-2.8.016.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 842ca0567ad2 3.13.0-119-generic #166-Ubuntu SMP Wed May 3 12:18:55 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision branch-2.8 / 6039a2a Default Java 1.7.0_131 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_131 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_131 findbugs v3.0.0 mvninstall https://builds.apache.org/job/PreCommit-YARN-Build/16457/artifact/patchprocess/patch-mvninstall-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt compile https://builds.apache.org/job/PreCommit-YARN-Build/16457/artifact/patchprocess/patch-compile-hadoop-yarn-project_hadoop-yarn-jdk1.7.0_131.txt javac https://builds.apache.org/job/PreCommit-YARN-Build/16457/artifact/patchprocess/patch-compile-hadoop-yarn-project_hadoop-yarn-jdk1.7.0_131.txt checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16457/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt mvnsite https://builds.apache.org/job/PreCommit-YARN-Build/16457/artifact/patchprocess/patch-mvnsite-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt findbugs https://builds.apache.org/job/PreCommit-YARN-Build/16457/artifact/patchprocess/patch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt javadoc https://builds.apache.org/job/PreCommit-YARN-Build/16457/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.8.0_131.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/16457/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.7.0_131.txt JDK v1.7.0_131 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/16457/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/16457/console Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          eepayne Eric Payne added a comment -

          Uploading YARN-5892.branch-2.8.017.patch because I used ConcurrentHashMap.newKeySet() in the previous patch, and that method doesn't exist in JDK 1.8.

          Replaced it with new ConcurrentHashMap<String,String>().keySet("dummy")

          Show
          eepayne Eric Payne added a comment - Uploading YARN-5892 .branch-2.8.017.patch because I used ConcurrentHashMap.newKeySet() in the previous patch, and that method doesn't exist in JDK 1.8. Replaced it with new ConcurrentHashMap<String,String>().keySet("dummy")
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 23s Docker mode activated.
                Prechecks
          +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.
                branch-2.8 Compile Tests
          0 mvndep 0m 26s Maven dependency ordering for branch
          +1 mvninstall 7m 27s branch-2.8 passed
          +1 compile 2m 3s branch-2.8 passed with JDK v1.8.0_131
          +1 compile 2m 26s branch-2.8 passed with JDK v1.7.0_131
          +1 checkstyle 0m 37s branch-2.8 passed
          +1 mvnsite 1m 34s branch-2.8 passed
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
          +1 findbugs 2m 25s branch-2.8 passed
          +1 javadoc 0m 59s branch-2.8 passed with JDK v1.8.0_131
          +1 javadoc 1m 6s branch-2.8 passed with JDK v1.7.0_131
                Patch Compile Tests
          0 mvndep 0m 12s Maven dependency ordering for patch
          -1 mvninstall 0m 23s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 compile 2m 13s the patch passed with JDK v1.8.0_131
          +1 javac 2m 13s the patch passed
          -1 compile 1m 43s hadoop-yarn in the patch failed with JDK v1.7.0_131.
          -1 javac 1m 43s hadoop-yarn in the patch failed with JDK v1.7.0_131.
          -0 checkstyle 0m 39s hadoop-yarn-project/hadoop-yarn: The patch generated 9 new + 286 unchanged - 1 fixed = 295 total (was 287)
          -1 mvnsite 0m 25s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 whitespace 0m 0s The patch has no whitespace issues.
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
          -1 findbugs 0m 22s hadoop-yarn-server-resourcemanager in the patch failed.
          -1 javadoc 0m 20s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.8.0_131 with JDK v1.8.0_131 generated 4 new + 973 unchanged - 0 fixed = 977 total (was 973)
          +1 javadoc 1m 3s the patch passed with JDK v1.7.0_131
                Other Tests
          +1 unit 2m 32s hadoop-yarn-common in the patch passed with JDK v1.7.0_131.
          -1 unit 0m 21s hadoop-yarn-server-resourcemanager in the patch failed with JDK v1.7.0_131.
          +1 unit 0m 10s hadoop-yarn-site in the patch passed with JDK v1.7.0_131.
          +1 asflicense 0m 19s The patch does not generate ASF License warnings.
          118m 40s



          Reason Tests
          JDK v1.8.0_131 Failed junit tests hadoop.yarn.server.resourcemanager.TestAMAuthorization
            hadoop.yarn.server.resourcemanager.TestClientRMTokens



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:d946387
          JIRA Issue YARN-5892
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12877622/YARN-5892.branch-2.8.017.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux e2c18b7ea035 3.13.0-116-generic #163-Ubuntu SMP Fri Mar 31 14:13:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision branch-2.8 / 6039a2a
          Default Java 1.7.0_131
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_131 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_131
          findbugs v3.0.0
          mvninstall https://builds.apache.org/job/PreCommit-YARN-Build/16464/artifact/patchprocess/patch-mvninstall-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          compile https://builds.apache.org/job/PreCommit-YARN-Build/16464/artifact/patchprocess/patch-compile-hadoop-yarn-project_hadoop-yarn-jdk1.7.0_131.txt
          javac https://builds.apache.org/job/PreCommit-YARN-Build/16464/artifact/patchprocess/patch-compile-hadoop-yarn-project_hadoop-yarn-jdk1.7.0_131.txt
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16464/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
          mvnsite https://builds.apache.org/job/PreCommit-YARN-Build/16464/artifact/patchprocess/patch-mvnsite-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          findbugs https://builds.apache.org/job/PreCommit-YARN-Build/16464/artifact/patchprocess/patch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          javadoc https://builds.apache.org/job/PreCommit-YARN-Build/16464/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.8.0_131.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/16464/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.7.0_131.txt
          JDK v1.7.0_131 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/16464/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/16464/console
          Powered by Apache Yetus 0.6.0-SNAPSHOT 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 23s Docker mode activated.       Prechecks +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.       branch-2.8 Compile Tests 0 mvndep 0m 26s Maven dependency ordering for branch +1 mvninstall 7m 27s branch-2.8 passed +1 compile 2m 3s branch-2.8 passed with JDK v1.8.0_131 +1 compile 2m 26s branch-2.8 passed with JDK v1.7.0_131 +1 checkstyle 0m 37s branch-2.8 passed +1 mvnsite 1m 34s branch-2.8 passed 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site +1 findbugs 2m 25s branch-2.8 passed +1 javadoc 0m 59s branch-2.8 passed with JDK v1.8.0_131 +1 javadoc 1m 6s branch-2.8 passed with JDK v1.7.0_131       Patch Compile Tests 0 mvndep 0m 12s Maven dependency ordering for patch -1 mvninstall 0m 23s hadoop-yarn-server-resourcemanager in the patch failed. +1 compile 2m 13s the patch passed with JDK v1.8.0_131 +1 javac 2m 13s the patch passed -1 compile 1m 43s hadoop-yarn in the patch failed with JDK v1.7.0_131. -1 javac 1m 43s hadoop-yarn in the patch failed with JDK v1.7.0_131. -0 checkstyle 0m 39s hadoop-yarn-project/hadoop-yarn: The patch generated 9 new + 286 unchanged - 1 fixed = 295 total (was 287) -1 mvnsite 0m 25s hadoop-yarn-server-resourcemanager in the patch failed. +1 whitespace 0m 0s The patch has no whitespace issues. 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site -1 findbugs 0m 22s hadoop-yarn-server-resourcemanager in the patch failed. -1 javadoc 0m 20s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.8.0_131 with JDK v1.8.0_131 generated 4 new + 973 unchanged - 0 fixed = 977 total (was 973) +1 javadoc 1m 3s the patch passed with JDK v1.7.0_131       Other Tests +1 unit 2m 32s hadoop-yarn-common in the patch passed with JDK v1.7.0_131. -1 unit 0m 21s hadoop-yarn-server-resourcemanager in the patch failed with JDK v1.7.0_131. +1 unit 0m 10s hadoop-yarn-site in the patch passed with JDK v1.7.0_131. +1 asflicense 0m 19s The patch does not generate ASF License warnings. 118m 40s Reason Tests JDK v1.8.0_131 Failed junit tests hadoop.yarn.server.resourcemanager.TestAMAuthorization   hadoop.yarn.server.resourcemanager.TestClientRMTokens Subsystem Report/Notes Docker Image:yetus/hadoop:d946387 JIRA Issue YARN-5892 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12877622/YARN-5892.branch-2.8.017.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux e2c18b7ea035 3.13.0-116-generic #163-Ubuntu SMP Fri Mar 31 14:13:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision branch-2.8 / 6039a2a Default Java 1.7.0_131 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_131 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_131 findbugs v3.0.0 mvninstall https://builds.apache.org/job/PreCommit-YARN-Build/16464/artifact/patchprocess/patch-mvninstall-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt compile https://builds.apache.org/job/PreCommit-YARN-Build/16464/artifact/patchprocess/patch-compile-hadoop-yarn-project_hadoop-yarn-jdk1.7.0_131.txt javac https://builds.apache.org/job/PreCommit-YARN-Build/16464/artifact/patchprocess/patch-compile-hadoop-yarn-project_hadoop-yarn-jdk1.7.0_131.txt checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16464/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt mvnsite https://builds.apache.org/job/PreCommit-YARN-Build/16464/artifact/patchprocess/patch-mvnsite-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt findbugs https://builds.apache.org/job/PreCommit-YARN-Build/16464/artifact/patchprocess/patch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt javadoc https://builds.apache.org/job/PreCommit-YARN-Build/16464/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.8.0_131.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/16464/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.7.0_131.txt JDK v1.7.0_131 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/16464/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/16464/console Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          eepayne Eric Payne added a comment -

          Replaced it with new ConcurrentHashMap<String,String>().keySet("dummy")

          Sigh. I need to be more careful about what is and is not in JDK 1.7. This method with this signature is also not in JDK1.7

          Trying again with YARN-5892.branch-2.8.018.patch:
          Collections.newSetFromMap(new ConcurrentHashMap<String, Boolean>());

          Sunil G, Wangda Tan, Jason Lowe: When the pre-commit build comes back clean, can you please have a look?

          Show
          eepayne Eric Payne added a comment - Replaced it with new ConcurrentHashMap<String,String>().keySet("dummy") Sigh. I need to be more careful about what is and is not in JDK 1.7. This method with this signature is also not in JDK1.7 Trying again with YARN-5892 .branch-2.8.018.patch: Collections.newSetFromMap(new ConcurrentHashMap<String, Boolean>()); Sunil G , Wangda Tan , Jason Lowe : When the pre-commit build comes back clean, can you please have a look?
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 17m 16s Docker mode activated.
                Prechecks
          +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.
                branch-2.8 Compile Tests
          0 mvndep 0m 23s Maven dependency ordering for branch
          +1 mvninstall 9m 0s branch-2.8 passed
          +1 compile 1m 51s branch-2.8 passed with JDK v1.8.0_131
          +1 compile 2m 12s branch-2.8 passed with JDK v1.7.0_131
          +1 checkstyle 0m 36s branch-2.8 passed
          +1 mvnsite 1m 26s branch-2.8 passed
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
          +1 findbugs 2m 16s branch-2.8 passed
          +1 javadoc 0m 57s branch-2.8 passed with JDK v1.8.0_131
          +1 javadoc 1m 6s branch-2.8 passed with JDK v1.7.0_131
                Patch Compile Tests
          0 mvndep 0m 11s Maven dependency ordering for patch
          +1 mvninstall 1m 7s the patch passed
          +1 compile 1m 49s the patch passed with JDK v1.8.0_131
          +1 javac 1m 49s the patch passed
          +1 compile 2m 10s the patch passed with JDK v1.7.0_131
          +1 javac 2m 10s the patch passed
          -0 checkstyle 0m 35s hadoop-yarn-project/hadoop-yarn: The patch generated 8 new + 285 unchanged - 1 fixed = 293 total (was 286)
          +1 mvnsite 1m 21s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site
          +1 findbugs 2m 39s the patch passed
          -1 javadoc 0m 19s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.8.0_131 with JDK v1.8.0_131 generated 4 new + 973 unchanged - 0 fixed = 977 total (was 973)
          +1 javadoc 1m 4s the patch passed with JDK v1.7.0_131
                Other Tests
          +1 unit 2m 26s hadoop-yarn-common in the patch passed with JDK v1.7.0_131.
          -1 unit 82m 11s hadoop-yarn-server-resourcemanager in the patch failed with JDK v1.7.0_131.
          +1 unit 0m 9s hadoop-yarn-site in the patch passed with JDK v1.7.0_131.
          +1 asflicense 0m 29s The patch does not generate ASF License warnings.
          223m 11s



          Reason Tests
          JDK v1.8.0_131 Failed junit tests hadoop.yarn.server.resourcemanager.TestAMAuthorization
            hadoop.yarn.server.resourcemanager.TestClientRMTokens
          JDK v1.7.0_131 Failed junit tests hadoop.yarn.server.resourcemanager.TestAMAuthorization
            hadoop.yarn.server.resourcemanager.TestClientRMTokens



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:d946387
          JIRA Issue YARN-5892
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12877715/YARN-5892.branch-2.8.018.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux b3c27bc4d69f 3.13.0-116-generic #163-Ubuntu SMP Fri Mar 31 14:13:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision branch-2.8 / 4daf574
          Default Java 1.7.0_131
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_131 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_131
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16476/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
          javadoc https://builds.apache.org/job/PreCommit-YARN-Build/16476/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.8.0_131.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/16476/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.7.0_131.txt
          JDK v1.7.0_131 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/16476/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/16476/console
          Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 17m 16s Docker mode activated.       Prechecks +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.       branch-2.8 Compile Tests 0 mvndep 0m 23s Maven dependency ordering for branch +1 mvninstall 9m 0s branch-2.8 passed +1 compile 1m 51s branch-2.8 passed with JDK v1.8.0_131 +1 compile 2m 12s branch-2.8 passed with JDK v1.7.0_131 +1 checkstyle 0m 36s branch-2.8 passed +1 mvnsite 1m 26s branch-2.8 passed 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site +1 findbugs 2m 16s branch-2.8 passed +1 javadoc 0m 57s branch-2.8 passed with JDK v1.8.0_131 +1 javadoc 1m 6s branch-2.8 passed with JDK v1.7.0_131       Patch Compile Tests 0 mvndep 0m 11s Maven dependency ordering for patch +1 mvninstall 1m 7s the patch passed +1 compile 1m 49s the patch passed with JDK v1.8.0_131 +1 javac 1m 49s the patch passed +1 compile 2m 10s the patch passed with JDK v1.7.0_131 +1 javac 2m 10s the patch passed -0 checkstyle 0m 35s hadoop-yarn-project/hadoop-yarn: The patch generated 8 new + 285 unchanged - 1 fixed = 293 total (was 286) +1 mvnsite 1m 21s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. 0 findbugs 0m 0s Skipped patched modules with no Java source: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site +1 findbugs 2m 39s the patch passed -1 javadoc 0m 19s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.8.0_131 with JDK v1.8.0_131 generated 4 new + 973 unchanged - 0 fixed = 977 total (was 973) +1 javadoc 1m 4s the patch passed with JDK v1.7.0_131       Other Tests +1 unit 2m 26s hadoop-yarn-common in the patch passed with JDK v1.7.0_131. -1 unit 82m 11s hadoop-yarn-server-resourcemanager in the patch failed with JDK v1.7.0_131. +1 unit 0m 9s hadoop-yarn-site in the patch passed with JDK v1.7.0_131. +1 asflicense 0m 29s The patch does not generate ASF License warnings. 223m 11s Reason Tests JDK v1.8.0_131 Failed junit tests hadoop.yarn.server.resourcemanager.TestAMAuthorization   hadoop.yarn.server.resourcemanager.TestClientRMTokens JDK v1.7.0_131 Failed junit tests hadoop.yarn.server.resourcemanager.TestAMAuthorization   hadoop.yarn.server.resourcemanager.TestClientRMTokens Subsystem Report/Notes Docker Image:yetus/hadoop:d946387 JIRA Issue YARN-5892 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12877715/YARN-5892.branch-2.8.018.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux b3c27bc4d69f 3.13.0-116-generic #163-Ubuntu SMP Fri Mar 31 14:13:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision branch-2.8 / 4daf574 Default Java 1.7.0_131 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_131 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_131 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16476/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt javadoc https://builds.apache.org/job/PreCommit-YARN-Build/16476/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.8.0_131.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/16476/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.7.0_131.txt JDK v1.7.0_131 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/16476/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/16476/console Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          sunilg Sunil G added a comment -

          Hi Eric Payne
          Thank you very much for the effort. Generally patch looks fine for me except below doubt
          In ActiveUsersManager, could we avoid activeUsersChanged if possible. May be we could keep an active set in ActiveUsersManager itself and we could clear this set when activate/deactivateApplication is invoked.

          Show
          sunilg Sunil G added a comment - Hi Eric Payne Thank you very much for the effort. Generally patch looks fine for me except below doubt In ActiveUsersManager , could we avoid activeUsersChanged if possible. May be we could keep an active set in ActiveUsersManager itself and we could clear this set when activate/deactivateApplication is invoked.
          Hide
          eepayne Eric Payne added a comment -

          Thanks Sunil G for the review.

          In ActiveUsersManager, could we avoid activeUsersChanged if possible. May be we could keep an active set in ActiveUsersManager itself and we could clear this set when activate/deactivateApplication is invoked.

          In ActiveUsersManager, the keyset of usersApplications already holds a list of active users, but it's not enough to know whether or not there are users in this keyset. LeafQueue also needs to know when a users is added to or removed from this set. This is so that LeafQueue#computUserLimit is only recomputing the sum of the active user's weights after a user goes active or inactive, not every time through the code.

          LeafQueue could possibly keep its own copy of ActiveUsersManager#getNumActiveUsers and then compare that with getNumActiveUsers when it needs to know if it must recompute the sum of active user weights. However, that seems more complicated and error prone than using activeUsersChanged.

          Show
          eepayne Eric Payne added a comment - Thanks Sunil G for the review. In ActiveUsersManager, could we avoid activeUsersChanged if possible. May be we could keep an active set in ActiveUsersManager itself and we could clear this set when activate/deactivateApplication is invoked. In ActiveUsersManager , the keyset of usersApplications already holds a list of active users, but it's not enough to know whether or not there are users in this keyset. LeafQueue also needs to know when a users is added to or removed from this set. This is so that LeafQueue#computUserLimit is only recomputing the sum of the active user's weights after a user goes active or inactive, not every time through the code. LeafQueue could possibly keep its own copy of ActiveUsersManager#getNumActiveUsers and then compare that with getNumActiveUsers when it needs to know if it must recompute the sum of active user weights. However, that seems more complicated and error prone than using activeUsersChanged .
          Hide
          sunilg Sunil G added a comment -

          Thanks Eric Payne. Makes sense. Latest patch looks good.

          Show
          sunilg Sunil G added a comment - Thanks Eric Payne . Makes sense. Latest patch looks good.
          Hide
          eepayne Eric Payne added a comment -

          Thanks Sunil G for all your help on this JIRA. Were you planning on committing the branch-2.8 patch, or were you expecting me to do that?

          Show
          eepayne Eric Payne added a comment - Thanks Sunil G for all your help on this JIRA. Were you planning on committing the branch-2.8 patch, or were you expecting me to do that?
          Hide
          sunilg Sunil G added a comment -

          I will help to commit the same to branch-2.8.

          Jenkins seems fine and test case failures are not related.

          Show
          sunilg Sunil G added a comment - I will help to commit the same to branch-2.8. Jenkins seems fine and test case failures are not related.
          Hide
          sunilg Sunil G added a comment -

          Committed to branch-2.8 as well. Resolving this issue. Thank you very much Eric Payne for this effort. Really appreciate the same!

          Show
          sunilg Sunil G added a comment - Committed to branch-2.8 as well. Resolving this issue. Thank you very much Eric Payne for this effort. Really appreciate the same!

            People

            • Assignee:
              eepayne Eric Payne
              Reporter:
              eepayne Eric Payne
            • Votes:
              0 Vote for this issue
              Watchers:
              13 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development