Details

    • Reviewed

    Attachments

      1. YARN-6424.01.patch
        2 kB
        Varun Saxena

      Issue Links

        Activity

          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 15s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 test4tests 0m 0s The patch 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.
          +1 mvninstall 14m 52s trunk passed
          +1 compile 0m 31s trunk passed
          +1 checkstyle 0m 26s trunk passed
          +1 mvnsite 0m 33s trunk passed
          +1 mvneclipse 0m 14s trunk passed
          +1 findbugs 0m 59s trunk passed
          +1 javadoc 0m 21s trunk passed
          +1 mvninstall 0m 30s the patch passed
          +1 compile 0m 29s the patch passed
          +1 javac 0m 29s the patch passed
          +1 checkstyle 0m 23s the patch passed
          +1 mvnsite 0m 31s the patch passed
          +1 mvneclipse 0m 11s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 2s the patch passed
          +1 javadoc 0m 18s the patch passed
          -1 unit 38m 42s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 asflicense 0m 17s The patch does not generate ASF License warnings.
          61m 56s



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



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

          This message was automatically generated.

          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 15s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s The patch 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. +1 mvninstall 14m 52s trunk passed +1 compile 0m 31s trunk passed +1 checkstyle 0m 26s trunk passed +1 mvnsite 0m 33s trunk passed +1 mvneclipse 0m 14s trunk passed +1 findbugs 0m 59s trunk passed +1 javadoc 0m 21s trunk passed +1 mvninstall 0m 30s the patch passed +1 compile 0m 29s the patch passed +1 javac 0m 29s the patch passed +1 checkstyle 0m 23s the patch passed +1 mvnsite 0m 31s the patch passed +1 mvneclipse 0m 11s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 2s the patch passed +1 javadoc 0m 18s the patch passed -1 unit 38m 42s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 61m 56s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.TestRMRestart Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue YARN-6424 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12861495/YARN-6424.01.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 0fa4332297c5 3.13.0-107-generic #154-Ubuntu SMP Tue Dec 20 09:57:27 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 318bfb0 Default Java 1.8.0_121 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-YARN-Build/15457/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/15457/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/15457/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          vrushalic Vrushali C added a comment -


          Thanks for the jira and patch Varun! I am wondering if it should be TimelineV2PublishEvent instead of ApplicationFinishPublishEvent at line 183 in the patch.

          vrushalic Vrushali C added a comment - Thanks for the jira and patch Varun! I am wondering if it should be TimelineV2PublishEvent instead of ApplicationFinishPublishEvent at line 183 in the patch.
          varun_saxena Varun Saxena added a comment -

          vrushalic, ApplicationFinishPublishEvent is what existing TimelineV2EventHandler expects. That's why I am sending it.
          Do you want me to change it to TimelineV2PublishEvent? We will have to change around a bit more of the code though for TimelineV2PublishEvent to take RMApp object.
          Let me know your opinion

          varun_saxena Varun Saxena added a comment - vrushalic , ApplicationFinishPublishEvent is what existing TimelineV2EventHandler expects. That's why I am sending it. Do you want me to change it to TimelineV2PublishEvent? We will have to change around a bit more of the code though for TimelineV2PublishEvent to take RMApp object. Let me know your opinion
          haibochen Haibo Chen added a comment -

          Agree with varun_saxena on that it should be an ApplicationFinishPublishEvent. We may also want to specify SystemMetricsEventType.PUBLISH_ENTITY in ApplicationFinishPublishEvent constructor if that is the only possible type.

          haibochen Haibo Chen added a comment - Agree with varun_saxena on that it should be an ApplicationFinishPublishEvent. We may also want to specify SystemMetricsEventType.PUBLISH_ENTITY in ApplicationFinishPublishEvent constructor if that is the only possible type.
          vrushalic Vrushali C added a comment -

          Thanks varun_saxena. No, we don't need to make more changes. Let's keep the patch as you have it. It looks like ApplicationFinishPublishEvent is being published and handled by TimelineV2 only, V1 does not seem to have it.

          We may also want to specify SystemMetricsEventType.PUBLISH_ENTITY in ApplicationFinishPublishEvent constructor if that is the only possible type.

          I think SystemMetricsEventType does have two types PUBLISH_ENTITY and PUBLISH_APPLICATION_FINISHED_ENTITY , so the current patch should be good.

          +1 LGTM.

          vrushalic Vrushali C added a comment - Thanks varun_saxena . No, we don't need to make more changes. Let's keep the patch as you have it. It looks like ApplicationFinishPublishEvent is being published and handled by TimelineV2 only, V1 does not seem to have it. We may also want to specify SystemMetricsEventType.PUBLISH_ENTITY in ApplicationFinishPublishEvent constructor if that is the only possible type. I think SystemMetricsEventType does have two types PUBLISH_ENTITY and PUBLISH_APPLICATION_FINISHED_ENTITY , so the current patch should be good. +1 LGTM.

          +1 LGTM

          rohithsharma Rohith Sharma K S added a comment - +1 LGTM

          committed to YARN-5355 branch.. thanks Varun for the patch and Vrushali for the review.

          rohithsharma Rohith Sharma K S added a comment - committed to YARN-5355 branch.. thanks Varun for the patch and Vrushali for the review.

          Should it be back ported to YARN-5355-branch2 also?

          rohithsharma Rohith Sharma K S added a comment - Should it be back ported to YARN-5355 -branch2 also?
          varun_saxena Varun Saxena added a comment -

          Thanks rohithsharma
          Yes, it can be committed to YARN-5355-branch-2. I think we can commit it to trunk too. Its basically a bug.

          varun_saxena Varun Saxena added a comment - Thanks rohithsharma Yes, it can be committed to YARN-5355 -branch-2. I think we can commit it to trunk too. Its basically a bug.

          Yes it is bug!! I committed to trunk and YARN-5355-branch-2 also.

          rohithsharma Rohith Sharma K S added a comment - Yes it is bug!! I committed to trunk and YARN-5355 -branch-2 also.
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11540 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11540/)
          YARN-6424. TimelineCollector is not stopped when an app finishes in RM. (rohithsharmaks: rev 1a9439e299910032ce6a1919dece3107c1c9de5b)

          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/metrics/TimelineServiceV2Publisher.java
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11540 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11540/ ) YARN-6424 . TimelineCollector is not stopped when an app finishes in RM. (rohithsharmaks: rev 1a9439e299910032ce6a1919dece3107c1c9de5b) (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/metrics/TimelineServiceV2Publisher.java
          varun_saxena Varun Saxena added a comment -

          We can still keep it as sub-task for YARN-5355 to keep ATSv2 JIRAs' together. Right?

          varun_saxena Varun Saxena added a comment - We can still keep it as sub-task for YARN-5355 to keep ATSv2 JIRAs' together. Right?
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11591 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11591/)
          YARN-6424. TimelineCollector is not stopped when an app finishes in RM. (rohithsharmaks: rev 1a9439e299910032ce6a1919dece3107c1c9de5b)

          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/metrics/TimelineServiceV2Publisher.java
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11591 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11591/ ) YARN-6424 . TimelineCollector is not stopped when an app finishes in RM. (rohithsharmaks: rev 1a9439e299910032ce6a1919dece3107c1c9de5b) (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/metrics/TimelineServiceV2Publisher.java

          People

            varun_saxena Varun Saxena
            varun_saxena Varun Saxena
            Votes:
            0 Vote for this issue
            Watchers:
            7 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: