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

Reduce RM app memory footprint once app has completed

    Details

    • Hadoop Flags:
      Reviewed

      Description

      The ResourceManager holds onto a configurable number of completed applications (yarn.resource.max-completed-applications, defaults to 10000), and the memory footprint of these completed applications can be significant. For example, the submissionContext in RMAppImpl contains references to protocolbuffer objects and other items that probably aren't necessary to keep around once the application has completed. We could significantly reduce the memory footprint of the RM by releasing objects that are no longer necessary once an application completes.

      1. YARN-65.001.patch
        5 kB
        Manikandan R
      2. YARN-65.002.patch
        4 kB
        Manikandan R
      3. YARN-65.003.patch
        3 kB
        Manikandan R
      4. YARN-65.004.patch
        22 kB
        Manikandan R
      5. YARN-65.005.patch
        21 kB
        Manikandan R
      6. YARN-65.006.patch
        21 kB
        Manikandan R
      7. YARN-65.007.patch
        21 kB
        Manikandan R
      8. YARN-65.008.patch
        61 kB
        Manikandan R
      9. YARN-65.009.patch
        63 kB
        Manikandan R
      10. YARN-65.010.patch
        63 kB
        Manikandan R
      11. YARN-65.011.patch
        59 kB
        Manikandan R
      12. YARN-65.012.patch
        58 kB
        Manikandan R
      13. YARN-65.013.patch
        58 kB
        Manikandan R
      14. YARN-65.014.patch
        55 kB
        Manikandan R
      15. YARN-65.015.patch
        55 kB
        Manikandan R

        Issue Links

          Activity

          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks for reviewing and committing the patch Rohith Sharma K S, but was wondering why not for 2.7 branch (2.7.5 version release) too, as many want to store more applications in the absence of reliable ATS in the existing deployed clusters. Thoughts?

          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks for reviewing and committing the patch Rohith Sharma K S , but was wondering why not for 2.7 branch (2.7.5 version release) too, as many want to store more applications in the absence of reliable ATS in the existing deployed clusters. Thoughts?
          Hide
          manirajv06@gmail.com Manikandan R added a comment -

          Rohith Sharma K S Naganarasimha G R Thanks for your reviews and commit

          Show
          manirajv06@gmail.com Manikandan R added a comment - Rohith Sharma K S Naganarasimha G R Thanks for your reviews and commit
          Hide
          rohithsharma Rohith Sharma K S added a comment -

          committed to trunk/branch-3/branch-2! thanks to Manikandan R for the patch and to Naganarasimha Garla for additional review!

          Show
          rohithsharma Rohith Sharma K S added a comment - committed to trunk/branch-3/branch-2! thanks to Manikandan R for the patch and to Naganarasimha Garla for additional review!
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #12976 (See https://builds.apache.org/job/Hadoop-trunk-Commit/12976/)
          YARN-65. Reduce RM app memory footprint once app has completed. (rohithsharmaks: rev 06e5a7b5cf141420d3a411088b87acba72e68cad)

          • (delete) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/MockRMMemoryStateStore.java
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/MockRM.java
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestWorkPreservingRMRestart.java
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/security/TestRMDelegationTokens.java
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/RMAppImpl.java
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestRMHA.java
          • (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/MockMemoryRMStateStore.java
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestContainerResourceUsage.java
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/TestRMAppTransitions.java
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestApplicationCleanup.java
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestRMRestart.java
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/applicationsmanager/TestAMRestart.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #12976 (See https://builds.apache.org/job/Hadoop-trunk-Commit/12976/ ) YARN-65 . Reduce RM app memory footprint once app has completed. (rohithsharmaks: rev 06e5a7b5cf141420d3a411088b87acba72e68cad) (delete) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/MockRMMemoryStateStore.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/MockRM.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestWorkPreservingRMRestart.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/security/TestRMDelegationTokens.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/RMAppImpl.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestRMHA.java (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/MockMemoryRMStateStore.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestContainerResourceUsage.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/TestRMAppTransitions.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestApplicationCleanup.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestRMRestart.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/applicationsmanager/TestAMRestart.java
          Hide
          rohithsharma Rohith Sharma K S added a comment -

          committing shortly

          Show
          rohithsharma Rohith Sharma K S added a comment - committing shortly
          Hide
          rohithsharma Rohith Sharma K S added a comment -

          +1 lgtm, Failed test cases are known failures! I will commit it later of today if no more objections

          Show
          rohithsharma Rohith Sharma K S added a comment - +1 lgtm, Failed test cases are known failures! I will commit it later of today if no more objections
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 17s Docker mode activated.
                Prechecks
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 11 new or modified test files.
                trunk Compile Tests
          +1 mvninstall 15m 0s trunk passed
          +1 compile 0m 35s trunk passed
          +1 checkstyle 0m 32s trunk passed
          +1 mvnsite 0m 37s trunk passed
          +1 findbugs 1m 7s trunk passed
          +1 javadoc 0m 22s trunk passed
                Patch Compile Tests
          +1 mvninstall 0m 36s the patch passed
          +1 compile 0m 34s the patch passed
          +1 javac 0m 34s the patch passed
          +1 checkstyle 0m 29s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 0 new + 444 unchanged - 1 fixed = 444 total (was 445)
          +1 mvnsite 0m 36s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 9s the patch passed
          +1 javadoc 0m 19s the patch passed
                Other Tests
          -1 unit 44m 42s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 asflicense 0m 16s The patch does not generate ASF License warnings.
          68m 33s



          Reason Tests
          Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.capacity.TestContainerAllocation
            hadoop.yarn.server.resourcemanager.scheduler.fair.TestFSAppStarvation



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:71bbb86
          JIRA Issue YARN-65
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12888747/YARN-65.015.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 1665017242bf 3.13.0-119-generic #166-Ubuntu SMP Wed May 3 12:18:55 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 415e5a1
          Default Java 1.8.0_144
          findbugs v3.1.0-RC1
          unit https://builds.apache.org/job/PreCommit-YARN-Build/17613/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/17613/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/17613/console
          Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 17s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 11 new or modified test files.       trunk Compile Tests +1 mvninstall 15m 0s trunk passed +1 compile 0m 35s trunk passed +1 checkstyle 0m 32s trunk passed +1 mvnsite 0m 37s trunk passed +1 findbugs 1m 7s trunk passed +1 javadoc 0m 22s trunk passed       Patch Compile Tests +1 mvninstall 0m 36s the patch passed +1 compile 0m 34s the patch passed +1 javac 0m 34s the patch passed +1 checkstyle 0m 29s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 0 new + 444 unchanged - 1 fixed = 444 total (was 445) +1 mvnsite 0m 36s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 9s the patch passed +1 javadoc 0m 19s the patch passed       Other Tests -1 unit 44m 42s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 68m 33s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.capacity.TestContainerAllocation   hadoop.yarn.server.resourcemanager.scheduler.fair.TestFSAppStarvation Subsystem Report/Notes Docker Image:yetus/hadoop:71bbb86 JIRA Issue YARN-65 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12888747/YARN-65.015.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 1665017242bf 3.13.0-119-generic #166-Ubuntu SMP Wed May 3 12:18:55 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 415e5a1 Default Java 1.8.0_144 findbugs v3.1.0-RC1 unit https://builds.apache.org/job/PreCommit-YARN-Build/17613/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/17613/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/17613/console Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          manirajv06@gmail.com Manikandan R added a comment -

          Fixed checkstyle and whitespace issues. Junit failures is not related to this patch as explained earlier.

          Show
          manirajv06@gmail.com Manikandan R added a comment - Fixed checkstyle and whitespace issues. Junit failures is not related to this patch as explained earlier.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 14s Docker mode activated.
                Prechecks
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 11 new or modified test files.
                trunk Compile Tests
          +1 mvninstall 13m 5s trunk passed
          +1 compile 0m 35s trunk passed
          +1 checkstyle 0m 31s trunk passed
          +1 mvnsite 0m 37s trunk passed
          +1 findbugs 0m 59s trunk passed
          +1 javadoc 0m 21s trunk passed
                Patch Compile Tests
          +1 mvninstall 0m 33s the patch passed
          +1 compile 0m 32s the patch passed
          +1 javac 0m 32s the patch passed
          -0 checkstyle 0m 29s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 3 new + 444 unchanged - 1 fixed = 447 total (was 445)
          +1 mvnsite 0m 34s the patch passed
          -1 whitespace 0m 0s The patch has 4 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
          +1 findbugs 1m 6s the patch passed
          +1 javadoc 0m 18s the patch passed
                Other Tests
          -1 unit 44m 24s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 asflicense 0m 14s The patch does not generate ASF License warnings.
          65m 57s



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



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:71bbb86
          JIRA Issue YARN-65
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12888706/YARN-65.014.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 7813aac3c740 3.13.0-119-generic #166-Ubuntu SMP Wed May 3 12:18:55 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / cda3378
          Default Java 1.8.0_144
          findbugs v3.1.0-RC1
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/17610/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          whitespace https://builds.apache.org/job/PreCommit-YARN-Build/17610/artifact/patchprocess/whitespace-eol.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/17610/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/17610/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/17610/console
          Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 14s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 11 new or modified test files.       trunk Compile Tests +1 mvninstall 13m 5s trunk passed +1 compile 0m 35s trunk passed +1 checkstyle 0m 31s trunk passed +1 mvnsite 0m 37s trunk passed +1 findbugs 0m 59s trunk passed +1 javadoc 0m 21s trunk passed       Patch Compile Tests +1 mvninstall 0m 33s the patch passed +1 compile 0m 32s the patch passed +1 javac 0m 32s the patch passed -0 checkstyle 0m 29s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 3 new + 444 unchanged - 1 fixed = 447 total (was 445) +1 mvnsite 0m 34s the patch passed -1 whitespace 0m 0s The patch has 4 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply +1 findbugs 1m 6s the patch passed +1 javadoc 0m 18s the patch passed       Other Tests -1 unit 44m 24s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 14s The patch does not generate ASF License warnings. 65m 57s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.capacity.TestContainerAllocation Subsystem Report/Notes Docker Image:yetus/hadoop:71bbb86 JIRA Issue YARN-65 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12888706/YARN-65.014.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 7813aac3c740 3.13.0-119-generic #166-Ubuntu SMP Wed May 3 12:18:55 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / cda3378 Default Java 1.8.0_144 findbugs v3.1.0-RC1 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/17610/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt whitespace https://builds.apache.org/job/PreCommit-YARN-Build/17610/artifact/patchprocess/whitespace-eol.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/17610/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/17610/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/17610/console Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          manirajv06@gmail.com Manikandan R added a comment -

          Rohith Sharma K S Thanks for very detailed reviews and trying out better solutions as well.

          1,2:

          Yes, MockMemoryRMStateStore#clone can be avoided and use cloned application submission context inside MockMemoryRMStateStore#loadState itself.

          3:

          Those two test cases were using old state (retrieved using MockMemoryRMStateStore#loadState) after RM restarts to reinstate the old state info and trying to load into the state store again. Now, this old state has to repopulated with cloned application submission context similar to MockMemoryRMStateStore#loadState flow and can be used further.

          Attached patch for the same.

          Show
          manirajv06@gmail.com Manikandan R added a comment - Rohith Sharma K S Thanks for very detailed reviews and trying out better solutions as well. 1,2: Yes, MockMemoryRMStateStore#clone can be avoided and use cloned application submission context inside MockMemoryRMStateStore#loadState itself. 3: Those two test cases were using old state (retrieved using MockMemoryRMStateStore#loadState) after RM restarts to reinstate the old state info and trying to load into the state store again. Now, this old state has to repopulated with cloned application submission context similar to MockMemoryRMStateStore#loadState flow and can be used further. Attached patch for the same.
          Hide
          rohithsharma Rohith Sharma K S added a comment -

          thanks Manikandan R for the updated patch! Overall your approach looks fine to me.
          Couple of comments on tests

          1. In MockMemoryRMStateStore#loadState, it is not required to create newStateData. Just iterate over applications and replace cloned application submission context like below
              for(Entry<ApplicationId, ApplicationStateData> state :
                    cloneState.getApplicationState().entrySet()) {
                  ApplicationStateData oldStateData = state.getValue();
                  oldStateData.setApplicationSubmissionContext(
                    this.appSubCtxtCopy.get(state.getKey()));
                }
            
          2. Given first comment is handled, then memory#clone is not required to call in each test cases. The instance of (MockMemoryRMStateStore) rm1.getRMStateStore(); can be passed! By this way, MockMemoryRMStateStore#clone method can also be removed.
          3. TestWorkPreservingRMRestart test fails after above change which need to be debugged. At high level. RMAppState object is being modified even though loadstate return copy!
          Show
          rohithsharma Rohith Sharma K S added a comment - thanks Manikandan R for the updated patch! Overall your approach looks fine to me. Couple of comments on tests In MockMemoryRMStateStore#loadState, it is not required to create newStateData. Just iterate over applications and replace cloned application submission context like below for (Entry<ApplicationId, ApplicationStateData> state : cloneState.getApplicationState().entrySet()) { ApplicationStateData oldStateData = state.getValue(); oldStateData.setApplicationSubmissionContext( this .appSubCtxtCopy.get(state.getKey())); } Given first comment is handled, then memory#clone is not required to call in each test cases. The instance of (MockMemoryRMStateStore) rm1.getRMStateStore(); can be passed! By this way, MockMemoryRMStateStore#clone method can also be removed. TestWorkPreservingRMRestart test fails after above change which need to be debugged. At high level. RMAppState object is being modified even though loadstate return copy!
          Hide
          manirajv06@gmail.com Manikandan R added a comment -

          Junit failures are not related to this patch.

          Show
          manirajv06@gmail.com Manikandan R added a comment - Junit failures are not related to this patch.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 19s Docker mode activated.
                Prechecks
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 11 new or modified test files.
                trunk Compile Tests
          +1 mvninstall 18m 19s trunk passed
          +1 compile 0m 34s trunk passed
          +1 checkstyle 0m 32s trunk passed
          +1 mvnsite 0m 37s trunk passed
          +1 findbugs 1m 3s trunk passed
          +1 javadoc 0m 21s trunk passed
                Patch Compile Tests
          +1 mvninstall 0m 33s the patch passed
          +1 compile 0m 31s the patch passed
          +1 javac 0m 31s the patch passed
          +1 checkstyle 0m 29s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 0 new + 443 unchanged - 1 fixed = 443 total (was 444)
          +1 mvnsite 0m 34s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 4s the patch passed
          +1 javadoc 0m 18s the patch passed
                Other Tests
          -1 unit 43m 46s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 asflicense 0m 16s The patch does not generate ASF License warnings.
          70m 37s



          Reason Tests
          Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.capacity.TestIncreaseAllocationExpirer
            hadoop.yarn.server.resourcemanager.scheduler.capacity.TestContainerAllocation
            hadoop.yarn.server.resourcemanager.scheduler.TestAbstractYarnScheduler
            hadoop.yarn.server.resourcemanager.scheduler.fair.TestFSAppStarvation



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:71bbb86
          JIRA Issue YARN-65
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12886302/YARN-65.013.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux e29617026d6c 3.13.0-129-generic #178-Ubuntu SMP Fri Aug 11 12:48:20 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / aa4b6fb
          Default Java 1.8.0_144
          findbugs v3.1.0-RC1
          unit https://builds.apache.org/job/PreCommit-YARN-Build/17393/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/17393/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/17393/console
          Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 19s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 11 new or modified test files.       trunk Compile Tests +1 mvninstall 18m 19s trunk passed +1 compile 0m 34s trunk passed +1 checkstyle 0m 32s trunk passed +1 mvnsite 0m 37s trunk passed +1 findbugs 1m 3s trunk passed +1 javadoc 0m 21s trunk passed       Patch Compile Tests +1 mvninstall 0m 33s the patch passed +1 compile 0m 31s the patch passed +1 javac 0m 31s the patch passed +1 checkstyle 0m 29s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 0 new + 443 unchanged - 1 fixed = 443 total (was 444) +1 mvnsite 0m 34s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 4s the patch passed +1 javadoc 0m 18s the patch passed       Other Tests -1 unit 43m 46s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 70m 37s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.capacity.TestIncreaseAllocationExpirer   hadoop.yarn.server.resourcemanager.scheduler.capacity.TestContainerAllocation   hadoop.yarn.server.resourcemanager.scheduler.TestAbstractYarnScheduler   hadoop.yarn.server.resourcemanager.scheduler.fair.TestFSAppStarvation Subsystem Report/Notes Docker Image:yetus/hadoop:71bbb86 JIRA Issue YARN-65 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12886302/YARN-65.013.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux e29617026d6c 3.13.0-129-generic #178-Ubuntu SMP Fri Aug 11 12:48:20 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / aa4b6fb Default Java 1.8.0_144 findbugs v3.1.0-RC1 unit https://builds.apache.org/job/PreCommit-YARN-Build/17393/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/17393/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/17393/console Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          manirajv06@gmail.com Manikandan R added a comment -

          Fixed TestApplicationLifetimeMonitor unit failure and attached patch for the same.

          Show
          manirajv06@gmail.com Manikandan R added a comment - Fixed TestApplicationLifetimeMonitor unit failure and attached patch for the same.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 18s Docker mode activated.
                Prechecks
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 11 new or modified test files.
                trunk Compile Tests
          +1 mvninstall 14m 0s trunk passed
          +1 compile 0m 35s trunk passed
          +1 checkstyle 0m 31s trunk passed
          +1 mvnsite 0m 38s trunk passed
          +1 findbugs 1m 1s trunk passed
          +1 javadoc 0m 21s trunk passed
                Patch Compile Tests
          +1 mvninstall 0m 34s the patch passed
          +1 compile 0m 32s the patch passed
          +1 javac 0m 32s the patch passed
          +1 checkstyle 0m 28s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 0 new + 444 unchanged - 1 fixed = 444 total (was 445)
          +1 mvnsite 0m 34s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 7s the patch passed
          +1 javadoc 0m 20s the patch passed
                Other Tests
          -1 unit 44m 37s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 asflicense 0m 13s The patch does not generate ASF License warnings.
          67m 9s



          Reason Tests
          Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.capacity.TestContainerAllocation
            hadoop.yarn.server.resourcemanager.rmapp.TestApplicationLifetimeMonitor



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:71bbb86
          JIRA Issue YARN-65
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12885998/YARN-65.012.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 64377a9f54c2 3.13.0-119-generic #166-Ubuntu SMP Wed May 3 12:18:55 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / b3a4d7d
          Default Java 1.8.0_144
          findbugs v3.1.0-RC1
          unit https://builds.apache.org/job/PreCommit-YARN-Build/17354/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/17354/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/17354/console
          Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 18s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 11 new or modified test files.       trunk Compile Tests +1 mvninstall 14m 0s trunk passed +1 compile 0m 35s trunk passed +1 checkstyle 0m 31s trunk passed +1 mvnsite 0m 38s trunk passed +1 findbugs 1m 1s trunk passed +1 javadoc 0m 21s trunk passed       Patch Compile Tests +1 mvninstall 0m 34s the patch passed +1 compile 0m 32s the patch passed +1 javac 0m 32s the patch passed +1 checkstyle 0m 28s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 0 new + 444 unchanged - 1 fixed = 444 total (was 445) +1 mvnsite 0m 34s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 7s the patch passed +1 javadoc 0m 20s the patch passed       Other Tests -1 unit 44m 37s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 13s The patch does not generate ASF License warnings. 67m 9s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.capacity.TestContainerAllocation   hadoop.yarn.server.resourcemanager.rmapp.TestApplicationLifetimeMonitor Subsystem Report/Notes Docker Image:yetus/hadoop:71bbb86 JIRA Issue YARN-65 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12885998/YARN-65.012.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 64377a9f54c2 3.13.0-119-generic #166-Ubuntu SMP Wed May 3 12:18:55 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / b3a4d7d Default Java 1.8.0_144 findbugs v3.1.0-RC1 unit https://builds.apache.org/job/PreCommit-YARN-Build/17354/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/17354/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/17354/console Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          manirajv06@gmail.com Manikandan R added a comment -

          Fixed checkstyle issues and attached patch. TestContainerAllocation failure is not related to this patch as described in https://issues.apache.org/jira/browse/YARN-7044. I think TestApplicationLifetimeMonitor failure is not related as errors are triggered from MockRM#waitForState, but will double check and update here.

          Show
          manirajv06@gmail.com Manikandan R added a comment - Fixed checkstyle issues and attached patch. TestContainerAllocation failure is not related to this patch as described in https://issues.apache.org/jira/browse/YARN-7044 . I think TestApplicationLifetimeMonitor failure is not related as errors are triggered from MockRM#waitForState , but will double check and update here.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 21s Docker mode activated.
                Prechecks
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 11 new or modified test files.
                trunk Compile Tests
          +1 mvninstall 14m 12s trunk passed
          +1 compile 0m 35s trunk passed
          +1 checkstyle 0m 28s trunk passed
          +1 mvnsite 0m 36s trunk passed
          +1 findbugs 1m 1s trunk passed
          +1 javadoc 0m 22s trunk passed
                Patch Compile Tests
          +1 mvninstall 0m 35s the patch passed
          +1 compile 0m 36s the patch passed
          +1 javac 0m 36s the patch passed
          -0 checkstyle 0m 30s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 6 new + 443 unchanged - 1 fixed = 449 total (was 444)
          +1 mvnsite 0m 36s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 15s the patch passed
          +1 javadoc 0m 20s the patch passed
                Other Tests
          -1 unit 45m 14s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 asflicense 0m 11s The patch does not generate ASF License warnings.
          68m 10s



          Reason Tests
          Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.capacity.TestContainerAllocation
            hadoop.yarn.server.resourcemanager.rmapp.TestApplicationLifetimeMonitor



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:71bbb86
          JIRA Issue YARN-65
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12885869/YARN-65.011.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 45ebd1779850 4.4.0-43-generic #63-Ubuntu SMP Wed Oct 12 13:48:03 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 13eda50
          Default Java 1.8.0_144
          findbugs v3.1.0-RC1
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/17337/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/17337/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/17337/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/17337/console
          Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 21s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 11 new or modified test files.       trunk Compile Tests +1 mvninstall 14m 12s trunk passed +1 compile 0m 35s trunk passed +1 checkstyle 0m 28s trunk passed +1 mvnsite 0m 36s trunk passed +1 findbugs 1m 1s trunk passed +1 javadoc 0m 22s trunk passed       Patch Compile Tests +1 mvninstall 0m 35s the patch passed +1 compile 0m 36s the patch passed +1 javac 0m 36s the patch passed -0 checkstyle 0m 30s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 6 new + 443 unchanged - 1 fixed = 449 total (was 444) +1 mvnsite 0m 36s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 15s the patch passed +1 javadoc 0m 20s the patch passed       Other Tests -1 unit 45m 14s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 11s The patch does not generate ASF License warnings. 68m 10s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.capacity.TestContainerAllocation   hadoop.yarn.server.resourcemanager.rmapp.TestApplicationLifetimeMonitor Subsystem Report/Notes Docker Image:yetus/hadoop:71bbb86 JIRA Issue YARN-65 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12885869/YARN-65.011.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 45ebd1779850 4.4.0-43-generic #63-Ubuntu SMP Wed Oct 12 13:48:03 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 13eda50 Default Java 1.8.0_144 findbugs v3.1.0-RC1 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/17337/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/17337/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/17337/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/17337/console Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          manirajv06@gmail.com Manikandan R added a comment -

          Rohith Sharma K S, Naganarasimha G R Thanks for review.

          Cloned ApplicationSubmissionContext before adding and updating the ApplicationStateData and kept the cloned object in map against their ApplicationId as there are many test cases wherein more than 1 apps gets submitted and RM restart happens after that. This cloned object would get used while passing statestore during RM restarts. Modified MockRMMemoryStateStore (which already extends MemoryRMStateStore) to accomodate all the above described changes and used the same in all test cases, rather than doing changes in MemoryRMStateStore. Inaddition, renamed MockRMMemoryStateStore to MockMemoryRMStateStore to reflect the proper base class name. With this approach, changes in test cases had become simpler. Attached patch for the same.

          Show
          manirajv06@gmail.com Manikandan R added a comment - Rohith Sharma K S , Naganarasimha G R Thanks for review. Cloned ApplicationSubmissionContext before adding and updating the ApplicationStateData and kept the cloned object in map against their ApplicationId as there are many test cases wherein more than 1 apps gets submitted and RM restart happens after that. This cloned object would get used while passing statestore during RM restarts. Modified MockRMMemoryStateStore (which already extends MemoryRMStateStore) to accomodate all the above described changes and used the same in all test cases, rather than doing changes in MemoryRMStateStore. Inaddition, renamed MockRMMemoryStateStore to MockMemoryRMStateStore to reflect the proper base class name. With this approach, changes in test cases had become simpler. Attached patch for the same.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Agree with Rohith Sharma K S,
          We should not modify each individual test class and the approach will be prone to have errors when new test cases are added.
          So i would suggest to clone (either using clone method or having explicit code) appState.setApplicationSubmissionContext inside MemoryRMStateStore while adding or updating the ApplicationStateData.

          Show
          Naganarasimha Naganarasimha G R added a comment - Agree with Rohith Sharma K S , We should not modify each individual test class and the approach will be prone to have errors when new test cases are added. So i would suggest to clone (either using clone method or having explicit code) appState.setApplicationSubmissionContext inside MemoryRMStateStore while adding or updating the ApplicationStateData.
          Hide
          rohithsharma Rohith Sharma K S added a comment -

          Test cases using MemoryRMStateStore were not passing because of NPE during recovery process

          All most all test cases uses MemoryRMStateStore! Its better to clone while storing in MemoryRMStateStore#state rather than changing each and every test cases. In future, if any more restart tests get added that reduces this failure.

          Show
          rohithsharma Rohith Sharma K S added a comment - Test cases using MemoryRMStateStore were not passing because of NPE during recovery process All most all test cases uses MemoryRMStateStore! Its better to clone while storing in MemoryRMStateStore#state rather than changing each and every test cases. In future, if any more restart tests get added that reduces this failure.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 30s Docker mode activated.
                Prechecks
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 6 new or modified test files.
                trunk Compile Tests
          +1 mvninstall 20m 5s trunk passed
          +1 compile 0m 43s trunk passed
          +1 checkstyle 0m 33s trunk passed
          +1 mvnsite 0m 43s trunk passed
          +1 findbugs 1m 15s trunk passed
          +1 javadoc 0m 25s trunk passed
                Patch Compile Tests
          +1 mvninstall 0m 40s the patch passed
          +1 compile 0m 40s the patch passed
          +1 javac 0m 40s the patch passed
          +1 checkstyle 0m 28s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 0 new + 387 unchanged - 2 fixed = 387 total (was 389)
          +1 mvnsite 0m 41s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 17s the patch passed
          +1 javadoc 0m 21s the patch passed
                Other Tests
          -1 unit 48m 36s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 asflicense 0m 18s The patch does not generate ASF License warnings.
          78m 41s



          Reason Tests
          Failed junit tests hadoop.yarn.server.resourcemanager.rmapp.TestRMAppTransitions
            hadoop.yarn.server.resourcemanager.scheduler.capacity.TestContainerAllocation
          Timed out junit tests org.apache.hadoop.yarn.server.resourcemanager.TestSubmitApplicationWithRMHA



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:14b5c93
          JIRA Issue YARN-65
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12882391/YARN-65.010.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 5895f9fe6e47 3.13.0-116-generic #163-Ubuntu SMP Fri Mar 31 14:13:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / dd7916d
          Default Java 1.8.0_144
          findbugs v3.1.0-RC1
          unit https://builds.apache.org/job/PreCommit-YARN-Build/16965/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/16965/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/16965/console
          Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 30s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 6 new or modified test files.       trunk Compile Tests +1 mvninstall 20m 5s trunk passed +1 compile 0m 43s trunk passed +1 checkstyle 0m 33s trunk passed +1 mvnsite 0m 43s trunk passed +1 findbugs 1m 15s trunk passed +1 javadoc 0m 25s trunk passed       Patch Compile Tests +1 mvninstall 0m 40s the patch passed +1 compile 0m 40s the patch passed +1 javac 0m 40s the patch passed +1 checkstyle 0m 28s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 0 new + 387 unchanged - 2 fixed = 387 total (was 389) +1 mvnsite 0m 41s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 17s the patch passed +1 javadoc 0m 21s the patch passed       Other Tests -1 unit 48m 36s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 18s The patch does not generate ASF License warnings. 78m 41s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.rmapp.TestRMAppTransitions   hadoop.yarn.server.resourcemanager.scheduler.capacity.TestContainerAllocation Timed out junit tests org.apache.hadoop.yarn.server.resourcemanager.TestSubmitApplicationWithRMHA Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue YARN-65 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12882391/YARN-65.010.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 5895f9fe6e47 3.13.0-116-generic #163-Ubuntu SMP Fri Mar 31 14:13:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / dd7916d Default Java 1.8.0_144 findbugs v3.1.0-RC1 unit https://builds.apache.org/job/PreCommit-YARN-Build/16965/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/16965/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/16965/console Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          manirajv06@gmail.com Manikandan R added a comment -

          Fixed checkstyle issue. Also, earlier attached patch requires a change in test case post YARN-6982 changes. Hence, marking this JIRA depends on YARN-6982. Attached patch for review.

          Show
          manirajv06@gmail.com Manikandan R added a comment - Fixed checkstyle issue. Also, earlier attached patch requires a change in test case post YARN-6982 changes. Hence, marking this JIRA depends on YARN-6982 . Attached patch for review.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 24s Docker mode activated.
                Prechecks
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 6 new or modified test files.
                trunk Compile Tests
          +1 mvninstall 15m 27s trunk passed
          +1 compile 0m 36s trunk passed
          +1 checkstyle 0m 33s trunk passed
          +1 mvnsite 0m 40s trunk passed
          +1 findbugs 1m 5s trunk passed
          +1 javadoc 0m 22s trunk passed
                Patch Compile Tests
          +1 mvninstall 0m 33s the patch passed
          +1 compile 0m 32s the patch passed
          +1 javac 0m 32s the patch passed
          -0 checkstyle 0m 28s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 1 new + 388 unchanged - 2 fixed = 389 total (was 390)
          +1 mvnsite 0m 33s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 4s the patch passed
          +1 javadoc 0m 18s the patch passed
                Other Tests
          -1 unit 44m 41s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 asflicense 0m 18s The patch does not generate ASF License warnings.
          68m 51s



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



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:14b5c93
          JIRA Issue YARN-65
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12881977/YARN-65.009.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux ebb291734826 3.13.0-119-generic #166-Ubuntu SMP Wed May 3 12:18:55 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / dadb0c2
          Default Java 1.8.0_144
          findbugs v3.1.0-RC1
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16911/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/16911/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/16911/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/16911/console
          Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 24s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 6 new or modified test files.       trunk Compile Tests +1 mvninstall 15m 27s trunk passed +1 compile 0m 36s trunk passed +1 checkstyle 0m 33s trunk passed +1 mvnsite 0m 40s trunk passed +1 findbugs 1m 5s trunk passed +1 javadoc 0m 22s trunk passed       Patch Compile Tests +1 mvninstall 0m 33s the patch passed +1 compile 0m 32s the patch passed +1 javac 0m 32s the patch passed -0 checkstyle 0m 28s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 1 new + 388 unchanged - 2 fixed = 389 total (was 390) +1 mvnsite 0m 33s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 4s the patch passed +1 javadoc 0m 18s the patch passed       Other Tests -1 unit 44m 41s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 18s The patch does not generate ASF License warnings. 68m 51s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.capacity.TestContainerAllocation Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue YARN-65 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12881977/YARN-65.009.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux ebb291734826 3.13.0-119-generic #166-Ubuntu SMP Wed May 3 12:18:55 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / dadb0c2 Default Java 1.8.0_144 findbugs v3.1.0-RC1 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16911/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/16911/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/16911/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/16911/console Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          manirajv06@gmail.com Manikandan R added a comment -

          Fixed checkstyle issues. Test case failure is not related to this patch.

          Show
          manirajv06@gmail.com Manikandan R added a comment - Fixed checkstyle issues. Test case failure is not related to this patch.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 19s Docker mode activated.
                Prechecks
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 6 new or modified test files.
                trunk Compile Tests
          +1 mvninstall 13m 48s trunk passed
          +1 compile 0m 36s trunk passed
          +1 checkstyle 0m 30s trunk passed
          +1 mvnsite 0m 38s trunk passed
          +1 findbugs 1m 3s trunk passed
          +1 javadoc 0m 22s trunk passed
                Patch Compile Tests
          +1 mvninstall 0m 38s the patch passed
          +1 compile 0m 33s the patch passed
          +1 javac 0m 33s the patch passed
          -0 checkstyle 0m 28s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 26 new + 389 unchanged - 1 fixed = 415 total (was 390)
          +1 mvnsite 0m 35s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 9s the patch passed
          +1 javadoc 0m 20s the patch passed
                Other Tests
          -1 unit 45m 50s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 asflicense 0m 15s The patch does not generate ASF License warnings.
          68m 21s



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



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:14b5c93
          JIRA Issue YARN-65
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12881914/YARN-65.008.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux d9e5d6ec8b72 3.13.0-119-generic #166-Ubuntu SMP Wed May 3 12:18:55 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 2e43c28
          Default Java 1.8.0_144
          findbugs v3.1.0-RC1
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16904/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/16904/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/16904/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/16904/console
          Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 19s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 6 new or modified test files.       trunk Compile Tests +1 mvninstall 13m 48s trunk passed +1 compile 0m 36s trunk passed +1 checkstyle 0m 30s trunk passed +1 mvnsite 0m 38s trunk passed +1 findbugs 1m 3s trunk passed +1 javadoc 0m 22s trunk passed       Patch Compile Tests +1 mvninstall 0m 38s the patch passed +1 compile 0m 33s the patch passed +1 javac 0m 33s the patch passed -0 checkstyle 0m 28s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 26 new + 389 unchanged - 1 fixed = 415 total (was 390) +1 mvnsite 0m 35s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 9s the patch passed +1 javadoc 0m 20s the patch passed       Other Tests -1 unit 45m 50s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 68m 21s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.capacity.TestContainerAllocation Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue YARN-65 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12881914/YARN-65.008.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux d9e5d6ec8b72 3.13.0-119-generic #166-Ubuntu SMP Wed May 3 12:18:55 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 2e43c28 Default Java 1.8.0_144 findbugs v3.1.0-RC1 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16904/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/16904/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/16904/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/16904/console Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          manirajv06@gmail.com Manikandan R added a comment -

          Rohith Sharma K S Bibin A Chundatt Naganarasimha G R Thanks for taking a closer look and suggestions.

          Since ACLs are getting stored in ApplicationACLManager as part of RMAppManager#createAndPopulateNewRMApp, we are setting AMContainerSpec to null and attached patch for the same. Test cases using MemoryRMStateStore were not passing because of NPE during recovery process. Copy of the stack trace -

          java.lang.NullPointerException
          at org.apache.hadoop.yarn.server.resourcemanager.RMAppManager.createAndPopulateNewRMApp(RMAppManager.java:432)
          at org.apache.hadoop.yarn.server.resourcemanager.RMAppManager.recoverApplication(RMAppManager.java:347)
          at org.apache.hadoop.yarn.server.resourcemanager.RMAppManager.recover(RMAppManager.java:537)
          at org.apache.hadoop.yarn.server.resourcemanager.ResourceManager.recover(ResourceManager.java:1403)
          at org.apache.hadoop.yarn.server.resourcemanager.ResourceManager$RMActiveServices.serviceStart(ResourceManager.java:767)
          at org.apache.hadoop.service.AbstractService.start(AbstractService.java:194)
          at org.apache.hadoop.yarn.server.resourcemanager.ResourceManager.startActiveServices(ResourceManager.java:1156)
          at org.apache.hadoop.yarn.server.resourcemanager.ResourceManager$1.run(ResourceManager.java:1196)
          at org.apache.hadoop.yarn.server.resourcemanager.ResourceManager$1.run(ResourceManager.java:1)
          at java.security.AccessController.doPrivileged(Native Method)
          at javax.security.auth.Subject.doAs(Subject.java:422)

          To fix this NPE and pass these test cases, preserved AMContainerSpec from MemoryRMStateStore, after app submission into the running RM and restored the same into MemoryRMStateStore before starting RM again. Attached patch contains these test case changes as well.

          Show
          manirajv06@gmail.com Manikandan R added a comment - Rohith Sharma K S Bibin A Chundatt Naganarasimha G R Thanks for taking a closer look and suggestions. Since ACLs are getting stored in ApplicationACLManager as part of RMAppManager#createAndPopulateNewRMApp , we are setting AMContainerSpec to null and attached patch for the same. Test cases using MemoryRMStateStore were not passing because of NPE during recovery process. Copy of the stack trace - java.lang.NullPointerException at org.apache.hadoop.yarn.server.resourcemanager.RMAppManager.createAndPopulateNewRMApp(RMAppManager.java:432) at org.apache.hadoop.yarn.server.resourcemanager.RMAppManager.recoverApplication(RMAppManager.java:347) at org.apache.hadoop.yarn.server.resourcemanager.RMAppManager.recover(RMAppManager.java:537) at org.apache.hadoop.yarn.server.resourcemanager.ResourceManager.recover(ResourceManager.java:1403) at org.apache.hadoop.yarn.server.resourcemanager.ResourceManager$RMActiveServices.serviceStart(ResourceManager.java:767) at org.apache.hadoop.service.AbstractService.start(AbstractService.java:194) at org.apache.hadoop.yarn.server.resourcemanager.ResourceManager.startActiveServices(ResourceManager.java:1156) at org.apache.hadoop.yarn.server.resourcemanager.ResourceManager$1.run(ResourceManager.java:1196) at org.apache.hadoop.yarn.server.resourcemanager.ResourceManager$1.run(ResourceManager.java:1) at java.security.AccessController.doPrivileged(Native Method) at javax.security.auth.Subject.doAs(Subject.java:422) To fix this NPE and pass these test cases, preserved AMContainerSpec from MemoryRMStateStore , after app submission into the running RM and restored the same into MemoryRMStateStore before starting RM again. Attached patch contains these test case changes as well.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks Rohith Sharma K S & Bibin A Chundatt, for sharing your views.

          I think we can improve it setting AMContainerSpec to null rather than setting individual fields.

          yes agree its not required, currently only minimal information is required from submissionContext like getUnmanagedAM while creating the report, else we could have set submission context to null.

          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks Rohith Sharma K S & Bibin A Chundatt , for sharing your views. I think we can improve it setting AMContainerSpec to null rather than setting individual fields. yes agree its not required, currently only minimal information is required from submissionContext like getUnmanagedAM while creating the report, else we could have set submission context to null.
          Hide
          bibinchundatt Bibin A Chundatt added a comment - - edited

          Yes , i agree. Since we have already populated ApplicationAclManager further the context might not be required.

          Show
          bibinchundatt Bibin A Chundatt added a comment - - edited Yes , i agree. Since we have already populated ApplicationAclManager further the context might not be required.
          Hide
          rohithsharma Rohith Sharma K S added a comment -

          We will loose ApplicationACL in that case

          I do not agree we need any more AMContainerSpec once application is completed. And more over, it has been set at in-memory. At RMStateStore, still real submission context exist. On RM restart at any point of time, still recovers real submission context. This change is only in RM in-memory side. So, it should not impact if submission context clears after application is finished.

          Show
          rohithsharma Rohith Sharma K S added a comment - We will loose ApplicationACL in that case I do not agree we need any more AMContainerSpec once application is completed. And more over, it has been set at in-memory. At RMStateStore, still real submission context exist. On RM restart at any point of time, still recovers real submission context. This change is only in RM in-memory side. So, it should not impact if submission context clears after application is finished.
          Hide
          bibinchundatt Bibin A Chundatt added a comment -

          I think we can improve it setting AMContainerSpec to null rather than setting individual fields.

          We will loose ApplicationACL in that case. RMAppManager#createAndPopulateNewRMApp

             // Inform the ACLs Manager
              this.applicationACLsManager.addApplication(applicationId,
                  submissionContext.getAMContainerSpec().getApplicationACLs());
          

          Any idea why the applicationACL was set to AM container Launch Context??

          Show
          bibinchundatt Bibin A Chundatt added a comment - I think we can improve it setting AMContainerSpec to null rather than setting individual fields. We will loose ApplicationACL in that case. RMAppManager#createAndPopulateNewRMApp // Inform the ACLs Manager this .applicationACLsManager.addApplication(applicationId, submissionContext.getAMContainerSpec().getApplicationACLs()); Any idea why the applicationACL was set to AM container Launch Context??
          Hide
          rohithsharma Rohith Sharma K S added a comment -

          While offline discussion with Naganarasimha G R, he pointed out clearing fields are done after application is finished. Thats reasonable.
          I think we can improve it setting AMContainerSpec to null rather than setting individual fields.

          Show
          rohithsharma Rohith Sharma K S added a comment - While offline discussion with Naganarasimha G R , he pointed out clearing fields are done after application is finished. Thats reasonable. I think we can improve it setting AMContainerSpec to null rather than setting individual fields.
          Hide
          rohithsharma Rohith Sharma K S added a comment -

          thanks Manikandan R for the patch.
          Clearing AMContainer launch context should not be done unless application is completed. It is always buggy!! And also I see existing code also a buggy which clears app.submissionContext.getAMContainerSpec().setTokensConf(null);. After application is submitted, at any point of time application can be updated. During application update, we also stores application submission context back to RMStateStore which overrides. And if RM restart after app update then it is a issue.

          As a solution, unused fields must be cleared only after app is finished may be in final transition.

          Show
          rohithsharma Rohith Sharma K S added a comment - thanks Manikandan R for the patch. Clearing AMContainer launch context should not be done unless application is completed. It is always buggy!! And also I see existing code also a buggy which clears app.submissionContext.getAMContainerSpec().setTokensConf(null); . After application is submitted, at any point of time application can be updated. During application update, we also stores application submission context back to RMStateStore which overrides. And if RM restart after app update then it is a issue. As a solution, unused fields must be cleared only after app is finished may be in final transition.
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 16s Docker mode activated.
                Prechecks
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
                trunk Compile Tests
          +1 mvninstall 14m 2s trunk passed
          +1 compile 0m 40s trunk passed
          +1 checkstyle 0m 30s trunk passed
          +1 mvnsite 0m 41s trunk passed
          +1 findbugs 1m 10s trunk passed
          +1 javadoc 0m 22s trunk passed
                Patch Compile Tests
          +1 mvninstall 0m 34s the patch passed
          +1 compile 0m 41s the patch passed
          +1 javac 0m 41s the patch passed
          +1 checkstyle 0m 25s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 0 new + 137 unchanged - 1 fixed = 137 total (was 138)
          +1 mvnsite 0m 41s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 16s the patch passed
          +1 javadoc 0m 19s the patch passed
                Other Tests
          +1 unit 43m 48s hadoop-yarn-server-resourcemanager in the patch passed.
          +1 asflicense 0m 16s The patch does not generate ASF License warnings.
          67m 6s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:14b5c93
          JIRA Issue YARN-65
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12880856/YARN-65.007.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 73e43261786f 3.13.0-116-generic #163-Ubuntu SMP Fri Mar 31 14:13:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 9891295
          Default Java 1.8.0_131
          findbugs v3.1.0-RC1
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/16774/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/16774/console
          Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 16s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.       trunk Compile Tests +1 mvninstall 14m 2s trunk passed +1 compile 0m 40s trunk passed +1 checkstyle 0m 30s trunk passed +1 mvnsite 0m 41s trunk passed +1 findbugs 1m 10s trunk passed +1 javadoc 0m 22s trunk passed       Patch Compile Tests +1 mvninstall 0m 34s the patch passed +1 compile 0m 41s the patch passed +1 javac 0m 41s the patch passed +1 checkstyle 0m 25s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 0 new + 137 unchanged - 1 fixed = 137 total (was 138) +1 mvnsite 0m 41s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 16s the patch passed +1 javadoc 0m 19s the patch passed       Other Tests +1 unit 43m 48s hadoop-yarn-server-resourcemanager in the patch passed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 67m 6s Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue YARN-65 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12880856/YARN-65.007.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 73e43261786f 3.13.0-116-generic #163-Ubuntu SMP Fri Mar 31 14:13:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 9891295 Default Java 1.8.0_131 findbugs v3.1.0-RC1 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/16774/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/16774/console Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          manirajv06@gmail.com Manikandan R added a comment -

          Fixed whitespace issues.

          Show
          manirajv06@gmail.com Manikandan R added a comment - Fixed whitespace issues.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 13s Docker mode activated.
                Prechecks
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
                trunk Compile Tests
          +1 mvninstall 13m 45s trunk passed
          +1 compile 0m 34s trunk passed
          +1 checkstyle 0m 27s trunk passed
          +1 mvnsite 0m 35s trunk passed
          +1 findbugs 0m 58s trunk passed
          +1 javadoc 0m 21s trunk passed
                Patch Compile Tests
          +1 mvninstall 0m 33s the patch passed
          +1 compile 0m 31s the patch passed
          +1 javac 0m 31s the patch passed
          +1 checkstyle 0m 24s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 0 new + 137 unchanged - 1 fixed = 137 total (was 138)
          +1 mvnsite 0m 33s 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 1m 4s the patch passed
          +1 javadoc 0m 18s the patch passed
                Other Tests
          -1 unit 42m 29s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 asflicense 0m 13s The patch does not generate ASF License warnings.
          64m 24s



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



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:14b5c93
          JIRA Issue YARN-65
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12880824/YARN-65.006.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 761edf4083b2 3.13.0-119-generic #166-Ubuntu SMP Wed May 3 12:18:55 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 9891295
          Default Java 1.8.0_131
          findbugs v3.1.0-RC1
          whitespace https://builds.apache.org/job/PreCommit-YARN-Build/16771/artifact/patchprocess/whitespace-eol.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/16771/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/16771/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/16771/console
          Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 13s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.       trunk Compile Tests +1 mvninstall 13m 45s trunk passed +1 compile 0m 34s trunk passed +1 checkstyle 0m 27s trunk passed +1 mvnsite 0m 35s trunk passed +1 findbugs 0m 58s trunk passed +1 javadoc 0m 21s trunk passed       Patch Compile Tests +1 mvninstall 0m 33s the patch passed +1 compile 0m 31s the patch passed +1 javac 0m 31s the patch passed +1 checkstyle 0m 24s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 0 new + 137 unchanged - 1 fixed = 137 total (was 138) +1 mvnsite 0m 33s 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 1m 4s the patch passed +1 javadoc 0m 18s the patch passed       Other Tests -1 unit 42m 29s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 13s The patch does not generate ASF License warnings. 64m 24s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.fair.TestFSAppStarvation Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue YARN-65 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12880824/YARN-65.006.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 761edf4083b2 3.13.0-119-generic #166-Ubuntu SMP Wed May 3 12:18:55 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 9891295 Default Java 1.8.0_131 findbugs v3.1.0-RC1 whitespace https://builds.apache.org/job/PreCommit-YARN-Build/16771/artifact/patchprocess/whitespace-eol.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/16771/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/16771/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/16771/console Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          manirajv06@gmail.com Manikandan R added a comment -

          Fixed whitespace & checkstyle issues. Junit failures are not related to this patch. Attached patch for review.

          Show
          manirajv06@gmail.com Manikandan R added a comment - Fixed whitespace & checkstyle issues. Junit failures are not related to this patch. Attached patch for review.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 22s Docker mode activated.
                Prechecks
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
                trunk Compile Tests
          +1 mvninstall 15m 4s trunk passed
          +1 compile 0m 37s trunk passed
          +1 checkstyle 0m 29s trunk passed
          +1 mvnsite 0m 39s trunk passed
          +1 findbugs 1m 12s trunk passed
          +1 javadoc 0m 23s trunk passed
                Patch Compile Tests
          +1 mvninstall 0m 37s the patch passed
          +1 compile 0m 34s the patch passed
          +1 javac 0m 34s the patch passed
          -0 checkstyle 0m 25s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 3 new + 137 unchanged - 1 fixed = 140 total (was 138)
          +1 mvnsite 0m 37s the patch passed
          -1 whitespace 0m 0s The patch has 2 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
          +1 findbugs 1m 12s the patch passed
          +1 javadoc 0m 24s the patch passed
                Other Tests
          -1 unit 46m 37s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 asflicense 0m 16s The patch does not generate ASF License warnings.
          70m 50s



          Reason Tests
          Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.fair.TestFSAppStarvation
          Timed out junit tests org.apache.hadoop.yarn.server.resourcemanager.TestSubmitApplicationWithRMHA



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:14b5c93
          JIRA Issue YARN-65
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12880680/YARN-65.005.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 5304b510a07b 3.13.0-116-generic #163-Ubuntu SMP Fri Mar 31 14:13:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / adb84f3
          Default Java 1.8.0_131
          findbugs v3.1.0-RC1
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16749/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          whitespace https://builds.apache.org/job/PreCommit-YARN-Build/16749/artifact/patchprocess/whitespace-eol.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/16749/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/16749/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/16749/console
          Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 22s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.       trunk Compile Tests +1 mvninstall 15m 4s trunk passed +1 compile 0m 37s trunk passed +1 checkstyle 0m 29s trunk passed +1 mvnsite 0m 39s trunk passed +1 findbugs 1m 12s trunk passed +1 javadoc 0m 23s trunk passed       Patch Compile Tests +1 mvninstall 0m 37s the patch passed +1 compile 0m 34s the patch passed +1 javac 0m 34s the patch passed -0 checkstyle 0m 25s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 3 new + 137 unchanged - 1 fixed = 140 total (was 138) +1 mvnsite 0m 37s the patch passed -1 whitespace 0m 0s The patch has 2 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply +1 findbugs 1m 12s the patch passed +1 javadoc 0m 24s the patch passed       Other Tests -1 unit 46m 37s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 70m 50s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.fair.TestFSAppStarvation Timed out junit tests org.apache.hadoop.yarn.server.resourcemanager.TestSubmitApplicationWithRMHA Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue YARN-65 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12880680/YARN-65.005.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 5304b510a07b 3.13.0-116-generic #163-Ubuntu SMP Fri Mar 31 14:13:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / adb84f3 Default Java 1.8.0_131 findbugs v3.1.0-RC1 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16749/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt whitespace https://builds.apache.org/job/PreCommit-YARN-Build/16749/artifact/patchprocess/whitespace-eol.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/16749/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/16749/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/16749/console Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          manirajv06@gmail.com Manikandan R added a comment -

          Naganarasimha G R Thanks for your review.

          Addressed all points and fixed checkstyle issues as well. Attached patch for review.

          Show
          manirajv06@gmail.com Manikandan R added a comment - Naganarasimha G R Thanks for your review. Addressed all points and fixed checkstyle issues as well. Attached patch for review.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Hi Manikandan R,
          thanks for providing the patch, please find my comments for the same

          1. RMAppImpl ln no 1095, not required
          2. RMAppImpl ln no 2022, why you need reference to RMAppImpl as method argument ? its not a static method IMO let the method be just a method in RMAppImpl. And let the method name be more meaningful (clearUnrequiredFields or something)
          3. TestRMAppTransitions ln no 21, Import the required classes
          4. TestRMAppTransitions ln no 131 & 132, Why localFS & tmpDir required to be fields? It can just be local variables right ?
          5. TestRMAppTransitions ln no 300, 351, please format the code based on the hadoop template for the modified code, and many of the check style issues are related to it. And as well fix other checkstyle issues
          6. TestRMAppTransitions ln no 1235, method name can be more meaningfull, verifyUnusedFields -> verifyRMAppFieldsForNonFinalTransitions and verifyUnusedFieldsForNullValues --> verifyRMAppFieldsForFinalTransitions
          Show
          Naganarasimha Naganarasimha G R added a comment - Hi Manikandan R , thanks for providing the patch, please find my comments for the same RMAppImpl ln no 1095, not required RMAppImpl ln no 2022, why you need reference to RMAppImpl as method argument ? its not a static method IMO let the method be just a method in RMAppImpl. And let the method name be more meaningful (clearUnrequiredFields or something) TestRMAppTransitions ln no 21, Import the required classes TestRMAppTransitions ln no 131 & 132, Why localFS & tmpDir required to be fields? It can just be local variables right ? TestRMAppTransitions ln no 300, 351, please format the code based on the hadoop template for the modified code, and many of the check style issues are related to it. And as well fix other checkstyle issues TestRMAppTransitions ln no 1235, method name can be more meaningfull, verifyUnusedFields -> verifyRMAppFieldsForNonFinalTransitions and verifyUnusedFieldsForNullValues --> verifyRMAppFieldsForFinalTransitions
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 18s Docker mode activated.
                Prechecks
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
                trunk Compile Tests
          +1 mvninstall 17m 45s trunk passed
          +1 compile 0m 44s trunk passed
          +1 checkstyle 0m 32s trunk passed
          +1 mvnsite 0m 49s trunk passed
          +1 findbugs 1m 20s trunk passed
          +1 javadoc 0m 29s trunk passed
                Patch Compile Tests
          +1 mvninstall 0m 45s the patch passed
          +1 compile 0m 43s the patch passed
          +1 javac 0m 43s the patch passed
          -0 checkstyle 0m 31s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 13 new + 137 unchanged - 1 fixed = 150 total (was 138)
          +1 mvnsite 0m 45s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 29s the patch passed
          +1 javadoc 0m 25s the patch passed
                Other Tests
          +1 unit 47m 7s hadoop-yarn-server-resourcemanager in the patch passed.
          +1 asflicense 0m 18s The patch does not generate ASF License warnings.
          75m 29s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:14b5c93
          JIRA Issue YARN-65
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12880287/YARN-65.004.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux c2b2bfa9805e 3.13.0-116-generic #163-Ubuntu SMP Fri Mar 31 14:13:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / f44b349
          Default Java 1.8.0_131
          findbugs v3.1.0-RC1
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16718/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/16718/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/16718/console
          Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 18s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.       trunk Compile Tests +1 mvninstall 17m 45s trunk passed +1 compile 0m 44s trunk passed +1 checkstyle 0m 32s trunk passed +1 mvnsite 0m 49s trunk passed +1 findbugs 1m 20s trunk passed +1 javadoc 0m 29s trunk passed       Patch Compile Tests +1 mvninstall 0m 45s the patch passed +1 compile 0m 43s the patch passed +1 javac 0m 43s the patch passed -0 checkstyle 0m 31s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 13 new + 137 unchanged - 1 fixed = 150 total (was 138) +1 mvnsite 0m 45s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 29s the patch passed +1 javadoc 0m 25s the patch passed       Other Tests +1 unit 47m 7s hadoop-yarn-server-resourcemanager in the patch passed. +1 asflicense 0m 18s The patch does not generate ASF License warnings. 75m 29s Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue YARN-65 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12880287/YARN-65.004.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux c2b2bfa9805e 3.13.0-116-generic #163-Ubuntu SMP Fri Mar 31 14:13:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / f44b349 Default Java 1.8.0_131 findbugs v3.1.0-RC1 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16718/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/16718/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/16718/console Powered by Apache Yetus 0.6.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 4m 35s Docker mode activated.
                Prechecks
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
                trunk Compile Tests
          +1 mvninstall 14m 42s trunk passed
          +1 compile 0m 44s trunk passed
          +1 checkstyle 0m 27s trunk passed
          +1 mvnsite 0m 42s trunk passed
          +1 findbugs 1m 10s trunk passed
          +1 javadoc 0m 21s trunk passed
                Patch Compile Tests
          +1 mvninstall 0m 42s the patch passed
          +1 compile 0m 35s the patch passed
          +1 javac 0m 35s the patch passed
          -0 checkstyle 0m 27s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 13 new + 137 unchanged - 1 fixed = 150 total (was 138)
          +1 mvnsite 0m 40s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 10s the patch passed
          +1 javadoc 0m 23s the patch passed
                Other Tests
          +1 unit 43m 30s hadoop-yarn-server-resourcemanager in the patch passed.
          +1 asflicense 0m 15s The patch does not generate ASF License warnings.
          71m 43s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:14b5c93
          JIRA Issue YARN-65
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12880287/YARN-65.004.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux f4b5a072a392 3.13.0-116-generic #163-Ubuntu SMP Fri Mar 31 14:13:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 02bf328
          Default Java 1.8.0_131
          findbugs v3.1.0-RC1
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16711/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/16711/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/16711/console
          Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 4m 35s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.       trunk Compile Tests +1 mvninstall 14m 42s trunk passed +1 compile 0m 44s trunk passed +1 checkstyle 0m 27s trunk passed +1 mvnsite 0m 42s trunk passed +1 findbugs 1m 10s trunk passed +1 javadoc 0m 21s trunk passed       Patch Compile Tests +1 mvninstall 0m 42s the patch passed +1 compile 0m 35s the patch passed +1 javac 0m 35s the patch passed -0 checkstyle 0m 27s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 13 new + 137 unchanged - 1 fixed = 150 total (was 138) +1 mvnsite 0m 40s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 10s the patch passed +1 javadoc 0m 23s the patch passed       Other Tests +1 unit 43m 30s hadoop-yarn-server-resourcemanager in the patch passed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 71m 43s Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue YARN-65 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12880287/YARN-65.004.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux f4b5a072a392 3.13.0-116-generic #163-Ubuntu SMP Fri Mar 31 14:13:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 02bf328 Default Java 1.8.0_131 findbugs v3.1.0-RC1 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16711/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/16711/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/16711/console Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          manirajv06@gmail.com Manikandan R added a comment -

          Naganarasimha Garla Thanks for your review.

          Addressed all comments and attached a new patch for the same.

          Show
          manirajv06@gmail.com Manikandan R added a comment - Naganarasimha Garla Thanks for your review. Addressed all comments and attached a new patch for the same.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks Manikandan R, for the latest patch.
          Few nits :

          1. environment,Commands, LocalResources,ServiceData : for all these fields it would be better to get the reference and clear rather than creating a new object.
          2. Encapsulating these modifications into a method is better for readability and reuse in future
          3. Please add some test cases for the same.
          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks Manikandan R , for the latest patch. Few nits : environment,Commands, LocalResources,ServiceData : for all these fields it would be better to get the reference and clear rather than creating a new object. Encapsulating these modifications into a method is better for readability and reuse in future Please add some test cases for the same.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 33s Docker mode activated.
                Prechecks
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
                trunk Compile Tests
          +1 mvninstall 15m 48s trunk passed
          +1 compile 0m 34s trunk passed
          +1 checkstyle 0m 27s trunk passed
          +1 mvnsite 0m 45s trunk passed
          +1 findbugs 1m 12s trunk passed
          +1 javadoc 0m 23s trunk passed
                Patch Compile Tests
          +1 mvninstall 0m 35s the patch passed
          +1 compile 0m 40s the patch passed
          +1 javac 0m 40s the patch passed
          -0 checkstyle 0m 27s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 6 new + 116 unchanged - 0 fixed = 122 total (was 116)
          +1 mvnsite 0m 39s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 20s the patch passed
          +1 javadoc 0m 25s the patch passed
                Other Tests
          -1 unit 47m 14s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 asflicense 0m 20s The patch does not generate ASF License warnings.
          72m 40s



          Reason Tests
          Failed junit tests hadoop.yarn.server.resourcemanager.security.TestDelegationTokenRenewer
            hadoop.yarn.server.resourcemanager.scheduler.fair.TestFSAppStarvation
          Timed out junit tests org.apache.hadoop.yarn.server.resourcemanager.TestSubmitApplicationWithRMHA



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:14b5c93
          JIRA Issue YARN-65
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12879664/YARN-65.003.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 18e03591becb 3.13.0-116-generic #163-Ubuntu SMP Fri Mar 31 14:13:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 0fd6d0f
          Default Java 1.8.0_131
          findbugs v3.1.0-RC1
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16621/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/16621/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/16621/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/16621/console
          Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 33s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.       trunk Compile Tests +1 mvninstall 15m 48s trunk passed +1 compile 0m 34s trunk passed +1 checkstyle 0m 27s trunk passed +1 mvnsite 0m 45s trunk passed +1 findbugs 1m 12s trunk passed +1 javadoc 0m 23s trunk passed       Patch Compile Tests +1 mvninstall 0m 35s the patch passed +1 compile 0m 40s the patch passed +1 javac 0m 40s the patch passed -0 checkstyle 0m 27s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 6 new + 116 unchanged - 0 fixed = 122 total (was 116) +1 mvnsite 0m 39s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 20s the patch passed +1 javadoc 0m 25s the patch passed       Other Tests -1 unit 47m 14s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 20s The patch does not generate ASF License warnings. 72m 40s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.security.TestDelegationTokenRenewer   hadoop.yarn.server.resourcemanager.scheduler.fair.TestFSAppStarvation Timed out junit tests org.apache.hadoop.yarn.server.resourcemanager.TestSubmitApplicationWithRMHA Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue YARN-65 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12879664/YARN-65.003.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 18e03591becb 3.13.0-116-generic #163-Ubuntu SMP Fri Mar 31 14:13:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 0fd6d0f Default Java 1.8.0_131 findbugs v3.1.0-RC1 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16621/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/16621/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/16621/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/16621/console Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          manirajv06@gmail.com Manikandan R added a comment -

          Fixed checkstyle and whitespace errors. Junit failure is not related to this patch as it has been captured separately in https://issues.apache.org/jira/browse/YARN-5816

          Show
          manirajv06@gmail.com Manikandan R added a comment - Fixed checkstyle and whitespace errors. Junit failure is not related to this patch as it has been captured separately in https://issues.apache.org/jira/browse/YARN-5816
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 17s Docker mode activated.
                Prechecks
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
                trunk Compile Tests
          +1 mvninstall 15m 18s trunk passed
          +1 compile 0m 36s trunk passed
          +1 checkstyle 0m 28s trunk passed
          +1 mvnsite 0m 36s trunk passed
          +1 findbugs 0m 59s trunk passed
          +1 javadoc 0m 20s trunk passed
                Patch Compile Tests
          +1 mvninstall 0m 33s the patch passed
          +1 compile 0m 33s the patch passed
          +1 javac 0m 33s the patch passed
          -0 checkstyle 0m 24s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 11 new + 116 unchanged - 0 fixed = 127 total (was 116)
          +1 mvnsite 0m 37s 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 1m 10s the patch passed
          +1 javadoc 0m 18s the patch passed
                Other Tests
          -1 unit 42m 45s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 asflicense 0m 14s The patch does not generate ASF License warnings.
          66m 22s



          Reason Tests
          Failed junit tests hadoop.yarn.server.resourcemanager.security.TestDelegationTokenRenewer



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:14b5c93
          JIRA Issue YARN-65
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12879561/YARN-65.002.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux fe507f58b511 3.13.0-116-generic #163-Ubuntu SMP Fri Mar 31 14:13:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 481385e
          Default Java 1.8.0_131
          findbugs v3.1.0-RC1
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16613/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          whitespace https://builds.apache.org/job/PreCommit-YARN-Build/16613/artifact/patchprocess/whitespace-eol.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/16613/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/16613/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/16613/console
          Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 17s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.       trunk Compile Tests +1 mvninstall 15m 18s trunk passed +1 compile 0m 36s trunk passed +1 checkstyle 0m 28s trunk passed +1 mvnsite 0m 36s trunk passed +1 findbugs 0m 59s trunk passed +1 javadoc 0m 20s trunk passed       Patch Compile Tests +1 mvninstall 0m 33s the patch passed +1 compile 0m 33s the patch passed +1 javac 0m 33s the patch passed -0 checkstyle 0m 24s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 11 new + 116 unchanged - 0 fixed = 127 total (was 116) +1 mvnsite 0m 37s 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 1m 10s the patch passed +1 javadoc 0m 18s the patch passed       Other Tests -1 unit 42m 45s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 14s The patch does not generate ASF License warnings. 66m 22s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.security.TestDelegationTokenRenewer Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue YARN-65 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12879561/YARN-65.002.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux fe507f58b511 3.13.0-116-generic #163-Ubuntu SMP Fri Mar 31 14:13:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 481385e Default Java 1.8.0_131 findbugs v3.1.0-RC1 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16613/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt whitespace https://builds.apache.org/job/PreCommit-YARN-Build/16613/artifact/patchprocess/whitespace-eol.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/16613/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/16613/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/16613/console Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          manirajv06@gmail.com Manikandan R added a comment -

          Thanks Naganarasimha Garla for review and suggestions.

          1, 2: Yes, this explains the test cases failures mentioned in my previous comment. At that time, I thought it was not because of AddApplicationToSchedulerTransition class. Now, got an understanding about this flow.

          3: Taken care.

          Attached patch for review.

          Show
          manirajv06@gmail.com Manikandan R added a comment - Thanks Naganarasimha Garla for review and suggestions. 1, 2: Yes, this explains the test cases failures mentioned in my previous comment. At that time, I thought it was not because of AddApplicationToSchedulerTransition class. Now, got an understanding about this flow. 3: Taken care. Attached patch for review.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks for working on it Manikandan R based on the discussion we had offline. Have assigned the issue to you.
          I see few main issues in the patch.

          1. RMAppImpl, ln no 1090, RMAppRecoveredTransition you are setting many of the parameters to null! what if the app is running ?
          2. RMAppImpl, ln no 1144, AddApplicationToSchedulerTransition, on add application all are getting set to null !
          3. RMAppImpl, ln no 2029, setApplicationACLs should not be set to null, as it will be required to validate while showing apps
          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks for working on it Manikandan R based on the discussion we had offline. Have assigned the issue to you. I see few main issues in the patch. RMAppImpl, ln no 1090, RMAppRecoveredTransition you are setting many of the parameters to null! what if the app is running ? RMAppImpl, ln no 1144, AddApplicationToSchedulerTransition, on add application all are getting set to null ! RMAppImpl, ln no 2029, setApplicationACLs should not be set to null, as it will be required to validate while showing apps
          Hide
          manirajv06@gmail.com Manikandan R added a comment -

          Based on earlier suggestions, made changes to free memory by clearing unused variables explicitly with either null or empty values, particularly submissioncontext object. As of now, we are clearing up

          app.submissionContext.getAMContainerSpec().setTokensConf(null);

          inside AddApplicationToSchedulerTransition, RMAppRecoveredTransition & FinalTransition class in RMAppImpl.java. Created a new method to clear all unused fields including above mentioned specific field so that it can be called from above mentioned classes.

          Attached initial patch for review.

          Couple of junits test cases like

          TestContainerAllocation.testLogAggregationContextPassedIntoContainerToken
          TestRMWebServicesAppsModification.testGetNewApplicationAndSubmit

          is failing because actual value would have been cleared and hence not matching with the expected value. To confirm whether those fields has been used in application report, checked the AppBlock.java and don't see any usage over there? Please correct me if this understanding is wrong. Based on feedback, we may need to fix the Junit test cases accordingly.

          In addition, to understand the memory savings, submitted an simple yarn app to RM (local setup) in series and tried to visualize memory usage using jconsole during GC before and after this code changes. I am not seeing any significant difference as may be because of app is very light and size of each field inside submissioncontext etc.

          Show
          manirajv06@gmail.com Manikandan R added a comment - Based on earlier suggestions, made changes to free memory by clearing unused variables explicitly with either null or empty values, particularly submissioncontext object. As of now, we are clearing up app.submissionContext.getAMContainerSpec().setTokensConf(null); inside AddApplicationToSchedulerTransition, RMAppRecoveredTransition & FinalTransition class in RMAppImpl.java. Created a new method to clear all unused fields including above mentioned specific field so that it can be called from above mentioned classes. Attached initial patch for review. Couple of junits test cases like TestContainerAllocation.testLogAggregationContextPassedIntoContainerToken TestRMWebServicesAppsModification.testGetNewApplicationAndSubmit is failing because actual value would have been cleared and hence not matching with the expected value. To confirm whether those fields has been used in application report, checked the AppBlock.java and don't see any usage over there? Please correct me if this understanding is wrong. Based on feedback, we may need to fix the Junit test cases accordingly. In addition, to understand the memory savings, submitted an simple yarn app to RM (local setup) in series and tried to visualize memory usage using jconsole during GC before and after this code changes. I am not seeing any significant difference as may be because of app is very light and size of each field inside submissioncontext etc.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks for the clarification Jason Lowe, may be i got confused because of your earlier comment mentioning

          when an application completes, replace the RMAppImpl object with something like a RMCompletedAppImpl that in turn has references to RMCompletedAppAttemptImpl rather than RMAppAttemptImpl. 
          

          But anyway as we are in same page we can take this forward now.

          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks for the clarification Jason Lowe , may be i got confused because of your earlier comment mentioning when an application completes, replace the RMAppImpl object with something like a RMCompletedAppImpl that in turn has references to RMCompletedAppAttemptImpl rather than RMAppAttemptImpl. But anyway as we are in same page we can take this forward now.
          Hide
          jlowe Jason Lowe added a comment -

          Nulling fields that reference objects that are no longer needed once the app completes is what I was thinking when I filed this and is what I meant by "can be freed once the application has finished."

          Show
          jlowe Jason Lowe added a comment - Nulling fields that reference objects that are no longer needed once the app completes is what I was thinking when I filed this and is what I meant by "can be freed once the application has finished."
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Jason Lowe, wow never expected to hit a issue which is so old.
          But was just thinking about a simpler solution where to set the references of the unused fields to null in the RMAppImpl instance. As you mentioned (long before) we were seeing many of the fields used in ApplicationSubmissionContext/ AM's ContainerLaunchContext are not required for reporting purpose hence planning to set them to null on the completion of the app and also at the same time we need to handle the same during recovery too. Thoughts ?

          Show
          Naganarasimha Naganarasimha G R added a comment - Jason Lowe , wow never expected to hit a issue which is so old. But was just thinking about a simpler solution where to set the references of the unused fields to null in the RMAppImpl instance. As you mentioned (long before) we were seeing many of the fields used in ApplicationSubmissionContext/ AM's ContainerLaunchContext are not required for reporting purpose hence planning to set them to null on the completion of the app and also at the same time we need to handle the same during recovery too. Thoughts ?
          Hide
          bolshakov.denis@gmail.com Denis Bolshakov added a comment -

          Thanks for such detailed comment. I will investigate more deeply.

          Kind regards,
          Denis

          31 Окт 2016 г. 21:00 пользователь "Jason Lowe (JIRA)" <jira@apache.org>

          Show
          bolshakov.denis@gmail.com Denis Bolshakov added a comment - Thanks for such detailed comment. I will investigate more deeply. Kind regards, Denis 31 Окт 2016 г. 21:00 пользователь "Jason Lowe (JIRA)" <jira@apache.org>
          Hide
          jlowe Jason Lowe added a comment -

          I believe the request is still valid. This isn't so much about adjusting the configurable number as it is reducing the memory waste per application. Even with fewer applications retained there are opportunities to reduce the resourcemanager's memory requirements.

          RMAppManager may only be tracking ApplicationId objects directly in completedApps, but the RMContext is tracking the full RMAppImpl objects that are looked up by those IDs (see RMContext#getRMApps and RMAppManager#checkAppNumCompletedLimit). It doesn't make sense for the RM to track only the ApplicationID for completed applications since that alone is not enough to build an application report should someone come along and query it. So the RM needs to track enough information to fill out a report for completed applications, but the submissionContext is a significant chunk of memory (it can be many megabytes in some cases) that can be freed once the application has finished. There are a couple of items from the submission context that are referenced when filling out an app report (e.g.: priority, whether the AM was unmanaged), but those can be copied out, allowing the submission context to be released once the app finishes.

          At one extreme we could take a similar approach to the MR job history server, using a cheaper representation for tracking completed applications, i.e.: when an application completes, replace the RMAppImpl object with something like a RMCompletedAppImpl that in turn has references to RMCompletedAppAttemptImpl rather than RMAppAttemptImpl. Then we can dump the unnecessary concurrent hash maps necessary when the job is active and all the one-off tracking fields that only make sense when the app is alive. Of course there are diminishing returns there and an increased maintenance burden, so it may not be worth it. However releasing the submissionContext probably gets us most of the memory savings with what should be a relatively straightforward change.

          Show
          jlowe Jason Lowe added a comment - I believe the request is still valid. This isn't so much about adjusting the configurable number as it is reducing the memory waste per application. Even with fewer applications retained there are opportunities to reduce the resourcemanager's memory requirements. RMAppManager may only be tracking ApplicationId objects directly in completedApps, but the RMContext is tracking the full RMAppImpl objects that are looked up by those IDs (see RMContext#getRMApps and RMAppManager#checkAppNumCompletedLimit). It doesn't make sense for the RM to track only the ApplicationID for completed applications since that alone is not enough to build an application report should someone come along and query it. So the RM needs to track enough information to fill out a report for completed applications, but the submissionContext is a significant chunk of memory (it can be many megabytes in some cases) that can be freed once the application has finished. There are a couple of items from the submission context that are referenced when filling out an app report (e.g.: priority, whether the AM was unmanaged), but those can be copied out, allowing the submission context to be released once the app finishes. At one extreme we could take a similar approach to the MR job history server, using a cheaper representation for tracking completed applications, i.e.: when an application completes, replace the RMAppImpl object with something like a RMCompletedAppImpl that in turn has references to RMCompletedAppAttemptImpl rather than RMAppAttemptImpl. Then we can dump the unnecessary concurrent hash maps necessary when the job is active and all the one-off tracking fields that only make sense when the app is alive. Of course there are diminishing returns there and an increased maintenance burden, so it may not be worth it. However releasing the submissionContext probably gets us most of the memory savings with what should be a relatively straightforward change.
          Hide
          bolshakov.denis@gmail.com Denis Bolshakov added a comment -

          Vinod Kumar Vavilapalli, I suggest to close this ticket:
          1. yarn.resource.max-completed-applications has been renamed, and current default is 1000
          2. RMAppManager keeps a list of ApplicationId (completedApps), not list of RMAppImpl, so it does not have such overhead anymore.

          Show
          bolshakov.denis@gmail.com Denis Bolshakov added a comment - Vinod Kumar Vavilapalli , I suggest to close this ticket: 1. yarn.resource.max-completed-applications has been renamed, and current default is 1000 2. RMAppManager keeps a list of ApplicationId (completedApps), not list of RMAppImpl, so it does not have such overhead anymore.

            People

            • Assignee:
              manirajv06@gmail.com Manikandan R
              Reporter:
              jlowe Jason Lowe
            • Votes:
              1 Vote for this issue
              Watchers:
              19 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development