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

Update fair scheduler to use pluggable auth provider

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.8.0
    • Fix Version/s: 2.9.0, 3.0.0-alpha2
    • Component/s: fairscheduler
    • Labels:
      None
    • Hadoop Flags:
      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.

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

        Issue Links

          Activity

          Hide
          Tao Jie Tao Jie added a comment -

          Thank you Sergey Shelukhin, 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.

          Show
          Tao Jie Tao Jie added a comment - Thank you Sergey Shelukhin , 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.
          Hide
          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).

          Show
          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).
          Hide
          Tao Jie Tao Jie added a comment -

          Sergey Shelukhin, 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.

          Show
          Tao Jie Tao Jie added a comment - Sergey Shelukhin , 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.
          Hide
          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.

          Show
          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.
          Hide
          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
          Show
          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
          Hide
          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.

          Show
          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.
          Hide
          templedf Daniel Templeton added a comment -

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

          Show
          templedf Daniel Templeton added a comment - Thanks for all the patches, Tao Jie , and thanks, Karthik Kambatla , for the reviews. Committed to trunk.
          Hide
          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

          Show
          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
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 16s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          0 mvndep 0m 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.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 16s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 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.
          Hide
          Tao Jie Tao Jie added a comment -

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

          Show
          Tao Jie Tao Jie added a comment - Daniel Templeton , thanks for your patient review and sorry for inaccurate understanding of your comments. Updated the patch..
          Hide
          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.

          Show
          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.
          Hide
          Tao Jie Tao Jie added a comment -

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

          Show
          Tao Jie Tao Jie added a comment - Updated the patch respect to Daniel Templeton 's comment. The test failure is irrelevant.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 15s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          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.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 15s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 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.
          Hide
          templedf Daniel Templeton added a comment -

          Thanks for updating the patch. Since Karthik Kambatla 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.
          Show
          templedf Daniel Templeton added a comment - Thanks for updating the patch. Since Karthik Kambatla 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.
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 15s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          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.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 15s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 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.
          Hide
          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.

          Show
          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.
          Hide
          Tao Jie Tao Jie added a comment -

          Hi Karthik Kambatla, 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.

          Show
          Tao Jie Tao Jie added a comment - Hi Karthik Kambatla , 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.
          Hide
          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.

          Show
          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.
          Hide
          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).

          Show
          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).
          Hide
          Tao Jie Tao Jie added a comment -

          Thank you for your comments, Karthik Kambatla.

          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 Daniel Templeton, 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.

          Show
          Tao Jie Tao Jie added a comment - Thank you for your comments, Karthik Kambatla . 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 Daniel Templeton , 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.
          Hide
          kasha Karthik Kambatla added a comment -

          Canceling patch..

          Show
          kasha Karthik Kambatla added a comment - Canceling patch..
          Hide
          kasha Karthik Kambatla added a comment -

          Thanks for working on this, taojie.

          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?
          Show
          kasha Karthik Kambatla added a comment - Thanks for working on this, taojie . 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?
          Hide
          Tao Jie Tao Jie added a comment -

          Thank you Daniel Templeton for your review! Karthik Kambatla, could you please give it a review?

          Show
          Tao Jie Tao Jie added a comment - Thank you Daniel Templeton for your review! Karthik Kambatla , could you please give it a review?
          Hide
          templedf Daniel Templeton added a comment -

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

          Show
          templedf Daniel Templeton added a comment - +1 (non-binding). Thanks for all the patches, Tao Jie !
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 18s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          0 mvndep 0m 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.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 18s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 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.
          Hide
          Tao Jie Tao Jie added a comment -

          Oops!Fix this misspelling and update the patch.

          Show
          Tao Jie Tao Jie added a comment - Oops!Fix this misspelling and update the patch.
          Hide
          templedf Daniel Templeton added a comment -

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

          Show
          templedf Daniel Templeton added a comment - LGTM! Except... I apologize for not seeing this earlier. Your destroy() method is misspelled as destory() .
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 12s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch 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.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 12s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch 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.
          Hide
          Tao Jie Tao Jie added a comment -

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

          Show
          Tao Jie Tao Jie added a comment - Patch updated. Fix findbugs with add synchronized on setQueueAcls()
          Hide
          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.

          Show
          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.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 19s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          0 mvndep 0m 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.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 19s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 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.
          Hide
          Tao Jie Tao Jie added a comment -

          Thanks for reply, Daniel Templeton. 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

          Show
          Tao Jie Tao Jie added a comment - Thanks for reply, Daniel Templeton . 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
          Hide
          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?)

          Show
          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?)
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 18s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          0 mvndep 0m 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.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 18s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 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.
          Hide
          Tao Jie Tao Jie added a comment -

          Daniel Templeton, 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!

          Show
          Tao Jie Tao Jie added a comment - Daniel Templeton , 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!
          Hide
          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.

          Show
          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.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 15s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          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.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 15s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 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.
          Hide
          Tao Jie Tao Jie added a comment -

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

          Show
          Tao Jie Tao Jie added a comment - Daniel Templeton , thanks for your review! And attached patch with tiny fix associated with your comments.
          Hide
          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.

          Show
          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.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 16s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          0 mvndep 0m 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.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 16s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 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.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 15s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 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.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 15s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 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.
          Hide
          Tao Jie Tao Jie added a comment -

          Attached file fixed findbugs and unittests.

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



          Vote Subsystem Runtime Comment
          0 reexec 0m 11s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch 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.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 11s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch 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.
          Hide
          Tao Jie Tao Jie added a comment -

          Fix for review.

          Show
          Tao Jie Tao Jie added a comment - Fix for review.
          Hide
          templedf Daniel Templeton added a comment -

          Tao Jie, you're welcome to take it.

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

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

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

            People

            • Assignee:
              Tao Jie Tao Jie
              Reporter:
              templedf Daniel Templeton
            • Votes:
              2 Vote for this issue
              Watchers:
              10 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development