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

Update fair scheduler to use pluggable auth provider

Details

    • Improvement
    • Status: Resolved
    • Major
    • Resolution: Fixed
    • 2.8.0
    • 2.9.0, 3.0.0-alpha2
    • fairscheduler
    • None
    • Reviewed

    Description

      Now that YARN-3100 has made the authorization pluggable, it should be supported by the fair scheduler. YARN-3100 only updated the capacity scheduler.

      Attachments

        1. YARN-4997-011.patch
          23 kB
          Tao Jie
        2. YARN-4997-010.patch
          23 kB
          Tao Jie
        3. YARN-4997-009.patch
          23 kB
          Tao Jie
        4. YARN-4997-008.patch
          23 kB
          Tao Jie
        5. YARN-4997-007.patch
          23 kB
          Tao Jie
        6. YARN-4997-006.patch
          23 kB
          Tao Jie
        7. YARN-4997-005.patch
          23 kB
          Tao Jie
        8. YARN-4997-004.patch
          23 kB
          Tao Jie
        9. YARN-4997-003.patch
          23 kB
          Tao Jie
        10. YARN-4997-002.patch
          22 kB
          Tao Jie
        11. YARN-4997-001.patch
          22 kB
          Tao Jie

        Issue Links

          Activity

            Tao Jie Tao Jie added a comment -

            hi templedf, I would like to take this over, if you haven't started work on this.

            Tao Jie Tao Jie added a comment - hi templedf , I would like to take this over, if you haven't started work on this.

            Tao Jie, you're welcome to take it.

            templedf Daniel Templeton added a comment - Tao Jie , you're welcome to take it.
            Tao Jie Tao Jie added a comment -

            Fix for review.

            Tao Jie Tao Jie added a comment - Fix for review.
            hadoopqa Hadoop QA added a comment -
            -1 overall



            Vote Subsystem Runtime Comment
            0 reexec 0m 11s Docker mode activated.
            +1 @author 0m 0s The patch does not contain any @author tags.
            +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
            0 mvndep 2m 47s Maven dependency ordering for branch
            +1 mvninstall 8m 32s trunk passed
            +1 compile 2m 55s trunk passed
            +1 checkstyle 0m 46s trunk passed
            +1 mvnsite 1m 26s trunk passed
            +1 mvneclipse 0m 33s trunk passed
            +1 findbugs 2m 19s trunk passed
            +1 javadoc 0m 52s trunk passed
            0 mvndep 0m 10s Maven dependency ordering for patch
            +1 mvninstall 1m 9s the patch passed
            +1 compile 2m 51s the patch passed
            +1 javac 2m 51s the patch passed
            -1 checkstyle 0m 44s hadoop-yarn-project/hadoop-yarn: The patch generated 8 new + 321 unchanged - 3 fixed = 329 total (was 324)
            +1 mvnsite 1m 25s the patch passed
            +1 mvneclipse 0m 31s the patch passed
            +1 whitespace 0m 0s The patch has no whitespace issues.
            -1 findbugs 1m 20s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
            +1 javadoc 0m 51s the patch passed
            +1 unit 2m 35s hadoop-yarn-common in the patch passed.
            -1 unit 35m 2s hadoop-yarn-server-resourcemanager in the patch failed.
            +1 asflicense 0m 16s The patch does not generate ASF License warnings.
            69m 29s



            Reason Tests
            FindBugs module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common
              Incorrect lazy initialization of static field org.apache.hadoop.yarn.security.YarnAuthorizationProvider.authorizer in org.apache.hadoop.yarn.security.YarnAuthorizationProvider.destory() At YarnAuthorizationProvider.java:field org.apache.hadoop.yarn.security.YarnAuthorizationProvider.authorizer in org.apache.hadoop.yarn.security.YarnAuthorizationProvider.destory() At YarnAuthorizationProvider.java:[lines 65-67]
            Failed junit tests hadoop.yarn.server.resourcemanager.reservation.TestFairSchedulerPlanFollower
              hadoop.yarn.server.resourcemanager.scheduler.fair.TestFSLeafQueue



            Subsystem Report/Notes
            Docker Image:yetus/hadoop:9560f25
            JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12819010/YARN-4997-001.patch
            JIRA Issue YARN-4997
            Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
            uname Linux bb67cd828e31 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
            Build tool maven
            Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
            git revision trunk / 8ebf2e9
            Default Java 1.8.0_101
            findbugs v3.0.0
            checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12556/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
            findbugs https://builds.apache.org/job/PreCommit-YARN-Build/12556/artifact/patchprocess/new-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common.html
            unit https://builds.apache.org/job/PreCommit-YARN-Build/12556/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
            unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/12556/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/12556/testReport/
            modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn
            Console output https://builds.apache.org/job/PreCommit-YARN-Build/12556/console
            Powered by Apache Yetus 0.3.0 http://yetus.apache.org

            This message was automatically generated.

            hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 11s 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 47s Maven dependency ordering for branch +1 mvninstall 8m 32s trunk passed +1 compile 2m 55s trunk passed +1 checkstyle 0m 46s trunk passed +1 mvnsite 1m 26s trunk passed +1 mvneclipse 0m 33s trunk passed +1 findbugs 2m 19s trunk passed +1 javadoc 0m 52s trunk passed 0 mvndep 0m 10s Maven dependency ordering for patch +1 mvninstall 1m 9s the patch passed +1 compile 2m 51s the patch passed +1 javac 2m 51s the patch passed -1 checkstyle 0m 44s hadoop-yarn-project/hadoop-yarn: The patch generated 8 new + 321 unchanged - 3 fixed = 329 total (was 324) +1 mvnsite 1m 25s the patch passed +1 mvneclipse 0m 31s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. -1 findbugs 1m 20s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) +1 javadoc 0m 51s the patch passed +1 unit 2m 35s hadoop-yarn-common in the patch passed. -1 unit 35m 2s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 69m 29s Reason Tests FindBugs module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common   Incorrect lazy initialization of static field org.apache.hadoop.yarn.security.YarnAuthorizationProvider.authorizer in org.apache.hadoop.yarn.security.YarnAuthorizationProvider.destory() At YarnAuthorizationProvider.java:field org.apache.hadoop.yarn.security.YarnAuthorizationProvider.authorizer in org.apache.hadoop.yarn.security.YarnAuthorizationProvider.destory() At YarnAuthorizationProvider.java: [lines 65-67] Failed junit tests hadoop.yarn.server.resourcemanager.reservation.TestFairSchedulerPlanFollower   hadoop.yarn.server.resourcemanager.scheduler.fair.TestFSLeafQueue Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12819010/YARN-4997-001.patch JIRA Issue YARN-4997 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux bb67cd828e31 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 8ebf2e9 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12556/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt findbugs https://builds.apache.org/job/PreCommit-YARN-Build/12556/artifact/patchprocess/new-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common.html unit https://builds.apache.org/job/PreCommit-YARN-Build/12556/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/12556/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/12556/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/12556/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
            Tao Jie Tao Jie added a comment -

            Attached file fixed findbugs and unittests.

            Tao Jie Tao Jie added a comment - Attached file fixed findbugs and unittests.
            hadoopqa Hadoop QA added a comment -
            -1 overall



            Vote Subsystem Runtime Comment
            0 reexec 0m 15s Docker mode activated.
            +1 @author 0m 0s The patch does not contain any @author tags.
            +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
            0 mvndep 0m 9s Maven dependency ordering for branch
            +1 mvninstall 6m 50s trunk passed
            +1 compile 2m 21s trunk passed
            +1 checkstyle 0m 41s trunk passed
            +1 mvnsite 1m 10s trunk passed
            +1 mvneclipse 0m 30s trunk passed
            +1 findbugs 1m 54s trunk passed
            +1 javadoc 0m 48s trunk passed
            0 mvndep 0m 10s Maven dependency ordering for patch
            +1 mvninstall 1m 0s the patch passed
            +1 compile 2m 14s the patch passed
            +1 javac 2m 14s the patch passed
            -1 checkstyle 0m 46s hadoop-yarn-project/hadoop-yarn: The patch generated 1 new + 334 unchanged - 3 fixed = 335 total (was 337)
            +1 mvnsite 1m 5s the patch passed
            +1 mvneclipse 0m 26s the patch passed
            -1 whitespace 0m 0s The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix.
            -1 findbugs 1m 6s 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 50s the patch passed
            +1 unit 2m 17s hadoop-yarn-common in the patch passed.
            +1 unit 33m 25s hadoop-yarn-server-resourcemanager in the patch passed.
            +1 asflicense 0m 17s The patch does not generate ASF License warnings.
            60m 8s



            Reason Tests
            FindBugs module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
              Inconsistent synchronization of org.apache.hadoop.yarn.server.resourcemanager.scheduler.fair.FairScheduler.authorizer; locked 50% of time Unsynchronized access at FairScheduler.java:50% of time Unsynchronized access at FairScheduler.java:[line 1602]



            Subsystem Report/Notes
            Docker Image:yetus/hadoop:9560f25
            JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12820898/YARN-4997-002.patch
            JIRA Issue YARN-4997
            Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
            uname Linux 8a642c07ec10 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
            Build tool maven
            Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
            git revision trunk / 204a205
            Default Java 1.8.0_101
            findbugs v3.0.0
            checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12559/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
            whitespace https://builds.apache.org/job/PreCommit-YARN-Build/12559/artifact/patchprocess/whitespace-eol.txt
            findbugs https://builds.apache.org/job/PreCommit-YARN-Build/12559/artifact/patchprocess/new-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.html
            Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12559/testReport/
            modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn
            Console output https://builds.apache.org/job/PreCommit-YARN-Build/12559/console
            Powered by Apache Yetus 0.3.0 http://yetus.apache.org

            This message was automatically generated.

            hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 15s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. 0 mvndep 0m 9s Maven dependency ordering for branch +1 mvninstall 6m 50s trunk passed +1 compile 2m 21s trunk passed +1 checkstyle 0m 41s trunk passed +1 mvnsite 1m 10s trunk passed +1 mvneclipse 0m 30s trunk passed +1 findbugs 1m 54s trunk passed +1 javadoc 0m 48s trunk passed 0 mvndep 0m 10s Maven dependency ordering for patch +1 mvninstall 1m 0s the patch passed +1 compile 2m 14s the patch passed +1 javac 2m 14s the patch passed -1 checkstyle 0m 46s hadoop-yarn-project/hadoop-yarn: The patch generated 1 new + 334 unchanged - 3 fixed = 335 total (was 337) +1 mvnsite 1m 5s the patch passed +1 mvneclipse 0m 26s the patch passed -1 whitespace 0m 0s The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix. -1 findbugs 1m 6s 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 50s the patch passed +1 unit 2m 17s hadoop-yarn-common in the patch passed. +1 unit 33m 25s hadoop-yarn-server-resourcemanager in the patch passed. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 60m 8s Reason Tests FindBugs module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager   Inconsistent synchronization of org.apache.hadoop.yarn.server.resourcemanager.scheduler.fair.FairScheduler.authorizer; locked 50% of time Unsynchronized access at FairScheduler.java:50% of time Unsynchronized access at FairScheduler.java: [line 1602] Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12820898/YARN-4997-002.patch JIRA Issue YARN-4997 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 8a642c07ec10 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 204a205 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12559/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt whitespace https://builds.apache.org/job/PreCommit-YARN-Build/12559/artifact/patchprocess/whitespace-eol.txt findbugs https://builds.apache.org/job/PreCommit-YARN-Build/12559/artifact/patchprocess/new-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.html Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12559/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/12559/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
            hadoopqa Hadoop QA added a comment -
            -1 overall



            Vote Subsystem Runtime Comment
            0 reexec 0m 16s Docker mode activated.
            +1 @author 0m 0s The patch does not contain any @author tags.
            +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
            0 mvndep 0m 10s Maven dependency ordering for branch
            +1 mvninstall 7m 1s trunk passed
            +1 compile 2m 25s trunk passed
            +1 checkstyle 0m 42s trunk passed
            +1 mvnsite 1m 11s trunk passed
            +1 mvneclipse 0m 32s trunk passed
            +1 findbugs 2m 5s trunk passed
            +1 javadoc 0m 51s trunk passed
            0 mvndep 0m 10s Maven dependency ordering for patch
            +1 mvninstall 0m 57s the patch passed
            +1 compile 2m 19s the patch passed
            +1 javac 2m 19s the patch passed
            -1 checkstyle 0m 40s hadoop-yarn-project/hadoop-yarn: The patch generated 1 new + 322 unchanged - 3 fixed = 323 total (was 325)
            +1 mvnsite 1m 5s the patch passed
            +1 mvneclipse 0m 26s the patch passed
            +1 whitespace 0m 0s The patch has no whitespace issues.
            -1 findbugs 1m 4s 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 44s the patch passed
            +1 unit 2m 14s hadoop-yarn-common in the patch passed.
            +1 unit 36m 23s hadoop-yarn-server-resourcemanager in the patch passed.
            +1 asflicense 0m 17s The patch does not generate ASF License warnings.
            63m 25s



            Reason Tests
            FindBugs module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
              Inconsistent synchronization of org.apache.hadoop.yarn.server.resourcemanager.scheduler.fair.FairScheduler.authorizer; locked 50% of time Unsynchronized access at FairScheduler.java:50% of time Unsynchronized access at FairScheduler.java:[line 1602]



            Subsystem Report/Notes
            Docker Image:yetus/hadoop:9560f25
            JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12820905/YARN-4997-002.patch
            JIRA Issue YARN-4997
            Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
            uname Linux 2fb4bc271934 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
            Build tool maven
            Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
            git revision trunk / 204a205
            Default Java 1.8.0_101
            findbugs v3.0.0
            checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12560/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
            findbugs https://builds.apache.org/job/PreCommit-YARN-Build/12560/artifact/patchprocess/new-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.html
            Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12560/testReport/
            modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn
            Console output https://builds.apache.org/job/PreCommit-YARN-Build/12560/console
            Powered by Apache Yetus 0.3.0 http://yetus.apache.org

            This message was automatically generated.

            hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 16s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 10s Maven dependency ordering for branch +1 mvninstall 7m 1s trunk passed +1 compile 2m 25s trunk passed +1 checkstyle 0m 42s trunk passed +1 mvnsite 1m 11s trunk passed +1 mvneclipse 0m 32s trunk passed +1 findbugs 2m 5s trunk passed +1 javadoc 0m 51s trunk passed 0 mvndep 0m 10s Maven dependency ordering for patch +1 mvninstall 0m 57s the patch passed +1 compile 2m 19s the patch passed +1 javac 2m 19s the patch passed -1 checkstyle 0m 40s hadoop-yarn-project/hadoop-yarn: The patch generated 1 new + 322 unchanged - 3 fixed = 323 total (was 325) +1 mvnsite 1m 5s the patch passed +1 mvneclipse 0m 26s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. -1 findbugs 1m 4s 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 44s the patch passed +1 unit 2m 14s hadoop-yarn-common in the patch passed. +1 unit 36m 23s hadoop-yarn-server-resourcemanager in the patch passed. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 63m 25s Reason Tests FindBugs module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager   Inconsistent synchronization of org.apache.hadoop.yarn.server.resourcemanager.scheduler.fair.FairScheduler.authorizer; locked 50% of time Unsynchronized access at FairScheduler.java:50% of time Unsynchronized access at FairScheduler.java: [line 1602] Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12820905/YARN-4997-002.patch JIRA Issue YARN-4997 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 2fb4bc271934 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 204a205 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12560/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt findbugs https://builds.apache.org/job/PreCommit-YARN-Build/12560/artifact/patchprocess/new-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.html Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12560/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/12560/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.

            Thanks for the patch, Tao Jie. Overall looks good to me. A couple of minor comments:

                    LOG.info(authorizer.getClass().getName() + " is destoryed.");
            

            Probably best to make the log message at DEBUG level and only do the string concat if debug is on.

                    if (queueInfo == null) {
                      authorizer.setPermission(allocsLoader.getDefaultPermissions(),
                          UserGroupInformation.getCurrentUser());
                      return;
                    }
            

            Since that's in a small method, can we please use an else instead of returning from the if?

                  AccessControlList operationAcl = acls.get(
                      SchedulerUtils.toAccessType(operation));
            

            Tiny quibble... Can we split the line on the equals instead of on the paren?

                authorizer
                    .setPermission(permissions, UserGroupInformation.getCurrentUser());
            

            Similarly, can we break on the comma here instead of the dot?

            Finally, for the public and protected methods you added, please add javadocs.

            templedf Daniel Templeton added a comment - Thanks for the patch, Tao Jie . Overall looks good to me. A couple of minor comments: LOG.info(authorizer.getClass().getName() + " is destoryed." ); Probably best to make the log message at DEBUG level and only do the string concat if debug is on. if (queueInfo == null ) { authorizer.setPermission(allocsLoader.getDefaultPermissions(), UserGroupInformation.getCurrentUser()); return ; } Since that's in a small method, can we please use an else instead of returning from the if ? AccessControlList operationAcl = acls.get( SchedulerUtils.toAccessType(operation)); Tiny quibble... Can we split the line on the equals instead of on the paren? authorizer .setPermission(permissions, UserGroupInformation.getCurrentUser()); Similarly, can we break on the comma here instead of the dot? Finally, for the public and protected methods you added, please add javadocs.
            Tao Jie Tao Jie added a comment -

            templedf, thanks for your review! And attached patch with tiny fix associated with your comments.

            Tao Jie Tao Jie added a comment - templedf , thanks for your review! And attached patch with tiny fix associated with your comments.
            hadoopqa Hadoop QA added a comment -
            -1 overall



            Vote Subsystem Runtime Comment
            0 reexec 0m 15s Docker mode activated.
            +1 @author 0m 0s The patch does not contain any @author tags.
            +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
            0 mvndep 0m 10s Maven dependency ordering for branch
            +1 mvninstall 8m 11s trunk passed
            +1 compile 2m 51s trunk passed
            +1 checkstyle 0m 46s trunk passed
            +1 mvnsite 1m 24s trunk passed
            +1 mvneclipse 0m 32s trunk passed
            +1 findbugs 2m 14s trunk passed
            +1 javadoc 0m 52s trunk passed
            0 mvndep 0m 12s Maven dependency ordering for patch
            +1 mvninstall 1m 2s the patch passed
            +1 compile 2m 43s the patch passed
            +1 javac 2m 43s the patch passed
            -1 checkstyle 0m 43s hadoop-yarn-project/hadoop-yarn: The patch generated 3 new + 322 unchanged - 3 fixed = 325 total (was 325)
            +1 mvnsite 1m 14s the patch passed
            +1 mvneclipse 0m 28s the patch passed
            +1 whitespace 0m 0s The patch has no whitespace issues.
            -1 findbugs 1m 18s 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 20s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 2 new + 963 unchanged - 0 fixed = 965 total (was 963)
            -1 unit 2m 28s hadoop-yarn-common in the patch failed.
            -1 unit 39m 9s hadoop-yarn-server-resourcemanager in the patch failed.
            +1 asflicense 0m 17s The patch does not generate ASF License warnings.
            69m 44s



            Reason Tests
            FindBugs module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
              Inconsistent synchronization of org.apache.hadoop.yarn.server.resourcemanager.scheduler.fair.FairScheduler.authorizer; locked 50% of time Unsynchronized access at FairScheduler.java:50% of time Unsynchronized access at FairScheduler.java:[line 1602]
            Failed junit tests hadoop.yarn.logaggregation.TestAggregatedLogFormat
              hadoop.yarn.server.resourcemanager.TestSubmitApplicationWithRMHA



            Subsystem Report/Notes
            Docker Image:yetus/hadoop:9560f25
            JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12823001/YARN-4997-003.patch
            JIRA Issue YARN-4997
            Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
            uname Linux c9977df59258 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
            Build tool maven
            Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
            git revision trunk / d00d3ad
            Default Java 1.8.0_101
            findbugs v3.0.0
            checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12721/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
            findbugs https://builds.apache.org/job/PreCommit-YARN-Build/12721/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/12721/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/12721/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common.txt
            unit https://builds.apache.org/job/PreCommit-YARN-Build/12721/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
            unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/12721/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common.txt https://builds.apache.org/job/PreCommit-YARN-Build/12721/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/12721/testReport/
            modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn
            Console output https://builds.apache.org/job/PreCommit-YARN-Build/12721/console
            Powered by Apache Yetus 0.3.0 http://yetus.apache.org

            This message was automatically generated.

            hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 15s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 10s Maven dependency ordering for branch +1 mvninstall 8m 11s trunk passed +1 compile 2m 51s trunk passed +1 checkstyle 0m 46s trunk passed +1 mvnsite 1m 24s trunk passed +1 mvneclipse 0m 32s trunk passed +1 findbugs 2m 14s trunk passed +1 javadoc 0m 52s trunk passed 0 mvndep 0m 12s Maven dependency ordering for patch +1 mvninstall 1m 2s the patch passed +1 compile 2m 43s the patch passed +1 javac 2m 43s the patch passed -1 checkstyle 0m 43s hadoop-yarn-project/hadoop-yarn: The patch generated 3 new + 322 unchanged - 3 fixed = 325 total (was 325) +1 mvnsite 1m 14s the patch passed +1 mvneclipse 0m 28s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. -1 findbugs 1m 18s 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 20s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 2 new + 963 unchanged - 0 fixed = 965 total (was 963) -1 unit 2m 28s hadoop-yarn-common in the patch failed. -1 unit 39m 9s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 69m 44s Reason Tests FindBugs module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager   Inconsistent synchronization of org.apache.hadoop.yarn.server.resourcemanager.scheduler.fair.FairScheduler.authorizer; locked 50% of time Unsynchronized access at FairScheduler.java:50% of time Unsynchronized access at FairScheduler.java: [line 1602] Failed junit tests hadoop.yarn.logaggregation.TestAggregatedLogFormat   hadoop.yarn.server.resourcemanager.TestSubmitApplicationWithRMHA Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12823001/YARN-4997-003.patch JIRA Issue YARN-4997 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux c9977df59258 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / d00d3ad Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12721/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt findbugs https://builds.apache.org/job/PreCommit-YARN-Build/12721/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/12721/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/12721/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/12721/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/12721/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common.txt https://builds.apache.org/job/PreCommit-YARN-Build/12721/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/12721/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/12721/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.

            Thanks for the update. I still have a few nits to pick.

            First, you're still missing some javadocs, and some of the javadocs you added is missing the @return tags.

            In this code:

              protected List<Permission> getDefaultPermissions() {
                if (defaultPermissions == null) {
                  List<Permission> permissions = new ArrayList<Permission>();
                  Map<AccessType, AccessControlList> acls =
                      new HashMap<AccessType, AccessControlList>();
                  for (QueueACL acl : QueueACL.values()) {
                    acls.put(SchedulerUtils.toAccessType(acl), EVERYBODY_ACL);
                  }
                  permissions.add(new Permission(
                      new PrivilegedEntity(EntityType.QUEUE, ROOT), acls));
                  defaultPermissions = permissions;
                }
                return defaultPermissions;
              }
            

            Why the intermediate permissions variable? It smells like maybe an attempt at thread safety. If so, it's not enough. If not, can we drop the extra variable since it looks suspicious?

            Here:

            -    public void onReload(AllocationConfiguration info);
            +    void onReload(AllocationConfiguration info) throws IOException;
            

            I'd rather we keep the public keyword. It's optional for interfaces, but it's easier to read with the keyword there.

            templedf Daniel Templeton added a comment - Thanks for the update. I still have a few nits to pick. First, you're still missing some javadocs, and some of the javadocs you added is missing the @return tags. In this code: protected List<Permission> getDefaultPermissions() { if (defaultPermissions == null ) { List<Permission> permissions = new ArrayList<Permission>(); Map<AccessType, AccessControlList> acls = new HashMap<AccessType, AccessControlList>(); for (QueueACL acl : QueueACL.values()) { acls.put(SchedulerUtils.toAccessType(acl), EVERYBODY_ACL); } permissions.add( new Permission( new PrivilegedEntity(EntityType.QUEUE, ROOT), acls)); defaultPermissions = permissions; } return defaultPermissions; } Why the intermediate permissions variable? It smells like maybe an attempt at thread safety. If so, it's not enough. If not, can we drop the extra variable since it looks suspicious? Here: - public void onReload(AllocationConfiguration info); + void onReload(AllocationConfiguration info) throws IOException; I'd rather we keep the public keyword. It's optional for interfaces, but it's easier to read with the keyword there.
            Tao Jie Tao Jie added a comment -

            templedf, thanks for reply! For getDefaultPermissions(), there is no reentrant problem since this method is only invoked by reloadAllocation(), which is already synchronized. The intermediate variable is used here is to avoid potential thread safety problem (Actually, it is not necessary) .
            public keyword in onReload method here is removed since it trigger a checkstyle warning. It's ok to keep it.
            Update the patch, please take a quick review!

            Tao Jie Tao Jie added a comment - templedf , thanks for reply! For getDefaultPermissions() , there is no reentrant problem since this method is only invoked by reloadAllocation() , which is already synchronized. The intermediate variable is used here is to avoid potential thread safety problem (Actually, it is not necessary) . public keyword in onReload method here is removed since it trigger a checkstyle warning. It's ok to keep it. Update the patch, please take a quick review!
            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 11s Maven dependency ordering for branch
            +1 mvninstall 7m 57s trunk passed
            +1 compile 2m 18s trunk passed
            +1 checkstyle 0m 41s trunk passed
            +1 mvnsite 1m 7s trunk passed
            +1 mvneclipse 0m 30s trunk passed
            +1 findbugs 1m 52s trunk passed
            +1 javadoc 0m 48s trunk passed
            0 mvndep 0m 9s Maven dependency ordering for patch
            +1 mvninstall 0m 56s the patch passed
            +1 compile 2m 15s the patch passed
            +1 javac 2m 15s the patch passed
            -1 checkstyle 0m 39s hadoop-yarn-project/hadoop-yarn: The patch generated 4 new + 322 unchanged - 3 fixed = 326 total (was 325)
            +1 mvnsite 1m 4s the patch passed
            +1 mvneclipse 0m 25s the patch passed
            +1 whitespace 0m 0s The patch has no whitespace issues.
            -1 findbugs 1m 4s 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 45s the patch passed
            -1 unit 2m 15s hadoop-yarn-common in the patch failed.
            +1 unit 38m 45s hadoop-yarn-server-resourcemanager in the patch passed.
            +1 asflicense 0m 18s The patch does not generate ASF License warnings.
            66m 5s



            Reason Tests
            FindBugs module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
              Inconsistent synchronization of org.apache.hadoop.yarn.server.resourcemanager.scheduler.fair.FairScheduler.authorizer; locked 50% of time Unsynchronized access at FairScheduler.java:50% of time Unsynchronized access at FairScheduler.java:[line 1602]
            Failed junit tests hadoop.yarn.logaggregation.TestAggregatedLogFormat



            Subsystem Report/Notes
            Docker Image:yetus/hadoop:9560f25
            JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12823160/YARN-4997-004.patch
            JIRA Issue YARN-4997
            Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
            uname Linux 7592c843bcdc 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
            Build tool maven
            Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
            git revision trunk / e83be44
            Default Java 1.8.0_101
            findbugs v3.0.0
            checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12738/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
            findbugs https://builds.apache.org/job/PreCommit-YARN-Build/12738/artifact/patchprocess/new-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.html
            unit https://builds.apache.org/job/PreCommit-YARN-Build/12738/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common.txt
            unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/12738/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common.txt
            Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12738/testReport/
            modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn
            Console output https://builds.apache.org/job/PreCommit-YARN-Build/12738/console
            Powered by Apache Yetus 0.3.0 http://yetus.apache.org

            This message was automatically generated.

            hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 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 11s Maven dependency ordering for branch +1 mvninstall 7m 57s trunk passed +1 compile 2m 18s trunk passed +1 checkstyle 0m 41s trunk passed +1 mvnsite 1m 7s trunk passed +1 mvneclipse 0m 30s trunk passed +1 findbugs 1m 52s trunk passed +1 javadoc 0m 48s trunk passed 0 mvndep 0m 9s Maven dependency ordering for patch +1 mvninstall 0m 56s the patch passed +1 compile 2m 15s the patch passed +1 javac 2m 15s the patch passed -1 checkstyle 0m 39s hadoop-yarn-project/hadoop-yarn: The patch generated 4 new + 322 unchanged - 3 fixed = 326 total (was 325) +1 mvnsite 1m 4s the patch passed +1 mvneclipse 0m 25s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. -1 findbugs 1m 4s 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 45s the patch passed -1 unit 2m 15s hadoop-yarn-common in the patch failed. +1 unit 38m 45s hadoop-yarn-server-resourcemanager in the patch passed. +1 asflicense 0m 18s The patch does not generate ASF License warnings. 66m 5s Reason Tests FindBugs module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager   Inconsistent synchronization of org.apache.hadoop.yarn.server.resourcemanager.scheduler.fair.FairScheduler.authorizer; locked 50% of time Unsynchronized access at FairScheduler.java:50% of time Unsynchronized access at FairScheduler.java: [line 1602] Failed junit tests hadoop.yarn.logaggregation.TestAggregatedLogFormat Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12823160/YARN-4997-004.patch JIRA Issue YARN-4997 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 7592c843bcdc 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / e83be44 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12738/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt findbugs https://builds.apache.org/job/PreCommit-YARN-Build/12738/artifact/patchprocess/new-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.html unit https://builds.apache.org/job/PreCommit-YARN-Build/12738/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/12738/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12738/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/12738/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.

            Thanks for the updated patch, Tao Jie. Please see what you can do about resolving the first and last checkstyle issues and the findbugs issue. Also, if you don't mind, instead of adding more collections without the diamond operator, could you convert the neighboring collections over to using the diamond operator? (Did that make sense?)

            templedf Daniel Templeton added a comment - Thanks for the updated patch, Tao Jie . Please see what you can do about resolving the first and last checkstyle issues and the findbugs issue. Also, if you don't mind, instead of adding more collections without the diamond operator, could you convert the neighboring collections over to using the diamond operator? (Did that make sense?)
            Tao Jie Tao Jie added a comment -

            Thanks for reply, templedf. I updated the code and found out that diamond operator is used since YARN-4207. It's fine to rebase the code and refine code style. I will update the patch soon, please review it again.
            Thank you in advance

            Tao Jie Tao Jie added a comment - Thanks for reply, templedf . I updated the code and found out that diamond operator is used since YARN-4207 . It's fine to rebase the code and refine code style. I will update the patch soon, please review it again. Thank you in advance
            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 9s Maven dependency ordering for branch
            +1 mvninstall 6m 42s trunk passed
            +1 compile 2m 19s trunk passed
            +1 checkstyle 0m 42s trunk passed
            +1 mvnsite 1m 8s trunk passed
            +1 mvneclipse 0m 29s trunk passed
            +1 findbugs 1m 50s trunk passed
            +1 javadoc 0m 48s trunk passed
            0 mvndep 0m 10s Maven dependency ordering for patch
            +1 mvninstall 0m 56s the patch passed
            +1 compile 2m 14s the patch passed
            +1 javac 2m 14s the patch passed
            -1 checkstyle 0m 39s hadoop-yarn-project/hadoop-yarn: The patch generated 3 new + 315 unchanged - 3 fixed = 318 total (was 318)
            +1 mvnsite 1m 3s the patch passed
            +1 mvneclipse 0m 26s the patch passed
            +1 whitespace 0m 0s The patch has no whitespace issues.
            -1 findbugs 1m 22s 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 50s the patch passed
            +1 unit 2m 32s hadoop-yarn-common in the patch passed.
            -1 unit 38m 35s hadoop-yarn-server-resourcemanager in the patch failed.
            +1 asflicense 0m 18s The patch does not generate ASF License warnings.
            65m 31s



            Reason Tests
            FindBugs module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
              Inconsistent synchronization of org.apache.hadoop.yarn.server.resourcemanager.scheduler.fair.FairScheduler.authorizer; locked 50% of time Unsynchronized access at FairScheduler.java:50% of time Unsynchronized access at FairScheduler.java:[line 1604]
            Failed junit tests hadoop.yarn.server.resourcemanager.TestNodeBlacklistingOnAMFailures



            Subsystem Report/Notes
            Docker Image:yetus/hadoop:9560f25
            JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12824808/YARN-4997-005.patch
            JIRA Issue YARN-4997
            Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
            uname Linux 48705d1b57cd 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
            Build tool maven
            Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
            git revision trunk / 115ecb5
            Default Java 1.8.0_101
            findbugs v3.0.0
            checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12848/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
            findbugs https://builds.apache.org/job/PreCommit-YARN-Build/12848/artifact/patchprocess/new-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.html
            unit https://builds.apache.org/job/PreCommit-YARN-Build/12848/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
            unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/12848/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/12848/testReport/
            modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn
            Console output https://builds.apache.org/job/PreCommit-YARN-Build/12848/console
            Powered by Apache Yetus 0.3.0 http://yetus.apache.org

            This message was automatically generated.

            hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 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 9s Maven dependency ordering for branch +1 mvninstall 6m 42s trunk passed +1 compile 2m 19s trunk passed +1 checkstyle 0m 42s trunk passed +1 mvnsite 1m 8s trunk passed +1 mvneclipse 0m 29s trunk passed +1 findbugs 1m 50s trunk passed +1 javadoc 0m 48s trunk passed 0 mvndep 0m 10s Maven dependency ordering for patch +1 mvninstall 0m 56s the patch passed +1 compile 2m 14s the patch passed +1 javac 2m 14s the patch passed -1 checkstyle 0m 39s hadoop-yarn-project/hadoop-yarn: The patch generated 3 new + 315 unchanged - 3 fixed = 318 total (was 318) +1 mvnsite 1m 3s the patch passed +1 mvneclipse 0m 26s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. -1 findbugs 1m 22s 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 50s the patch passed +1 unit 2m 32s hadoop-yarn-common in the patch passed. -1 unit 38m 35s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 18s The patch does not generate ASF License warnings. 65m 31s Reason Tests FindBugs module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager   Inconsistent synchronization of org.apache.hadoop.yarn.server.resourcemanager.scheduler.fair.FairScheduler.authorizer; locked 50% of time Unsynchronized access at FairScheduler.java:50% of time Unsynchronized access at FairScheduler.java: [line 1604] Failed junit tests hadoop.yarn.server.resourcemanager.TestNodeBlacklistingOnAMFailures Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12824808/YARN-4997-005.patch JIRA Issue YARN-4997 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 48705d1b57cd 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 115ecb5 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12848/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt findbugs https://builds.apache.org/job/PreCommit-YARN-Build/12848/artifact/patchprocess/new-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.html unit https://builds.apache.org/job/PreCommit-YARN-Build/12848/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/12848/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/12848/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/12848/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.

            Thanks for the fresh patch. I looked more closely at the findbugs issue, and the problem is that setQueueAcls() isn't synchronized. Yes, it's only ever called from a synchronized context now, but no guarantee of that in the future. To prevent introducing issues later, it would be safest to also synchronize setQueueAcls(). The overhead will be minimal.

            templedf Daniel Templeton added a comment - Thanks for the fresh patch. I looked more closely at the findbugs issue, and the problem is that setQueueAcls() isn't synchronized. Yes, it's only ever called from a synchronized context now, but no guarantee of that in the future. To prevent introducing issues later, it would be safest to also synchronize setQueueAcls() . The overhead will be minimal.
            Tao Jie Tao Jie added a comment -

            Patch updated. Fix findbugs with add synchronized on setQueueAcls()

            Tao Jie Tao Jie added a comment - Patch updated. Fix findbugs with add synchronized on setQueueAcls()
            hadoopqa Hadoop QA added a comment -
            -1 overall



            Vote Subsystem Runtime Comment
            0 reexec 0m 12s Docker mode activated.
            +1 @author 0m 0s The patch does not contain any @author tags.
            +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
            0 mvndep 0m 58s Maven dependency ordering for branch
            +1 mvninstall 7m 3s trunk passed
            +1 compile 2m 20s trunk passed
            +1 checkstyle 0m 41s trunk passed
            +1 mvnsite 1m 8s trunk passed
            +1 mvneclipse 0m 30s trunk passed
            +1 findbugs 1m 56s trunk passed
            +1 javadoc 0m 47s trunk passed
            0 mvndep 0m 9s Maven dependency ordering for patch
            +1 mvninstall 1m 0s the patch passed
            +1 compile 2m 20s the patch passed
            +1 javac 2m 20s the patch passed
            -1 checkstyle 0m 39s hadoop-yarn-project/hadoop-yarn: The patch generated 3 new + 314 unchanged - 3 fixed = 317 total (was 317)
            +1 mvnsite 1m 5s the patch passed
            +1 mvneclipse 0m 26s the patch passed
            +1 whitespace 0m 0s The patch has no whitespace issues.
            +1 findbugs 2m 9s the patch passed
            +1 javadoc 0m 43s the patch passed
            +1 unit 2m 18s hadoop-yarn-common in the patch passed.
            -1 unit 34m 4s hadoop-yarn-server-resourcemanager in the patch failed.
            +1 asflicense 0m 18s The patch does not generate ASF License warnings.
            61m 36s



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



            Subsystem Report/Notes
            Docker Image:yetus/hadoop:9560f25
            JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12825067/YARN-4997-006.patch
            JIRA Issue YARN-4997
            Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
            uname Linux 0018f079314b 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
            Build tool maven
            Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
            git revision trunk / 6f9c346
            Default Java 1.8.0_101
            findbugs v3.0.0
            checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12861/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
            unit https://builds.apache.org/job/PreCommit-YARN-Build/12861/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
            unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/12861/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/12861/testReport/
            modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn
            Console output https://builds.apache.org/job/PreCommit-YARN-Build/12861/console
            Powered by Apache Yetus 0.3.0 http://yetus.apache.org

            This message was automatically generated.

            hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 12s 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 58s Maven dependency ordering for branch +1 mvninstall 7m 3s trunk passed +1 compile 2m 20s trunk passed +1 checkstyle 0m 41s trunk passed +1 mvnsite 1m 8s trunk passed +1 mvneclipse 0m 30s trunk passed +1 findbugs 1m 56s trunk passed +1 javadoc 0m 47s trunk passed 0 mvndep 0m 9s Maven dependency ordering for patch +1 mvninstall 1m 0s the patch passed +1 compile 2m 20s the patch passed +1 javac 2m 20s the patch passed -1 checkstyle 0m 39s hadoop-yarn-project/hadoop-yarn: The patch generated 3 new + 314 unchanged - 3 fixed = 317 total (was 317) +1 mvnsite 1m 5s the patch passed +1 mvneclipse 0m 26s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 2m 9s the patch passed +1 javadoc 0m 43s the patch passed +1 unit 2m 18s hadoop-yarn-common in the patch passed. -1 unit 34m 4s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 18s The patch does not generate ASF License warnings. 61m 36s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.capacity.TestNodeLabelContainerAllocation Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12825067/YARN-4997-006.patch JIRA Issue YARN-4997 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 0018f079314b 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 6f9c346 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12861/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/12861/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/12861/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/12861/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/12861/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.

            LGTM! Except... I apologize for not seeing this earlier. Your destroy() method is misspelled as destory().

            templedf Daniel Templeton added a comment - LGTM! Except... I apologize for not seeing this earlier. Your destroy() method is misspelled as destory() .
            Tao Jie Tao Jie added a comment -

            Oops!Fix this misspelling and update the patch.

            Tao Jie Tao Jie added a comment - Oops!Fix this misspelling and update the patch.
            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 26s Maven dependency ordering for branch
            +1 mvninstall 7m 53s trunk passed
            +1 compile 2m 26s trunk passed
            +1 checkstyle 0m 42s trunk passed
            +1 mvnsite 1m 11s trunk passed
            +1 mvneclipse 0m 32s trunk passed
            +1 findbugs 1m 58s trunk passed
            +1 javadoc 0m 52s trunk passed
            0 mvndep 0m 10s Maven dependency ordering for patch
            +1 mvninstall 1m 3s the patch passed
            +1 compile 2m 26s the patch passed
            +1 javac 2m 26s the patch passed
            -1 checkstyle 0m 41s hadoop-yarn-project/hadoop-yarn: The patch generated 3 new + 315 unchanged - 3 fixed = 318 total (was 318)
            +1 mvnsite 1m 10s the patch passed
            +1 mvneclipse 0m 28s the patch passed
            +1 whitespace 0m 0s The patch has no whitespace issues.
            +1 findbugs 2m 13s the patch passed
            +1 javadoc 0m 48s the patch passed
            +1 unit 2m 37s hadoop-yarn-common in the patch passed.
            +1 unit 38m 15s hadoop-yarn-server-resourcemanager in the patch passed.
            +1 asflicense 0m 17s The patch does not generate ASF License warnings.
            67m 14s



            Subsystem Report/Notes
            Docker Image:yetus/hadoop:9560f25
            JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12825067/YARN-4997-006.patch
            JIRA Issue YARN-4997
            Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
            uname Linux 3a17dda026cd 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
            Build tool maven
            Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
            git revision trunk / c37346d
            Default Java 1.8.0_101
            findbugs v3.0.0
            checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12869/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
            Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12869/testReport/
            modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn
            Console output https://builds.apache.org/job/PreCommit-YARN-Build/12869/console
            Powered by Apache Yetus 0.3.0 http://yetus.apache.org

            This message was automatically generated.

            hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 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 26s Maven dependency ordering for branch +1 mvninstall 7m 53s trunk passed +1 compile 2m 26s trunk passed +1 checkstyle 0m 42s trunk passed +1 mvnsite 1m 11s trunk passed +1 mvneclipse 0m 32s trunk passed +1 findbugs 1m 58s trunk passed +1 javadoc 0m 52s trunk passed 0 mvndep 0m 10s Maven dependency ordering for patch +1 mvninstall 1m 3s the patch passed +1 compile 2m 26s the patch passed +1 javac 2m 26s the patch passed -1 checkstyle 0m 41s hadoop-yarn-project/hadoop-yarn: The patch generated 3 new + 315 unchanged - 3 fixed = 318 total (was 318) +1 mvnsite 1m 10s the patch passed +1 mvneclipse 0m 28s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 2m 13s the patch passed +1 javadoc 0m 48s the patch passed +1 unit 2m 37s hadoop-yarn-common in the patch passed. +1 unit 38m 15s hadoop-yarn-server-resourcemanager in the patch passed. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 67m 14s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12825067/YARN-4997-006.patch JIRA Issue YARN-4997 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 3a17dda026cd 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / c37346d Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12869/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12869/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/12869/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.

            +1 (non-binding). Thanks for all the patches, Tao Jie!

            templedf Daniel Templeton added a comment - +1 (non-binding). Thanks for all the patches, Tao Jie !
            Tao Jie Tao Jie added a comment -

            Thank you templedf for your review! kasha, could you please give it a review?

            Tao Jie Tao Jie added a comment - Thank you templedf for your review! kasha , could you please give it a review?

            Thanks for working on this, cassanada.

            Few (mostly minor) comments on the latest patch:

            1. YarnAuthorizationProvider#destroy should be marked @VisibleForTesting.
            2. Noticed there is QueueACL is mapreduce code as well that can be dropped altogether? e.g. mapred QueueManager, many parts (all of?) QueueACL etc. Can we file a follow-up JIRA to drop all of that?
            3. AllocationFileLoaderService:
              1. getDefaultPermissions: don't need to specify type when creating an arraylist for defaultPermissions.
              2. Listener is an interface. Don't need to specify visibility - public?
            4. FairScheduler
              1. onReload: Is there a need to lock the scheduler when setting permissions? Would it be okay to limit the synchronized block to whatever was synchronized before?
              2. Similarly, is there a reason to synchronize setQueueAcls?
              3. In setQueueAcls, we seem to initially set to default permissions and then "overwrite" it with final permissions. Is the first one necessary? I quickly looked at implementation of ConfiguredAuthorizationProvider, setPermission's semantics appear to be somewhere between append and overwrite. If it is append, may be we should change that name to addPermission?
            kasha Karthik Kambatla added a comment - Thanks for working on this, cassanada . Few (mostly minor) comments on the latest patch: YarnAuthorizationProvider#destroy should be marked @VisibleForTesting. Noticed there is QueueACL is mapreduce code as well that can be dropped altogether? e.g. mapred QueueManager, many parts (all of?) QueueACL etc. Can we file a follow-up JIRA to drop all of that? AllocationFileLoaderService: getDefaultPermissions: don't need to specify type when creating an arraylist for defaultPermissions. Listener is an interface. Don't need to specify visibility - public? FairScheduler onReload: Is there a need to lock the scheduler when setting permissions? Would it be okay to limit the synchronized block to whatever was synchronized before? Similarly, is there a reason to synchronize setQueueAcls? In setQueueAcls, we seem to initially set to default permissions and then "overwrite" it with final permissions. Is the first one necessary? I quickly looked at implementation of ConfiguredAuthorizationProvider, setPermission's semantics appear to be somewhere between append and overwrite. If it is append, may be we should change that name to addPermission?

            Canceling patch..

            kasha Karthik Kambatla added a comment - Canceling patch..
            Tao Jie Tao Jie added a comment -

            Thank you for your comments, kasha.

            Noticed there is QueueACL is mapreduce code as well that can be dropped altogether? e.g. mapred QueueManager, many parts (all of?) QueueACL etc. Can we file a follow-up JIRA to drop all of that?

            I am not sure if such code in mapred.QueueMananger still works today. I prefer to clean up those mapreduce code in another JIRA.

            onReload: Is there a need to lock the scheduler when setting permissions? Would it be okay to limit the synchronized block to whatever was synchronized before?

            Have disscussed with templedf, synchronized block added here is to avoid findbugs warning. Actually I will be glad to remove redundant lock here.

            In setQueueAcls, we seem to initially set to default permissions and then "overwrite" it with final permissions. Is the first one necessary? I quickly looked at implementation of ConfiguredAuthorizationProvider, setPermission's semantics appear to be somewhere between append and overwrite. If it is append, may be we should change that name to addPermission?

            I also feel a little confused about semantics of setPermission. However this abstract method is introduced by YARN-3100, and I'm not sure setPermission has implemented in Ranger or Sentry. I prefer to keep setPermission here as CapacityScheduler does to keep compatibility. Maybe we could refactor it in another JIRA, (maybe could separate setPermission to setPermission, addPermission, removePermission, clearPermission). Does it make sense?
            And I will update this patch soon.

            Tao Jie Tao Jie added a comment - Thank you for your comments, kasha . Noticed there is QueueACL is mapreduce code as well that can be dropped altogether? e.g. mapred QueueManager, many parts (all of?) QueueACL etc. Can we file a follow-up JIRA to drop all of that? I am not sure if such code in mapred.QueueMananger still works today. I prefer to clean up those mapreduce code in another JIRA. onReload: Is there a need to lock the scheduler when setting permissions? Would it be okay to limit the synchronized block to whatever was synchronized before? Have disscussed with templedf , synchronized block added here is to avoid findbugs warning. Actually I will be glad to remove redundant lock here. In setQueueAcls, we seem to initially set to default permissions and then "overwrite" it with final permissions. Is the first one necessary? I quickly looked at implementation of ConfiguredAuthorizationProvider, setPermission's semantics appear to be somewhere between append and overwrite. If it is append, may be we should change that name to addPermission? I also feel a little confused about semantics of setPermission . However this abstract method is introduced by YARN-3100 , and I'm not sure setPermission has implemented in Ranger or Sentry. I prefer to keep setPermission here as CapacityScheduler does to keep compatibility. Maybe we could refactor it in another JIRA, (maybe could separate setPermission to setPermission , addPermission , removePermission , clearPermission ). Does it make sense? And I will update this patch soon.
            Tao Jie Tao Jie added a comment -

            I looked more closely at synchronized in onReload. onReload here is under the lock of AllocationFileLoaderService, but initialization of authorizer is under the lock of FairScheduler. As a result, we'd better to keep all access to authorizer under the same lock of FairScheduler(Actually, initScheduler and reload won't happen at the same time, but they are called in different threads).

            Tao Jie Tao Jie added a comment - I looked more closely at synchronized in onReload. onReload here is under the lock of AllocationFileLoaderService , but initialization of authorizer is under the lock of FairScheduler . As a result, we'd better to keep all access to authorizer under the same lock of FairScheduler (Actually, initScheduler and reload won't happen at the same time, but they are called in different threads).
            zhangqiang2 Qiang Zhang added a comment - - edited

            Hi,everyOne
            this issue is very good,when this patch release?
            I want to test in the ranger with the new yarn version.

            zhangqiang2 Qiang Zhang added a comment - - edited Hi,everyOne this issue is very good,when this patch release? I want to test in the ranger with the new yarn version.
            Tao Jie Tao Jie added a comment -

            Hi kasha, I rebased the patch for review.
            For the semantics of setPermission, I checked code in RANGER, where RangerYarnAuthorizer extends YarnAuthorizationProvider and override method setPermission. As a result, we should keep this method as it used to be.

            Tao Jie Tao Jie added a comment - Hi kasha , I rebased the patch for review. For the semantics of setPermission , I checked code in RANGER, where RangerYarnAuthorizer extends YarnAuthorizationProvider and override method setPermission . As a result, we should keep this method as it used to be.
            hadoopqa Hadoop QA added a comment -
            -1 overall



            Vote Subsystem Runtime Comment
            0 reexec 0m 26s 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 9m 6s trunk passed
            +1 compile 3m 5s trunk passed
            +1 checkstyle 0m 52s trunk passed
            +1 mvnsite 1m 33s trunk passed
            +1 mvneclipse 0m 42s trunk passed
            +1 findbugs 2m 16s trunk passed
            +1 javadoc 1m 10s trunk passed
            0 mvndep 0m 14s Maven dependency ordering for patch
            +1 mvninstall 1m 16s the patch passed
            +1 compile 2m 58s the patch passed
            +1 javac 2m 58s the patch passed
            -0 checkstyle 0m 40s hadoop-yarn-project/hadoop-yarn: The patch generated 2 new + 300 unchanged - 4 fixed = 302 total (was 304)
            +1 mvnsite 1m 9s the patch passed
            +1 mvneclipse 0m 29s the patch passed
            -1 whitespace 0m 0s The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
            +1 findbugs 2m 22s the patch passed
            +1 javadoc 0m 51s the patch passed
            +1 unit 2m 23s hadoop-yarn-common in the patch passed.
            +1 unit 39m 30s hadoop-yarn-server-resourcemanager in the patch passed.
            +1 asflicense 0m 20s The patch does not generate ASF License warnings.
            79m 26s



            Subsystem Report/Notes
            Docker Image:yetus/hadoop:9560f25
            JIRA Issue YARN-4997
            JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12836308/YARN-4997-009.patch
            Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
            uname Linux 00a0e48d85a0 3.13.0-96-generic #143-Ubuntu SMP Mon Aug 29 20:15:20 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
            Build tool maven
            Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
            git revision trunk / 7ba74be
            Default Java 1.8.0_101
            findbugs v3.0.0
            checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13724/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
            whitespace https://builds.apache.org/job/PreCommit-YARN-Build/13724/artifact/patchprocess/whitespace-eol.txt
            Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13724/testReport/
            modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn
            Console output https://builds.apache.org/job/PreCommit-YARN-Build/13724/console
            Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

            This message was automatically generated.

            hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 26s 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 9m 6s trunk passed +1 compile 3m 5s trunk passed +1 checkstyle 0m 52s trunk passed +1 mvnsite 1m 33s trunk passed +1 mvneclipse 0m 42s trunk passed +1 findbugs 2m 16s trunk passed +1 javadoc 1m 10s trunk passed 0 mvndep 0m 14s Maven dependency ordering for patch +1 mvninstall 1m 16s the patch passed +1 compile 2m 58s the patch passed +1 javac 2m 58s the patch passed -0 checkstyle 0m 40s hadoop-yarn-project/hadoop-yarn: The patch generated 2 new + 300 unchanged - 4 fixed = 302 total (was 304) +1 mvnsite 1m 9s the patch passed +1 mvneclipse 0m 29s the patch passed -1 whitespace 0m 0s The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply +1 findbugs 2m 22s the patch passed +1 javadoc 0m 51s the patch passed +1 unit 2m 23s hadoop-yarn-common in the patch passed. +1 unit 39m 30s hadoop-yarn-server-resourcemanager in the patch passed. +1 asflicense 0m 20s The patch does not generate ASF License warnings. 79m 26s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue YARN-4997 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12836308/YARN-4997-009.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 00a0e48d85a0 3.13.0-96-generic #143-Ubuntu SMP Mon Aug 29 20:15:20 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 7ba74be Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13724/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt whitespace https://builds.apache.org/job/PreCommit-YARN-Build/13724/artifact/patchprocess/whitespace-eol.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13724/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/13724/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
            hadoopqa Hadoop QA added a comment -
            +1 overall



            Vote Subsystem Runtime Comment
            0 reexec 0m 15s Docker mode activated.
            +1 @author 0m 0s The patch does not contain any @author tags.
            +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
            0 mvndep 0m 9s Maven dependency ordering for branch
            +1 mvninstall 7m 28s trunk passed
            +1 compile 2m 18s trunk passed
            +1 checkstyle 0m 41s trunk passed
            +1 mvnsite 1m 7s trunk passed
            +1 mvneclipse 0m 30s trunk passed
            +1 findbugs 1m 51s trunk passed
            +1 javadoc 0m 54s trunk passed
            0 mvndep 0m 10s Maven dependency ordering for patch
            +1 mvninstall 1m 0s the patch passed
            +1 compile 2m 26s the patch passed
            +1 javac 2m 26s the patch passed
            -0 checkstyle 0m 42s hadoop-yarn-project/hadoop-yarn: The patch generated 2 new + 300 unchanged - 4 fixed = 302 total (was 304)
            +1 mvnsite 1m 12s the patch passed
            +1 mvneclipse 0m 30s the patch passed
            +1 whitespace 0m 0s The patch has no whitespace issues.
            +1 findbugs 2m 16s the patch passed
            +1 javadoc 0m 47s the patch passed
            +1 unit 2m 18s hadoop-yarn-common in the patch passed.
            +1 unit 35m 3s hadoop-yarn-server-resourcemanager in the patch passed.
            +1 asflicense 0m 19s The patch does not generate ASF License warnings.
            69m 33s



            Subsystem Report/Notes
            Docker Image:yetus/hadoop:9560f25
            JIRA Issue YARN-4997
            JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12836328/YARN-4997-009.patch
            Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
            uname Linux cf2f37136b55 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
            Build tool maven
            Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
            git revision trunk / 7d2d8d2
            Default Java 1.8.0_101
            findbugs v3.0.0
            checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13730/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
            Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13730/testReport/
            modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn
            Console output https://builds.apache.org/job/PreCommit-YARN-Build/13730/console
            Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

            This message was automatically generated.

            hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 15s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 9s Maven dependency ordering for branch +1 mvninstall 7m 28s trunk passed +1 compile 2m 18s trunk passed +1 checkstyle 0m 41s trunk passed +1 mvnsite 1m 7s trunk passed +1 mvneclipse 0m 30s trunk passed +1 findbugs 1m 51s trunk passed +1 javadoc 0m 54s trunk passed 0 mvndep 0m 10s Maven dependency ordering for patch +1 mvninstall 1m 0s the patch passed +1 compile 2m 26s the patch passed +1 javac 2m 26s the patch passed -0 checkstyle 0m 42s hadoop-yarn-project/hadoop-yarn: The patch generated 2 new + 300 unchanged - 4 fixed = 302 total (was 304) +1 mvnsite 1m 12s the patch passed +1 mvneclipse 0m 30s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 2m 16s the patch passed +1 javadoc 0m 47s the patch passed +1 unit 2m 18s hadoop-yarn-common in the patch passed. +1 unit 35m 3s hadoop-yarn-server-resourcemanager in the patch passed. +1 asflicense 0m 19s The patch does not generate ASF License warnings. 69m 33s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue YARN-4997 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12836328/YARN-4997-009.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux cf2f37136b55 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 7d2d8d2 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13730/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13730/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/13730/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.

            Thanks for updating the patch. Since kasha is out for a little while, I'm jumping back in. Looks like you (pl.) decided to drop the synchronized and screw the checkstyle complaint. In the interest of not going in circles, I can live with that. Other minor nits:

            • Can we have AllocationConfiguration.getQueueAcls() wrap the Map in a Collections.unmodifiableMap()? It makes me a little nervous to expose mutable data structures in getters.
            • The javadoc for AllocationFileLoaderService. getDefaultPermissions() should start with a summary sentence that ends with a period. Aside from not being a good summary, the current first sentence is missing the period.
            • In FairScheduler, you messed up the indentation of the first line of applyChildDefaults()'s javadoc.
            templedf Daniel Templeton added a comment - Thanks for updating the patch. Since kasha is out for a little while, I'm jumping back in. Looks like you (pl.) decided to drop the synchronized and screw the checkstyle complaint. In the interest of not going in circles, I can live with that. Other minor nits: Can we have AllocationConfiguration.getQueueAcls() wrap the Map in a Collections.unmodifiableMap() ? It makes me a little nervous to expose mutable data structures in getters. The javadoc for AllocationFileLoaderService. getDefaultPermissions() should start with a summary sentence that ends with a period. Aside from not being a good summary, the current first sentence is missing the period. In FairScheduler , you messed up the indentation of the first line of applyChildDefaults()'s javadoc.
            hadoopqa Hadoop QA added a comment -
            -1 overall



            Vote Subsystem Runtime Comment
            0 reexec 0m 15s Docker mode activated.
            +1 @author 0m 0s The patch does not contain any @author tags.
            +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
            0 mvndep 0m 9s Maven dependency ordering for branch
            +1 mvninstall 6m 55s trunk passed
            +1 compile 5m 23s trunk passed
            +1 checkstyle 0m 49s trunk passed
            +1 mvnsite 1m 25s trunk passed
            +1 mvneclipse 0m 43s trunk passed
            +1 findbugs 2m 6s trunk passed
            +1 javadoc 1m 0s trunk passed
            0 mvndep 0m 10s Maven dependency ordering for patch
            +1 mvninstall 0m 58s the patch passed
            +1 compile 4m 40s the patch passed
            +1 javac 4m 40s the patch passed
            -0 checkstyle 0m 45s hadoop-yarn-project/hadoop-yarn: The patch generated 2 new + 289 unchanged - 4 fixed = 291 total (was 293)
            +1 mvnsite 1m 18s the patch passed
            +1 mvneclipse 0m 40s the patch passed
            +1 whitespace 0m 0s The patch has no whitespace issues.
            +1 findbugs 2m 19s the patch passed
            +1 javadoc 0m 59s the patch passed
            +1 unit 2m 24s hadoop-yarn-common in the patch passed.
            -1 unit 42m 28s hadoop-yarn-server-resourcemanager in the patch failed.
            +1 asflicense 0m 29s The patch does not generate ASF License warnings.
            84m 6s



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



            Subsystem Report/Notes
            Docker Image:yetus/hadoop:a9ad5d6
            JIRA Issue YARN-4997
            JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12840787/YARN-4997-010.patch
            Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
            uname Linux b4e143324bcb 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
            Build tool maven
            Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
            git revision trunk / 47ca9e2
            Default Java 1.8.0_111
            findbugs v3.0.0
            checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/14093/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
            unit https://builds.apache.org/job/PreCommit-YARN-Build/14093/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/14093/testReport/
            modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn
            Console output https://builds.apache.org/job/PreCommit-YARN-Build/14093/console
            Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

            This message was automatically generated.

            hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 15s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 9s Maven dependency ordering for branch +1 mvninstall 6m 55s trunk passed +1 compile 5m 23s trunk passed +1 checkstyle 0m 49s trunk passed +1 mvnsite 1m 25s trunk passed +1 mvneclipse 0m 43s trunk passed +1 findbugs 2m 6s trunk passed +1 javadoc 1m 0s trunk passed 0 mvndep 0m 10s Maven dependency ordering for patch +1 mvninstall 0m 58s the patch passed +1 compile 4m 40s the patch passed +1 javac 4m 40s the patch passed -0 checkstyle 0m 45s hadoop-yarn-project/hadoop-yarn: The patch generated 2 new + 289 unchanged - 4 fixed = 291 total (was 293) +1 mvnsite 1m 18s the patch passed +1 mvneclipse 0m 40s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 2m 19s the patch passed +1 javadoc 0m 59s the patch passed +1 unit 2m 24s hadoop-yarn-common in the patch passed. -1 unit 42m 28s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 29s The patch does not generate ASF License warnings. 84m 6s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.TestRMRestart Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue YARN-4997 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12840787/YARN-4997-010.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux b4e143324bcb 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 47ca9e2 Default Java 1.8.0_111 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/14093/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/14093/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/14093/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/14093/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
            Tao Jie Tao Jie added a comment -

            Updated the patch respect to templedf's comment. The test failure is irrelevant.

            Tao Jie Tao Jie added a comment - Updated the patch respect to templedf 's comment. The test failure is irrelevant.

            Thanks, Tao Jie. Sorry to be fussy, but can we add a first line to the AllocationFileLoaderService.getDefaultPermissions() javadoc, something like, "Returns the list of default permissions." Javadoc takes the first sentence as the summary that is shown in the table of methods.

            templedf Daniel Templeton added a comment - Thanks, Tao Jie . Sorry to be fussy, but can we add a first line to the AllocationFileLoaderService.getDefaultPermissions() javadoc, something like, "Returns the list of default permissions." Javadoc takes the first sentence as the summary that is shown in the table of methods.
            Tao Jie Tao Jie added a comment -

            templedf, thanks for your patient review and sorry for inaccurate understanding of your comments.
            Updated the patch..

            Tao Jie Tao Jie added a comment - templedf , thanks for your patient review and sorry for inaccurate understanding of your comments. Updated the patch..
            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 10s Maven dependency ordering for branch
            +1 mvninstall 7m 1s trunk passed
            +1 compile 4m 58s trunk passed
            +1 checkstyle 0m 51s trunk passed
            +1 mvnsite 1m 33s trunk passed
            +1 mvneclipse 0m 43s trunk passed
            +1 findbugs 2m 23s trunk passed
            +1 javadoc 1m 7s trunk passed
            0 mvndep 0m 11s Maven dependency ordering for patch
            +1 mvninstall 1m 1s the patch passed
            +1 compile 4m 50s the patch passed
            +1 javac 4m 50s the patch passed
            -0 checkstyle 0m 49s hadoop-yarn-project/hadoop-yarn: The patch generated 2 new + 287 unchanged - 4 fixed = 289 total (was 291)
            +1 mvnsite 1m 28s the patch passed
            +1 mvneclipse 0m 44s the patch passed
            +1 whitespace 0m 0s The patch has no whitespace issues.
            +1 findbugs 2m 41s the patch passed
            +1 javadoc 1m 2s the patch passed
            +1 unit 2m 33s hadoop-yarn-common in the patch passed.
            -1 unit 39m 50s hadoop-yarn-server-resourcemanager in the patch failed.
            +1 asflicense 0m 29s The patch does not generate ASF License warnings.
            82m 46s



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



            Subsystem Report/Notes
            Docker Image:yetus/hadoop:a9ad5d6
            JIRA Issue YARN-4997
            JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12840995/YARN-4997-011.patch
            Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
            uname Linux 81a82c6e2ca7 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
            Build tool maven
            Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
            git revision trunk / aeecfa2
            Default Java 1.8.0_111
            findbugs v3.0.0
            checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/14116/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
            unit https://builds.apache.org/job/PreCommit-YARN-Build/14116/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/14116/testReport/
            modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn
            Console output https://builds.apache.org/job/PreCommit-YARN-Build/14116/console
            Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

            This message was automatically generated.

            hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 16s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 10s Maven dependency ordering for branch +1 mvninstall 7m 1s trunk passed +1 compile 4m 58s trunk passed +1 checkstyle 0m 51s trunk passed +1 mvnsite 1m 33s trunk passed +1 mvneclipse 0m 43s trunk passed +1 findbugs 2m 23s trunk passed +1 javadoc 1m 7s trunk passed 0 mvndep 0m 11s Maven dependency ordering for patch +1 mvninstall 1m 1s the patch passed +1 compile 4m 50s the patch passed +1 javac 4m 50s the patch passed -0 checkstyle 0m 49s hadoop-yarn-project/hadoop-yarn: The patch generated 2 new + 287 unchanged - 4 fixed = 289 total (was 291) +1 mvnsite 1m 28s the patch passed +1 mvneclipse 0m 44s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 2m 41s the patch passed +1 javadoc 1m 2s the patch passed +1 unit 2m 33s hadoop-yarn-common in the patch passed. -1 unit 39m 50s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 29s The patch does not generate ASF License warnings. 82m 46s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.TestRMRestart Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue YARN-4997 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12840995/YARN-4997-011.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 81a82c6e2ca7 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / aeecfa2 Default Java 1.8.0_111 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/14116/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/14116/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/14116/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/14116/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.

            I'll let you slide on the method length checkstyle complaints, and it looks like the test failure is unrelated. +1

            templedf Daniel Templeton added a comment - I'll let you slide on the method length checkstyle complaints, and it looks like the test failure is unrelated. +1

            Thanks for all the patches, Tao Jie, and thanks, kasha, for the reviews. Committed to trunk.

            templedf Daniel Templeton added a comment - Thanks for all the patches, Tao Jie , and thanks, kasha , for the reviews. Committed to trunk.

            Tao Jie, would you mind checking if the change you made in the FairScheduler test needs to be made in any other tests? If so, please file a new JIRA.

            templedf Daniel Templeton added a comment - Tao Jie , would you mind checking if the change you made in the FairScheduler test needs to be made in any other tests? If so, please file a new JIRA.
            hudson Hudson added a comment -

            SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10915 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10915/)
            YARN-4997. Update fair scheduler to use pluggable auth provider (templedf: rev b3befc021b0e2d63d1a3710ea450797d1129f1f5)

            • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/security/YarnAuthorizationProvider.java
            • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/FSQueue.java
            • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/AllocationConfiguration.java
            • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/FairScheduler.java
            • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/TestFairScheduler.java
            • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/AllocationFileLoaderService.java
            hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10915 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10915/ ) YARN-4997 . Update fair scheduler to use pluggable auth provider (templedf: rev b3befc021b0e2d63d1a3710ea450797d1129f1f5) (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/security/YarnAuthorizationProvider.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/FSQueue.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/AllocationConfiguration.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/FairScheduler.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/TestFairScheduler.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/AllocationFileLoaderService.java
            sershe Sergey Shelukhin added a comment - - edited

            this break the following piece of code in Hive that seems to be determining the default queue (I am not familiar with all the APIs called here), because the listener interface is made invisible while the setReloadListener API that it is passed into is still public.

            AllocationFileLoaderService allocsLoader = new AllocationFileLoaderService();
                allocsLoader.init(conf);
                allocsLoader.setReloadListener(new AllocationFileLoaderService.Listener() {
                  @Override
                  public void onReload(AllocationConfiguration allocs) {
                    allocConf.set(allocs);
                  }
                });
                try {
                  allocsLoader.reloadAllocations();
                } catch (Exception ex) {
                  throw new IOException("Failed to load queue allocations", ex);
                }
                if (allocConf.get() == null) {
                  allocConf.set(new AllocationConfiguration(conf));
                }
                QueuePlacementPolicy queuePolicy = allocConf.get().getPlacementPolicy();
                if (queuePolicy != null) {
                  requestedQueue = queuePolicy.assignAppToQueue(requestedQueue, userName);
            

            Can you recommend a way to utilize reloadAllocations and get the queue (or placement policy, or allocConf) without invoking this? Or some other workaround.
            Or otherwise can you please change Listener back to public?

            Checking the code in 2.6, it seems like we can getConfig from the loader, but whatever reloadAllocations might derive from the placement policy element in the xml file is not accessible.

            sershe Sergey Shelukhin added a comment - - edited this break the following piece of code in Hive that seems to be determining the default queue (I am not familiar with all the APIs called here), because the listener interface is made invisible while the setReloadListener API that it is passed into is still public. AllocationFileLoaderService allocsLoader = new AllocationFileLoaderService(); allocsLoader.init(conf); allocsLoader.setReloadListener(new AllocationFileLoaderService.Listener() { @Override public void onReload(AllocationConfiguration allocs) { allocConf.set(allocs); } }); try { allocsLoader.reloadAllocations(); } catch (Exception ex) { throw new IOException("Failed to load queue allocations", ex); } if (allocConf.get() == null) { allocConf.set(new AllocationConfiguration(conf)); } QueuePlacementPolicy queuePolicy = allocConf.get().getPlacementPolicy(); if (queuePolicy != null) { requestedQueue = queuePolicy.assignAppToQueue(requestedQueue, userName); Can you recommend a way to utilize reloadAllocations and get the queue (or placement policy, or allocConf) without invoking this? Or some other workaround. Or otherwise can you please change Listener back to public? Checking the code in 2.6, it seems like we can getConfig from the loader, but whatever reloadAllocations might derive from the placement policy element in the xml file is not accessible.
            Tao Jie Tao Jie added a comment -

            sershe, we have discussed about the modifier of interface Listener earlier in this patch. We removed public on interface Listener since it got a findbugs warning, and found public here is not necessary.
            Since this breaks Hive code, I prefer to add public back to Listener.

            Tao Jie Tao Jie added a comment - sershe , we have discussed about the modifier of interface Listener earlier in this patch. We removed public on interface Listener since it got a findbugs warning, and found public here is not necessary. Since this breaks Hive code, I prefer to add public back to Listener.

            We'd be ok with a different way; our existing code, as is, seems very convoluted to me. All we need is to get the correct placement policy (presumably, based on both config and the xml file that reloadAllocations uses).

            sershe Sergey Shelukhin added a comment - We'd be ok with a different way; our existing code, as is, seems very convoluted to me. All we need is to get the correct placement policy (presumably, based on both config and the xml file that reloadAllocations uses).
            Tao Jie Tao Jie added a comment -

            Thank you sershe, I have created another JIRA YARN-6000 to handle this.
            It's OK if you change Hive code to walk around and make the logic more clear. However once it break the code, we should get it fixed. Otherwise, when we update the Hadoop version, but not Hive(maybe have not released yet), it would fail.

            Tao Jie Tao Jie added a comment - Thank you sershe , I have created another JIRA YARN-6000 to handle this. It's OK if you change Hive code to walk around and make the logic more clear. However once it break the code, we should get it fixed. Otherwise, when we update the Hadoop version, but not Hive(maybe have not released yet), it would fail.

            People

              Tao Jie Tao Jie
              templedf Daniel Templeton
              Votes:
              2 Vote for this issue
              Watchers:
              9 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: