Uploaded image for project: 'Hadoop YARN'
  1. Hadoop YARN
  2. YARN-5355 YARN Timeline Service v.2: alpha 2
  3. YARN-5699

Retrospect yarn entity fields which are publishing in events info fields.

    Details

    • Type: Sub-task
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.9.0, 3.0.0-alpha2
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Currently, all the container information are published at 2 places. Some of them are at entity info(top-level) and some are at event info.

      For containers, some of the event info should be published at container info level. For example : container exist status, container state, createdTime, finished time. These are general information to container required for container-report. So it is better to publish at top level info field.

      1. 0001-YARN-5699.patch
        16 kB
        Rohith Sharma K S
      2. 0001-YARN-5699.YARN-5355.patch
        16 kB
        Rohith Sharma K S
      3. 0002-YARN-5699.YARN-5355.patch
        11 kB
        Rohith Sharma K S
      4. 0002-YARN-5699.patch
        46 kB
        Rohith Sharma K S
      5. 0003-YARN-5699.patch
        47 kB
        Rohith Sharma K S
      6. 0003-YARN-5699.YARN-5355.patch
        47 kB
        Rohith Sharma K S

        Activity

        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10622 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10622/)
        YARN-5699. Retrospect yarn entity fields which are publishing in events (sjlee: rev 1f304b0c7f261369dd68839507bb609a949965ad)

        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/timelineservice/NMTimelinePublisher.java
        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/test/java/org/apache/hadoop/yarn/server/applicationhistoryservice/TestApplicationHistoryManagerOnTimelineStore.java
        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/metrics/TestSystemMetricsPublisher.java
        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/ApplicationHistoryManagerOnTimelineStore.java
        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/metrics/TimelineServiceV1Publisher.java
        • (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
        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/metrics/AppAttemptMetricsConstants.java
        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/metrics/ContainerMetricsConstants.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10622 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10622/ ) YARN-5699 . Retrospect yarn entity fields which are publishing in events (sjlee: rev 1f304b0c7f261369dd68839507bb609a949965ad) (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/timelineservice/NMTimelinePublisher.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/test/java/org/apache/hadoop/yarn/server/applicationhistoryservice/TestApplicationHistoryManagerOnTimelineStore.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/metrics/TestSystemMetricsPublisher.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/ApplicationHistoryManagerOnTimelineStore.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/metrics/TimelineServiceV1Publisher.java (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 (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/metrics/AppAttemptMetricsConstants.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/metrics/ContainerMetricsConstants.java
        Hide
        sjlee0 Sangjin Lee added a comment -

        Committed it to trunk, YARN-5355 and YARN-5355-branch-2. Thanks Rohith Sharma K S for your contribution, and others for valuable reviews!

        Show
        sjlee0 Sangjin Lee added a comment - Committed it to trunk, YARN-5355 and YARN-5355 -branch-2. Thanks Rohith Sharma K S for your contribution, and others for valuable reviews!
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 18s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
        0 mvndep 0m 51s Maven dependency ordering for branch
        +1 mvninstall 6m 47s YARN-5355 passed
        +1 compile 1m 29s YARN-5355 passed
        +1 checkstyle 0m 30s YARN-5355 passed
        +1 mvnsite 1m 46s YARN-5355 passed
        +1 mvneclipse 0m 55s YARN-5355 passed
        +1 findbugs 2m 40s YARN-5355 passed
        +1 javadoc 1m 5s YARN-5355 passed
        0 mvndep 0m 8s Maven dependency ordering for patch
        +1 mvninstall 1m 26s the patch passed
        +1 compile 1m 27s the patch passed
        +1 javac 1m 27s the patch passed
        +1 checkstyle 0m 28s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server: The patch generated 0 new + 102 unchanged - 11 fixed = 102 total (was 113)
        +1 mvnsite 1m 35s the patch passed
        +1 mvneclipse 0m 46s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 3m 1s the patch passed
        +1 javadoc 0m 56s the patch passed
        +1 unit 0m 25s hadoop-yarn-server-common in the patch passed.
        +1 unit 13m 12s hadoop-yarn-server-nodemanager in the patch passed.
        +1 unit 3m 8s hadoop-yarn-server-applicationhistoryservice in the patch passed.
        -1 unit 35m 55s hadoop-yarn-server-resourcemanager in the patch failed.
        +1 asflicense 0m 16s The patch does not generate ASF License warnings.
        80m 8s



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



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12833590/0003-YARN-5699.YARN-5355.patch
        JIRA Issue YARN-5699
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux b2780d2750ed 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision YARN-5355 / 0c18631
        Default Java 1.8.0_101
        findbugs v3.0.0
        unit https://builds.apache.org/job/PreCommit-YARN-Build/13405/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/13405/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/13405/testReport/
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/13405/console
        Powered by Apache Yetus 0.3.0 http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 18s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. 0 mvndep 0m 51s Maven dependency ordering for branch +1 mvninstall 6m 47s YARN-5355 passed +1 compile 1m 29s YARN-5355 passed +1 checkstyle 0m 30s YARN-5355 passed +1 mvnsite 1m 46s YARN-5355 passed +1 mvneclipse 0m 55s YARN-5355 passed +1 findbugs 2m 40s YARN-5355 passed +1 javadoc 1m 5s YARN-5355 passed 0 mvndep 0m 8s Maven dependency ordering for patch +1 mvninstall 1m 26s the patch passed +1 compile 1m 27s the patch passed +1 javac 1m 27s the patch passed +1 checkstyle 0m 28s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server: The patch generated 0 new + 102 unchanged - 11 fixed = 102 total (was 113) +1 mvnsite 1m 35s the patch passed +1 mvneclipse 0m 46s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 1s the patch passed +1 javadoc 0m 56s the patch passed +1 unit 0m 25s hadoop-yarn-server-common in the patch passed. +1 unit 13m 12s hadoop-yarn-server-nodemanager in the patch passed. +1 unit 3m 8s hadoop-yarn-server-applicationhistoryservice in the patch passed. -1 unit 35m 55s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 80m 8s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.security.TestDelegationTokenRenewer Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12833590/0003-YARN-5699.YARN-5355.patch JIRA Issue YARN-5699 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux b2780d2750ed 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision YARN-5355 / 0c18631 Default Java 1.8.0_101 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-YARN-Build/13405/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/13405/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/13405/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server Console output https://builds.apache.org/job/PreCommit-YARN-Build/13405/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
        Hide
        rohithsharma Rohith Sharma K S added a comment -

        Updated patch to YARN-5355 branch

        Show
        rohithsharma Rohith Sharma K S added a comment - Updated patch to YARN-5355 branch
        Hide
        rohithsharma Rohith Sharma K S added a comment -

        Yes, current patch given based on trunk. Sure, I will upload patch for YARN-5355 branch.

        Show
        rohithsharma Rohith Sharma K S added a comment - Yes, current patch given based on trunk. Sure, I will upload patch for YARN-5355 branch.
        Hide
        sjlee0 Sangjin Lee added a comment -

        The latest patch LGTM. I'll wait until tomorrow to give folks time to chime in before I commit.

        Rohith Sharma K S, just to be clear, is the intent to commit this to trunk? If I take the latest patch (https://issues.apache.org/jira/secure/attachment/12833383/0003-YARN-5699.patch) it does not apply cleanly to the YARN-5355 branch. Could you please upload the corresponding YARN-5355 patch?

        Show
        sjlee0 Sangjin Lee added a comment - The latest patch LGTM. I'll wait until tomorrow to give folks time to chime in before I commit. Rohith Sharma K S , just to be clear, is the intent to commit this to trunk? If I take the latest patch ( https://issues.apache.org/jira/secure/attachment/12833383/0003-YARN-5699.patch ) it does not apply cleanly to the YARN-5355 branch. Could you please upload the corresponding YARN-5355 patch?
        Hide
        rohithsharma Rohith Sharma K S added a comment -

        Test failures are unrelated to patch. YARN-5377 and YARN-5737 are tracking JIRA for test failures.

        Show
        rohithsharma Rohith Sharma K S added a comment - Test failures are unrelated to patch. YARN-5377 and YARN-5737 are tracking JIRA for test failures.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 16s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
        0 mvndep 0m 9s Maven dependency ordering for branch
        +1 mvninstall 8m 11s trunk passed
        +1 compile 1m 41s trunk passed
        +1 checkstyle 0m 32s trunk passed
        +1 mvnsite 1m 55s trunk passed
        +1 mvneclipse 0m 58s trunk passed
        +1 findbugs 2m 57s trunk passed
        +1 javadoc 1m 8s trunk passed
        0 mvndep 0m 8s Maven dependency ordering for patch
        +1 mvninstall 1m 27s the patch passed
        +1 compile 1m 35s the patch passed
        +1 javac 1m 35s the patch passed
        +1 checkstyle 0m 30s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server: The patch generated 0 new + 103 unchanged - 11 fixed = 103 total (was 114)
        +1 mvnsite 1m 43s the patch passed
        +1 mvneclipse 0m 49s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 3m 19s the patch passed
        +1 javadoc 1m 4s the patch passed
        +1 unit 0m 30s hadoop-yarn-server-common in the patch passed.
        -1 unit 14m 59s hadoop-yarn-server-nodemanager in the patch failed.
        -1 unit 2m 45s hadoop-yarn-server-applicationhistoryservice in the patch failed.
        +1 unit 35m 36s hadoop-yarn-server-resourcemanager in the patch passed.
        +1 asflicense 0m 16s The patch does not generate ASF License warnings.
        83m 34s



        Reason Tests
        Failed junit tests hadoop.yarn.server.nodemanager.containermanager.queuing.TestQueuingContainerManager
          hadoop.yarn.server.applicationhistoryservice.webapp.TestAHSWebServices



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

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 16s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. 0 mvndep 0m 9s Maven dependency ordering for branch +1 mvninstall 8m 11s trunk passed +1 compile 1m 41s trunk passed +1 checkstyle 0m 32s trunk passed +1 mvnsite 1m 55s trunk passed +1 mvneclipse 0m 58s trunk passed +1 findbugs 2m 57s trunk passed +1 javadoc 1m 8s trunk passed 0 mvndep 0m 8s Maven dependency ordering for patch +1 mvninstall 1m 27s the patch passed +1 compile 1m 35s the patch passed +1 javac 1m 35s the patch passed +1 checkstyle 0m 30s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server: The patch generated 0 new + 103 unchanged - 11 fixed = 103 total (was 114) +1 mvnsite 1m 43s the patch passed +1 mvneclipse 0m 49s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 19s the patch passed +1 javadoc 1m 4s the patch passed +1 unit 0m 30s hadoop-yarn-server-common in the patch passed. -1 unit 14m 59s hadoop-yarn-server-nodemanager in the patch failed. -1 unit 2m 45s hadoop-yarn-server-applicationhistoryservice in the patch failed. +1 unit 35m 36s hadoop-yarn-server-resourcemanager in the patch passed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 83m 34s Reason Tests Failed junit tests hadoop.yarn.server.nodemanager.containermanager.queuing.TestQueuingContainerManager   hadoop.yarn.server.applicationhistoryservice.webapp.TestAHSWebServices Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12833383/0003-YARN-5699.patch JIRA Issue YARN-5699 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 748573d8db0b 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / dbe663d Default Java 1.8.0_101 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-YARN-Build/13394/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/13394/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-applicationhistoryservice.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/13394/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt https://builds.apache.org/job/PreCommit-YARN-Build/13394/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-applicationhistoryservice.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13394/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server Console output https://builds.apache.org/job/PreCommit-YARN-Build/13394/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
        Hide
        rohithsharma Rohith Sharma K S added a comment -

        Updated the patch fixing review comment and checkstyle.

        Show
        rohithsharma Rohith Sharma K S added a comment - Updated the patch fixing review comment and checkstyle.
        Hide
        rohithsharma Rohith Sharma K S added a comment -

        Basically TestApplicationHistoryManagerOnTimelineStore runs on ATSv1. They publish the entities to store and read it back where they verify each fields for validating published value is read. In ATSv2, there is such framework exists as of now. I have raised a JIRA YARN-5737 to validate each fields that are published in the form of report.

        Show
        rohithsharma Rohith Sharma K S added a comment - Basically TestApplicationHistoryManagerOnTimelineStore runs on ATSv1. They publish the entities to store and read it back where they verify each fields for validating published value is read. In ATSv2, there is such framework exists as of now. I have raised a JIRA YARN-5737 to validate each fields that are published in the form of report.
        Hide
        sjlee0 Sangjin Lee added a comment -

        OK it's more of curiosity on my part as what this test does is obviously different than what we are doing in this patch. I am fine with opening a different JIRA to look into this test.

        Show
        sjlee0 Sangjin Lee added a comment - OK it's more of curiosity on my part as what this test does is obviously different than what we are doing in this patch. I am fine with opening a different JIRA to look into this test.
        Hide
        rohithsharma Rohith Sharma K S added a comment -

        the name DIAGNOSTICS_INFO_INFO doesn't sound quite right; just DIAGNOSTICS_INFO?

        make sense to me, I did not notice this. I will update this.

        I'm a little confused by this test; it appears that we're preparing these entities as before (i.e. all this info is at the event level). Is that intended? Don't we want to reflect the same changes in this test too? I wonder how this test is passing then (or what it's testing even)?

        frankly, I haven not done any modification in this test classes explicitly. I did *MetricsConstants classes refactoring which resulted in changes in some of the test classes. I can look at these test class and if any minor modifications I can handle it in this patch itself, otherwise we might need to re visit whole test class again that could be done in separate JIRA.

        And also I notice that there is no specific test cases that are validating individual fields for entity that are published to ATSv2 in TestSystemMetricsPublisherForV2. Again it is bunch of test cases to be added to validate each fields.

        Show
        rohithsharma Rohith Sharma K S added a comment - the name DIAGNOSTICS_INFO_INFO doesn't sound quite right; just DIAGNOSTICS_INFO? make sense to me, I did not notice this. I will update this. I'm a little confused by this test; it appears that we're preparing these entities as before (i.e. all this info is at the event level). Is that intended? Don't we want to reflect the same changes in this test too? I wonder how this test is passing then (or what it's testing even)? frankly, I haven not done any modification in this test classes explicitly . I did *MetricsConstants classes refactoring which resulted in changes in some of the test classes. I can look at these test class and if any minor modifications I can handle it in this patch itself, otherwise we might need to re visit whole test class again that could be done in separate JIRA. And also I notice that there is no specific test cases that are validating individual fields for entity that are published to ATSv2 in TestSystemMetricsPublisherForV2. Again it is bunch of test cases to be added to validate each fields.
        Hide
        sjlee0 Sangjin Lee added a comment -

        Thanks for the update patch Rohith Sharma K S! I think it's almost there. A few comments.

        (AppAttemptMetricsConstants.java)

        • l.55: the name DIAGNOSTICS_INFO_INFO doesn't sound quite right; just DIAGNOSTICS_INFO?

        (ContainerMetricsConstants.java)

        • l.64: same as above

        (TestApplicationHistoryManagerOnTimelineStore.java)

        • I'm a little confused by this test; it appears that we're preparing these entities as before (i.e. all this info is at the event level). Is that intended? Don't we want to reflect the same changes in this test too? I wonder how this test is passing then (or what it's testing even)?

        (NMTimelinePublisher.java)

        • l.207: I suspect it might be the same, but just to be explicit, should we do toString()?
        Show
        sjlee0 Sangjin Lee added a comment - Thanks for the update patch Rohith Sharma K S ! I think it's almost there. A few comments. (AppAttemptMetricsConstants.java) l.55: the name DIAGNOSTICS_INFO_INFO doesn't sound quite right; just DIAGNOSTICS_INFO ? (ContainerMetricsConstants.java) l.64: same as above (TestApplicationHistoryManagerOnTimelineStore.java) I'm a little confused by this test; it appears that we're preparing these entities as before (i.e. all this info is at the event level). Is that intended? Don't we want to reflect the same changes in this test too? I wonder how this test is passing then (or what it's testing even)? (NMTimelinePublisher.java) l.207: I suspect it might be the same, but just to be explicit, should we do toString() ?
        Hide
        rohithsharma Rohith Sharma K S added a comment -

        Test failure is not related to patch, i.e YARN-5679 handles it.
        Checkstyle is related to patch, I will fix it and upload a new patch.
        Before that I would like to hear review comments if any so that I can upload single patch.

        Show
        rohithsharma Rohith Sharma K S added a comment - Test failure is not related to patch, i.e YARN-5679 handles it. Checkstyle is related to patch, I will fix it and upload a new patch. Before that I would like to hear review comments if any so that I can upload single patch.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 17s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
        0 mvndep 0m 10s Maven dependency ordering for branch
        +1 mvninstall 6m 47s trunk passed
        +1 compile 1m 30s trunk passed
        +1 checkstyle 0m 31s trunk passed
        +1 mvnsite 1m 47s trunk passed
        +1 mvneclipse 0m 54s trunk passed
        +1 findbugs 2m 42s trunk passed
        +1 javadoc 1m 5s trunk passed
        0 mvndep 0m 8s Maven dependency ordering for patch
        +1 mvninstall 1m 25s the patch passed
        +1 compile 1m 26s the patch passed
        +1 javac 1m 26s the patch passed
        -1 checkstyle 0m 28s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server: The patch generated 1 new + 102 unchanged - 11 fixed = 103 total (was 113)
        +1 mvnsite 1m 38s the patch passed
        +1 mvneclipse 0m 45s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 3m 4s the patch passed
        +1 javadoc 0m 56s the patch passed
        +1 unit 0m 24s hadoop-yarn-server-common in the patch passed.
        +1 unit 14m 58s hadoop-yarn-server-nodemanager in the patch passed.
        -1 unit 3m 29s hadoop-yarn-server-applicationhistoryservice in the patch failed.
        +1 unit 38m 59s hadoop-yarn-server-resourcemanager in the patch passed.
        +1 asflicense 0m 16s The patch does not generate ASF License warnings.
        84m 40s



        Reason Tests
        Failed junit tests hadoop.yarn.server.applicationhistoryservice.webapp.TestAHSWebServices



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

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 17s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. 0 mvndep 0m 10s Maven dependency ordering for branch +1 mvninstall 6m 47s trunk passed +1 compile 1m 30s trunk passed +1 checkstyle 0m 31s trunk passed +1 mvnsite 1m 47s trunk passed +1 mvneclipse 0m 54s trunk passed +1 findbugs 2m 42s trunk passed +1 javadoc 1m 5s trunk passed 0 mvndep 0m 8s Maven dependency ordering for patch +1 mvninstall 1m 25s the patch passed +1 compile 1m 26s the patch passed +1 javac 1m 26s the patch passed -1 checkstyle 0m 28s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server: The patch generated 1 new + 102 unchanged - 11 fixed = 103 total (was 113) +1 mvnsite 1m 38s the patch passed +1 mvneclipse 0m 45s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 4s the patch passed +1 javadoc 0m 56s the patch passed +1 unit 0m 24s hadoop-yarn-server-common in the patch passed. +1 unit 14m 58s hadoop-yarn-server-nodemanager in the patch passed. -1 unit 3m 29s hadoop-yarn-server-applicationhistoryservice in the patch failed. +1 unit 38m 59s hadoop-yarn-server-resourcemanager in the patch passed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 84m 40s Reason Tests Failed junit tests hadoop.yarn.server.applicationhistoryservice.webapp.TestAHSWebServices Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12833093/0002-YARN-5699.patch JIRA Issue YARN-5699 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 5a112343ca6d 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 901eca0 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13377/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/13377/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-applicationhistoryservice.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/13377/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-applicationhistoryservice.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13377/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server Console output https://builds.apache.org/job/PreCommit-YARN-Build/13377/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
        Hide
        rohithsharma Rohith Sharma K S added a comment -

        Updating the patch with following changes. Patch is rebased against trunk as this need to commit in trunk.

        1. Added container finish time to get published on container finish event. container create time is not added, user can take this time from entity created time itself.
        2. appUpdated event publish kept unmodified as per review comment.
        3. Removed duplicated information getting published during attempt start and attempt end time as per Varun's review comment.
        4. tracking URL and original trackingURL published in entity info level. Original tracking URL life time is same as application run time. When application is running, client need to construct URL with proxy/without proxy as it is done for rendering AppsBlock.
        5. At last, all the constants are renamed to *_INFO rather than *_EVENT_INFO or *_ENTITY_INFO.
        Show
        rohithsharma Rohith Sharma K S added a comment - Updating the patch with following changes. Patch is rebased against trunk as this need to commit in trunk. Added container finish time to get published on container finish event. container create time is not added, user can take this time from entity created time itself. appUpdated event publish kept unmodified as per review comment. Removed duplicated information getting published during attempt start and attempt end time as per Varun's review comment. tracking URL and original trackingURL published in entity info level. Original tracking URL life time is same as application run time. When application is running, client need to construct URL with proxy/without proxy as it is done for rendering AppsBlock. At last, all the constants are renamed to *_INFO rather than *_EVENT_INFO or *_ENTITY_INFO.
        Hide
        sjlee0 Sangjin Lee added a comment -

        I'd like to know what would be made much easier with having the created time as entity info that is inconvenient or difficult with the explicit created time attribute. I get that it would be more symmetric with the finished time, but that alone is not a strong argument for replicating this info.

        Since we're talking about container entities, this is probably the most number of entities in the storage. If we can help not add a column per object unnecessarily, I think it would be a good thing.

        Show
        sjlee0 Sangjin Lee added a comment - I'd like to know what would be made much easier with having the created time as entity info that is inconvenient or difficult with the explicit created time attribute. I get that it would be more symmetric with the finished time, but that alone is not a strong argument for replicating this info. Since we're talking about container entities, this is probably the most number of entities in the storage. If we can help not add a column per object unnecessarily, I think it would be a good thing.
        Hide
        rohithsharma Rohith Sharma K S added a comment -

        So I'm not sure whether we need to store it again as part of the entity info.

        I agree with Li lu. Even I don't have strong opinion to add duplicate created time with different info key, since it is available in entity level. Though created time is same entity createdTime, adding duplicate createdTime with info key as YARN_CONTAINER_CREATED_TIME and YARN_CONTAINER_FINISHED_TIME makes more user facing to understand it.

        Show
        rohithsharma Rohith Sharma K S added a comment - So I'm not sure whether we need to store it again as part of the entity info. I agree with Li lu. Even I don't have strong opinion to add duplicate created time with different info key, since it is available in entity level. Though created time is same entity createdTime, adding duplicate createdTime with info key as YARN_CONTAINER_CREATED_TIME and YARN_CONTAINER_FINISHED_TIME makes more user facing to understand it.
        Hide
        gtCarrera9 Li Lu added a comment -

        OK, I don't have a strong preference on that. Once the data is available on entity level I think we're fine. The benefit for replicating it is not to make created time different to end time, but I'm fine with either ways.

        Show
        gtCarrera9 Li Lu added a comment - OK, I don't have a strong preference on that. Once the data is available on entity level I think we're fine. The benefit for replicating it is not to make created time different to end time, but I'm fine with either ways.
        Hide
        sjlee0 Sangjin Lee added a comment -

        The created time just seems redundant. Is there something that cannot be done by the explicit created time attribute? Rohith said

        It is only for easier accessibility rather than going through event time stamps.

        We're already setting it to the entity created time, so you would not be going through event time stamps for the created time. So I'm not sure whether we need to store it again as part of the entity info.

        Show
        sjlee0 Sangjin Lee added a comment - The created time just seems redundant. Is there something that cannot be done by the explicit created time attribute? Rohith said It is only for easier accessibility rather than going through event time stamps. We're already setting it to the entity created time, so you would not be going through event time stamps for the created time. So I'm not sure whether we need to store it again as part of the entity info.
        Hide
        gtCarrera9 Li Lu added a comment -

        Thanks Sangjin Lee. This said I'm generally fine with Rohith's planning. End time looks belong to an entity but not just an event, too. For start time, it looks natural to have some queries like "list all the apps start in a time window and finished in a time window"? So if it's not too expensive, I think it's fine to also put it in info field?

        Show
        gtCarrera9 Li Lu added a comment - Thanks Sangjin Lee . This said I'm generally fine with Rohith's planning. End time looks belong to an entity but not just an event, too. For start time, it looks natural to have some queries like "list all the apps start in a time window and finished in a time window"? So if it's not too expensive, I think it's fine to also put it in info field?
        Hide
        sjlee0 Sangjin Lee added a comment -

        Thanks for the clarification. Then what do you think of the currently proposed set of items in the patch? Do you think they all belong at the entity level? I think one thing we were debating was the diagnostic info.

        Show
        sjlee0 Sangjin Lee added a comment - Thanks for the clarification. Then what do you think of the currently proposed set of items in the patch? Do you think they all belong at the entity level? I think one thing we were debating was the diagnostic info.
        Hide
        gtCarrera9 Li Lu added a comment -

        Are you proposing elevating all event info items to the entity info unless they really belong only to events?

        This is my intention. If the event carries information about the timeline entity, why not posting it as an entity level info?

        Show
        gtCarrera9 Li Lu added a comment - Are you proposing elevating all event info items to the entity info unless they really belong only to events? This is my intention. If the event carries information about the timeline entity, why not posting it as an entity level info?
        Hide
        sjlee0 Sangjin Lee added a comment -

        What about end time? End time has to be get from event time stamp. But anyway, I have added a comment for streamlining YARN entities publishing, I think we should decide for providing YARN entities converter rather than fixing individual many bugs.

        Created time is a first-class attribute of an entity, so it would be good to use that instead of a generic entity info field. End time is not a first-class attribute, so entity info would be a natural place.

        Actually let's take one step backward... What is the key benefit for keeping most data in event info, rather than in entity info? To me, storing data in entity info map looks a good representation that the data "belongs to" the whole entity, but not just one event. Even though some data may come from a certain type of timeline event, if it adds useful information to the whole entity, we should put them in entity info field. I'm not sure if there are some implementation side concerns that prevent us from doing this. However, even if so, I believe we should fix those blockers but not solve the problem the other way around (avoid putting data in entity info, but just put them in event info). I would de-prioritize the work on making the v2 code path consistent with v1's if this is a problem for our current design.

        I'm not so sure if I understand what you meant by this. Are you proposing elevating all event info items to the entity info unless they really belong only to events? Or are you proposing not to do this work? Bit confused...

        Show
        sjlee0 Sangjin Lee added a comment - What about end time? End time has to be get from event time stamp. But anyway, I have added a comment for streamlining YARN entities publishing, I think we should decide for providing YARN entities converter rather than fixing individual many bugs. Created time is a first-class attribute of an entity, so it would be good to use that instead of a generic entity info field. End time is not a first-class attribute, so entity info would be a natural place. Actually let's take one step backward... What is the key benefit for keeping most data in event info, rather than in entity info? To me, storing data in entity info map looks a good representation that the data "belongs to" the whole entity, but not just one event. Even though some data may come from a certain type of timeline event, if it adds useful information to the whole entity, we should put them in entity info field. I'm not sure if there are some implementation side concerns that prevent us from doing this. However, even if so, I believe we should fix those blockers but not solve the problem the other way around (avoid putting data in entity info, but just put them in event info). I would de-prioritize the work on making the v2 code path consistent with v1's if this is a problem for our current design. I'm not so sure if I understand what you meant by this. Are you proposing elevating all event info items to the entity info unless they really belong only to events? Or are you proposing not to do this work? Bit confused...
        Hide
        gtCarrera9 Li Lu added a comment -

        Actually let's take one step backward... What is the key benefit for keeping most data in event info, rather than in entity info? To me, storing data in entity info map looks a good representation that the data "belongs to" the whole entity, but not just one event. Even though some data may come from a certain type of timeline event, if it adds useful information to the whole entity, we should put them in entity info field. I'm not sure if there are some implementation side concerns that prevent us from doing this. However, even if so, I believe we should fix those blockers but not solve the problem the other way around (avoid putting data in entity info, but just put them in event info). I would de-prioritize the work on making the v2 code path consistent with v1's if this is a problem for our current design.

        Show
        gtCarrera9 Li Lu added a comment - Actually let's take one step backward... What is the key benefit for keeping most data in event info, rather than in entity info? To me, storing data in entity info map looks a good representation that the data "belongs to" the whole entity, but not just one event. Even though some data may come from a certain type of timeline event, if it adds useful information to the whole entity, we should put them in entity info field. I'm not sure if there are some implementation side concerns that prevent us from doing this. However, even if so, I believe we should fix those blockers but not solve the problem the other way around (avoid putting data in entity info, but just put them in event info). I would de-prioritize the work on making the v2 code path consistent with v1's if this is a problem for our current design.
        Hide
        rohithsharma Rohith Sharma K S added a comment -

        Created time is at the topmost level in an entity. We don't need to go through event timestamps for it.

        What about end time? End time has to be get from event time stamp. But anyway, I have added a comment for streamlining YARN entities publishing, I think we should decide for providing YARN entities converter rather than fixing individual many bugs.

        Show
        rohithsharma Rohith Sharma K S added a comment - Created time is at the topmost level in an entity. We don't need to go through event timestamps for it. What about end time? End time has to be get from event time stamp. But anyway, I have added a comment for streamlining YARN entities publishing, I think we should decide for providing YARN entities converter rather than fixing individual many bugs.
        Hide
        varun_saxena Varun Saxena added a comment -

        It is only for easier accessibility rather than going through event time stamps.

        Created time is at the topmost level in an entity. We don't need to go through event timestamps for it. Other intention of having created time in info would be if we want to send a query like return me all entities which have a specific created time in milliseconds. Highly unlikely somebody would want this. Even this though, can be achieved by using createdTimeStart/end filters.
        To me, we are repeating this information by keeping it in info. I think it will serve your use case even otherwise. Thoughts ?

        cc Rohith Sharma K S

        Show
        varun_saxena Varun Saxena added a comment - It is only for easier accessibility rather than going through event time stamps. Created time is at the topmost level in an entity. We don't need to go through event timestamps for it. Other intention of having created time in info would be if we want to send a query like return me all entities which have a specific created time in milliseconds. Highly unlikely somebody would want this. Even this though, can be achieved by using createdTimeStart/end filters. To me, we are repeating this information by keeping it in info. I think it will serve your use case even otherwise. Thoughts ? cc Rohith Sharma K S
        Hide
        rohithsharma Rohith Sharma K S added a comment -

        To me, goal for moving event info to entity info is User should be able to get application/application-attempt/container reports at entity-info rather than entity-event-info. The main advantage is user need not iterate trough every events to get the report details. Iterating is time consuming when rendering data in web UI by deserializing the pay load.

        Li Lu

        A bigger concern is on the code itself. Right now we have a lot of info key name constants ending as "EVENT_INFO",

        True, I do a change removing EVENT in all the fields in next patches.

        Sangjin Lee

        regarding the container state that was removed in YARN-5156; perhaps we should add it as entity info here (with COMPLETED)?

        I will make a change adding container_state in next patches

        we’re already setting the entity created time in l.190; do we need to add it to the entity info? I would say, we don't need this

        It is only for easier accessibility rather than going through event time stamps.

        Show
        rohithsharma Rohith Sharma K S added a comment - To me, goal for moving event info to entity info is User should be able to get application/application-attempt/container reports at entity-info rather than entity-event-info. The main advantage is user need not iterate trough every events to get the report details. Iterating is time consuming when rendering data in web UI by deserializing the pay load. Li Lu A bigger concern is on the code itself. Right now we have a lot of info key name constants ending as "EVENT_INFO", True, I do a change removing EVENT in all the fields in next patches. Sangjin Lee regarding the container state that was removed in YARN-5156 ; perhaps we should add it as entity info here (with COMPLETED)? I will make a change adding container_state in next patches we’re already setting the entity created time in l.190; do we need to add it to the entity info? I would say, we don't need this It is only for easier accessibility rather than going through event time stamps.
        Hide
        gtCarrera9 Li Lu added a comment -

        Thanks for the work Rohith Sharma K S! Generally fine but a few concerns:
        1. I agree with Varun and Sangjin that we may need to think twice about diagnostic info. This field is way too cumbersome for users to directly query. I think it's fine to leave it in event info.
        2. When we get app state updated event, we may want to post the event info as is, but at the same time update entity level information?
        3. A bigger concern is on the code itself. Right now we have a lot of info key name constants ending as "EVENT_INFO", but actually we no longer use them as event info. Instead, we may post them as entity info, or event info, or both. We need to update or simplify those constants to make them less confusing.

        Show
        gtCarrera9 Li Lu added a comment - Thanks for the work Rohith Sharma K S ! Generally fine but a few concerns: 1. I agree with Varun and Sangjin that we may need to think twice about diagnostic info. This field is way too cumbersome for users to directly query. I think it's fine to leave it in event info. 2. When we get app state updated event, we may want to post the event info as is, but at the same time update entity level information? 3. A bigger concern is on the code itself. Right now we have a lot of info key name constants ending as "EVENT_INFO", but actually we no longer use them as event info. Instead, we may post them as entity info, or event info, or both. We need to update or simplify those constants to make them less confusing.
        Hide
        varun_saxena Varun Saxena added a comment -

        regarding the container state that was removed in YARN-5156; perhaps we should add it as entity info here (with COMPLETED)?

        Agree, we can do so. That was one of the suggested options in YARN-5156, set it to RUNNING in container start and COMPLETED on container end.

        Show
        varun_saxena Varun Saxena added a comment - regarding the container state that was removed in YARN-5156 ; perhaps we should add it as entity info here (with COMPLETED)? Agree, we can do so. That was one of the suggested options in YARN-5156 , set it to RUNNING in container start and COMPLETED on container end.
        Hide
        sjlee0 Sangjin Lee added a comment -

        Thanks Rohith Sharma K S for the patch! Sorry it took me a while to get to this.

        I generally agree with pulling some info from events to the entity level, especially if they can be part of queries. We do want to be careful not to put everything at the entity level, however. We probably want to limit it to only the things we will actually query.

        In more detail,
        (NMTimelinePublisher.java)

        • l.182: we’re already setting the entity created time in l.190; do we need to add it to the entity info? I would say, we don't need this
        • l.203: do we need to add diagnostics to the entity level? Not sure if it is useful. Can we keep it on the event?
        • l.307: the same comment as Varun’s; should be the other way around?
        • regarding the container state that was removed in YARN-5156; perhaps we should add it as entity info here (with COMPLETED)?

        (ContainerMetricsConstants.java)

        • l.44: see NMTimelinePublisher.java

        (TimelineServiceV2Publisher.java)

        • l.158: same as above; is diagnostics needed at the entity level?
        • l.190: this should be entity.setInfo() not tEvent.setInfo()
        • l.289: same as above

        It would be great if we can add some unit tests to check these. Thanks!

        Show
        sjlee0 Sangjin Lee added a comment - Thanks Rohith Sharma K S for the patch! Sorry it took me a while to get to this. I generally agree with pulling some info from events to the entity level, especially if they can be part of queries. We do want to be careful not to put everything at the entity level, however. We probably want to limit it to only the things we will actually query. In more detail, (NMTimelinePublisher.java) l.182: we’re already setting the entity created time in l.190; do we need to add it to the entity info? I would say, we don't need this l.203: do we need to add diagnostics to the entity level? Not sure if it is useful. Can we keep it on the event? l.307: the same comment as Varun’s; should be the other way around? regarding the container state that was removed in YARN-5156 ; perhaps we should add it as entity info here (with COMPLETED)? (ContainerMetricsConstants.java) l.44: see NMTimelinePublisher.java (TimelineServiceV2Publisher.java) l.158: same as above; is diagnostics needed at the entity level? l.190: this should be entity.setInfo() not tEvent.setInfo() l.289: same as above It would be great if we can add some unit tests to check these. Thanks!
        Hide
        varun_saxena Varun Saxena added a comment -

        For tracking URL part though, AHS will not be enabled with ATSv2 and new Web UI.

        Show
        varun_saxena Varun Saxena added a comment - For tracking URL part though, AHS will not be enabled with ATSv2 and new Web UI.
        Hide
        varun_saxena Varun Saxena added a comment -

        Thanks Rohith Sharma K S for the latest patch.
        Few comments:

        1. In NMTimelinePublisher#publishContainerEvent, shouldn't we check against httpAddress being null ? Instead of not being null.
        2. I see that we are publishing tracking URL twice. If AHS is enabled, tracking URL is changed to AHS web endpoint when attempt finishes. So info field will have its latest value as AHS url. In that case it should be attached to event also so that we know the original AM tracking URL too (i.e. when attempt was registered). Thoughts ?
        3. In TimelineServiceV2Publisher#appStateUpdated, changes are not required as we are effectively doing the same thing as before. We should definitely publish this info at event level because event is app state update event. Also should we update it at info level too so that we can filter apps while they are running based on their current state ?
        4. Do we need to publish master container info at app attempt level twice ?

        Updated patch also fixes a another bug i.e always NM published http address port used to come as zero.

        Thanks for the fix.

        Show
        varun_saxena Varun Saxena added a comment - Thanks Rohith Sharma K S for the latest patch. Few comments: In NMTimelinePublisher#publishContainerEvent, shouldn't we check against httpAddress being null ? Instead of not being null. I see that we are publishing tracking URL twice. If AHS is enabled, tracking URL is changed to AHS web endpoint when attempt finishes. So info field will have its latest value as AHS url. In that case it should be attached to event also so that we know the original AM tracking URL too (i.e. when attempt was registered). Thoughts ? In TimelineServiceV2Publisher#appStateUpdated, changes are not required as we are effectively doing the same thing as before. We should definitely publish this info at event level because event is app state update event. Also should we update it at info level too so that we can filter apps while they are running based on their current state ? Do we need to publish master container info at app attempt level twice ? Updated patch also fixes a another bug i.e always NM published http address port used to come as zero. Thanks for the fix.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 17s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 test4tests 0m 0s The patch 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.
        0 mvndep 0m 9s Maven dependency ordering for branch
        +1 mvninstall 7m 2s YARN-5355 passed
        +1 compile 1m 38s YARN-5355 passed
        +1 checkstyle 0m 29s YARN-5355 passed
        +1 mvnsite 1m 34s YARN-5355 passed
        +1 mvneclipse 0m 41s YARN-5355 passed
        +1 findbugs 2m 18s YARN-5355 passed
        +1 javadoc 0m 51s YARN-5355 passed
        0 mvndep 0m 9s Maven dependency ordering for patch
        +1 mvninstall 1m 8s the patch passed
        +1 compile 1m 26s the patch passed
        +1 javac 1m 26s the patch passed
        +1 checkstyle 0m 27s the patch passed
        +1 mvnsite 1m 18s the patch passed
        +1 mvneclipse 0m 35s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 2m 24s the patch passed
        +1 javadoc 0m 44s the patch passed
        +1 unit 0m 25s hadoop-yarn-server-common in the patch passed.
        +1 unit 13m 11s hadoop-yarn-server-nodemanager in the patch passed.
        +1 unit 36m 41s hadoop-yarn-server-resourcemanager in the patch passed.
        +1 asflicense 0m 16s The patch does not generate ASF License warnings.
        74m 35s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12831965/0002-YARN-5699.YARN-5355.patch
        JIRA Issue YARN-5699
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 35d5a49f12ea 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision YARN-5355 / 5d7ad39
        Default Java 1.8.0_101
        findbugs v3.0.0
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13307/testReport/
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/13307/console
        Powered by Apache Yetus 0.3.0 http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 17s 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. 0 mvndep 0m 9s Maven dependency ordering for branch +1 mvninstall 7m 2s YARN-5355 passed +1 compile 1m 38s YARN-5355 passed +1 checkstyle 0m 29s YARN-5355 passed +1 mvnsite 1m 34s YARN-5355 passed +1 mvneclipse 0m 41s YARN-5355 passed +1 findbugs 2m 18s YARN-5355 passed +1 javadoc 0m 51s YARN-5355 passed 0 mvndep 0m 9s Maven dependency ordering for patch +1 mvninstall 1m 8s the patch passed +1 compile 1m 26s the patch passed +1 javac 1m 26s the patch passed +1 checkstyle 0m 27s the patch passed +1 mvnsite 1m 18s the patch passed +1 mvneclipse 0m 35s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 2m 24s the patch passed +1 javadoc 0m 44s the patch passed +1 unit 0m 25s hadoop-yarn-server-common in the patch passed. +1 unit 13m 11s hadoop-yarn-server-nodemanager in the patch passed. +1 unit 36m 41s hadoop-yarn-server-resourcemanager in the patch passed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 74m 35s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12831965/0002-YARN-5699.YARN-5355.patch JIRA Issue YARN-5699 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 35d5a49f12ea 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision YARN-5355 / 5d7ad39 Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13307/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server Console output https://builds.apache.org/job/PreCommit-YARN-Build/13307/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
        Hide
        rohithsharma Rohith Sharma K S added a comment -

        Updated patch also fixes a another bug i.e always NM published http address port used to come as zero. This is because of when NMPublisher started, web server was not started which resulting in http port used be zero.

        Show
        rohithsharma Rohith Sharma K S added a comment - Updated patch also fixes a another bug i.e always NM published http address port used to come as zero. This is because of when NMPublisher started, web server was not started which resulting in http port used be zero.
        Hide
        rohithsharma Rohith Sharma K S added a comment -

        Updated patch by reverting changes made in NMTimelinePublisher.

        Show
        rohithsharma Rohith Sharma K S added a comment - Updated patch by reverting changes made in NMTimelinePublisher.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 21s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 test4tests 0m 0s The patch 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.
        0 mvndep 3m 48s Maven dependency ordering for branch
        +1 mvninstall 7m 45s YARN-5355 passed
        +1 compile 1m 34s YARN-5355 passed
        +1 checkstyle 0m 33s YARN-5355 passed
        +1 mvnsite 1m 31s YARN-5355 passed
        +1 mvneclipse 0m 46s YARN-5355 passed
        +1 findbugs 2m 17s YARN-5355 passed
        +1 javadoc 0m 55s YARN-5355 passed
        0 mvndep 0m 9s Maven dependency ordering for patch
        +1 mvninstall 1m 14s the patch passed
        +1 compile 1m 33s the patch passed
        +1 javac 1m 33s the patch passed
        +1 checkstyle 0m 28s the patch passed
        +1 mvnsite 1m 19s the patch passed
        +1 mvneclipse 0m 34s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 2m 30s the patch passed
        +1 javadoc 0m 50s the patch passed
        +1 unit 0m 25s hadoop-yarn-server-common in the patch passed.
        +1 unit 13m 10s hadoop-yarn-server-nodemanager in the patch passed.
        -1 unit 37m 4s hadoop-yarn-server-resourcemanager in the patch failed.
        +1 asflicense 0m 16s The patch does not generate ASF License warnings.
        80m 1s



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



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12831943/0001-YARN-5699.YARN-5355.patch
        JIRA Issue YARN-5699
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 3c5553fe091d 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision YARN-5355 / 5d7ad39
        Default Java 1.8.0_101
        findbugs v3.0.0
        unit https://builds.apache.org/job/PreCommit-YARN-Build/13306/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/13306/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/13306/testReport/
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/13306/console
        Powered by Apache Yetus 0.3.0 http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 21s 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. 0 mvndep 3m 48s Maven dependency ordering for branch +1 mvninstall 7m 45s YARN-5355 passed +1 compile 1m 34s YARN-5355 passed +1 checkstyle 0m 33s YARN-5355 passed +1 mvnsite 1m 31s YARN-5355 passed +1 mvneclipse 0m 46s YARN-5355 passed +1 findbugs 2m 17s YARN-5355 passed +1 javadoc 0m 55s YARN-5355 passed 0 mvndep 0m 9s Maven dependency ordering for patch +1 mvninstall 1m 14s the patch passed +1 compile 1m 33s the patch passed +1 javac 1m 33s the patch passed +1 checkstyle 0m 28s the patch passed +1 mvnsite 1m 19s the patch passed +1 mvneclipse 0m 34s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 2m 30s the patch passed +1 javadoc 0m 50s the patch passed +1 unit 0m 25s hadoop-yarn-server-common in the patch passed. +1 unit 13m 10s hadoop-yarn-server-nodemanager in the patch passed. -1 unit 37m 4s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 80m 1s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.capacity.TestNodeLabelContainerAllocation Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12831943/0001-YARN-5699.YARN-5355.patch JIRA Issue YARN-5699 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 3c5553fe091d 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision YARN-5355 / 5d7ad39 Default Java 1.8.0_101 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-YARN-Build/13306/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/13306/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/13306/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server Console output https://builds.apache.org/job/PreCommit-YARN-Build/13306/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
        Hide
        varun_saxena Varun Saxena added a comment -

        Otherwise we miss the this information else container_state info should be published from NM

        So the reason this was removed in YARN-5156 was that container state can either be RUNNING and COMPLETED and it becomes COMPLETED only on CONTAINER_FINISHED event. Which means it can be easily deduced from event. At the time container state was carried in event info so the majority opinion went with removing container state altogether.
        I have made a comment on YARN-5156 too.

        Show
        varun_saxena Varun Saxena added a comment - Otherwise we miss the this information else container_state info should be published from NM So the reason this was removed in YARN-5156 was that container state can either be RUNNING and COMPLETED and it becomes COMPLETED only on CONTAINER_FINISHED event. Which means it can be easily deduced from event. At the time container state was carried in event info so the majority opinion went with removing container state altogether. I have made a comment on YARN-5156 too.
        Hide
        Naganarasimha Naganarasimha G R added a comment -

        Had missed YARN-5156 let me check how to include that scenario too

        Show
        Naganarasimha Naganarasimha G R added a comment - Had missed YARN-5156 let me check how to include that scenario too
        Hide
        Naganarasimha Naganarasimha G R added a comment -

        There was a detailed discussion on this and then we concluded it to be in NM only and yes all the required information is getting published from NM right ?

        Show
        Naganarasimha Naganarasimha G R added a comment - There was a detailed discussion on this and then we concluded it to be in NM only and yes all the required information is getting published from NM right ?
        Hide
        rohithsharma Rohith Sharma K S added a comment -

        Is this intentional ?

        yes, because the same information also published by RM. So I think we can keep at one place.

        [
          {
            "events": [
              {
                "id": "YARN_RM_CONTAINER_FINISHED",
                "timestamp": 1475469588288,
                "info": {
                  "YARN_CONTAINER_STATE": "COMPLETE",
                  "YARN_CONTAINER_EXIT_STATUS": 0,
                  "YARN_CONTAINER_DIAGNOSTICS_INFO": ""
                }
              },
              {
                "id": "YARN_RM_CONTAINER_CREATED",
                "timestamp": 1475469570721,
                "info": {
                  "YARN_CONTAINER_ALLOCATED_PORT": 25006,
                  "YARN_CONTAINER_ALLOCATED_MEMORY": 1024,
                  "YARN_CONTAINER_ALLOCATED_PRIORITY": 0,
                  "YARN_CONTAINER_ALLOCATED_HOST_HTTP_ADDRESS": "http://ctr-e29-1471411959733-0412-01-000004.hwx.site:25008",
                  "YARN_CONTAINER_ALLOCATED_HOST": "ctr-e29-1471411959733-0412-01-000004.hwx.site",
                  "YARN_CONTAINER_ALLOCATED_VCORE": 1
                }
              },
              {
                "id": "YARN_CONTAINER_CREATED",
                "timestamp": -1,
                "info": {}
              },
              {
                "id": "YARN_CONTAINER_FINISHED",
                "timestamp": -1,
                "info": {
                  "YARN_CONTAINER_STATE": "RUNNING",
                  "YARN_CONTAINER_EXIT_STATUS": 0,
                  "YARN_CONTAINER_DIAGNOSTICS_INFO": ""
                }
              },
              {
                "id": "YARN_NM_CONTAINER_LOCALIZATION_FINISHED",
                "timestamp": -1,
                "info": {}
              },
              {
                "id": "YARN_NM_CONTAINER_LOCALIZATION_STARTED",
                "timestamp": -1,
                "info": {}
              }
            ],
            "type": "YARN_CONTAINER",
            "id": "container_e09_1475277121920_0010_01_000001",
            "createdtime": -1,
            "info": {
              "YARN_CONTAINER_ALLOCATED_PORT": 25006,
              "UID": "yarn-cluster!application_1475277121920_0010!YARN_CONTAINER!container_e09_1475277121920_0010_01_000001",
              "YARN_CONTAINER_ALLOCATED_MEMORY": 1024,
              "SYSTEM_INFO_PARENT_ENTITY": {
                "type": "YARN_APPLICATION_ATTEMPT",
                "id": "appattempt_1475277121920_0010_000001"
              },
              "YARN_CONTAINER_ALLOCATED_PRIORITY": "0",
              "YARN_CONTAINER_ALLOCATED_HOST": "ctr-e29-1471411959733-0412-01-000004.hwx.site",
              "YARN_CONTAINER_ALLOCATED_HOST_HTTP_ADDRESS": "ctr-e29-1471411959733-0412-01-000004.hwx.site:0",
              "YARN_CONTAINER_ALLOCATED_VCORE": 1
            },
            "configs": {},
            "isrelatedto": {},
            "relatesto": {}
          }
        ]
        

        Container event publishing from RM is optional.

        I see. But I think we should make it mandatory to publish from RM since some information always get from RM like container_state. Otherwise we miss the this information else container_state info should be published from NM. YARN-5156 removes from NM publisher.

        Show
        rohithsharma Rohith Sharma K S added a comment - Is this intentional ? yes, because the same information also published by RM. So I think we can keep at one place. [ { "events" : [ { "id" : "YARN_RM_CONTAINER_FINISHED" , "timestamp" : 1475469588288, "info" : { "YARN_CONTAINER_STATE" : "COMPLETE" , "YARN_CONTAINER_EXIT_STATUS" : 0, "YARN_CONTAINER_DIAGNOSTICS_INFO" : "" } }, { "id" : "YARN_RM_CONTAINER_CREATED" , "timestamp" : 1475469570721, "info" : { "YARN_CONTAINER_ALLOCATED_PORT" : 25006, "YARN_CONTAINER_ALLOCATED_MEMORY" : 1024, "YARN_CONTAINER_ALLOCATED_PRIORITY" : 0, "YARN_CONTAINER_ALLOCATED_HOST_HTTP_ADDRESS" : "http: //ctr-e29-1471411959733-0412-01-000004.hwx.site:25008" , "YARN_CONTAINER_ALLOCATED_HOST" : "ctr-e29-1471411959733-0412-01-000004.hwx.site" , "YARN_CONTAINER_ALLOCATED_VCORE" : 1 } }, { "id" : "YARN_CONTAINER_CREATED" , "timestamp" : -1, "info" : {} }, { "id" : "YARN_CONTAINER_FINISHED" , "timestamp" : -1, "info" : { "YARN_CONTAINER_STATE" : "RUNNING" , "YARN_CONTAINER_EXIT_STATUS" : 0, "YARN_CONTAINER_DIAGNOSTICS_INFO" : "" } }, { "id" : "YARN_NM_CONTAINER_LOCALIZATION_FINISHED" , "timestamp" : -1, "info" : {} }, { "id" : "YARN_NM_CONTAINER_LOCALIZATION_STARTED" , "timestamp" : -1, "info" : {} } ], "type" : "YARN_CONTAINER" , "id" : "container_e09_1475277121920_0010_01_000001" , "createdtime" : -1, "info" : { "YARN_CONTAINER_ALLOCATED_PORT" : 25006, "UID" : "yarn-cluster!application_1475277121920_0010!YARN_CONTAINER!container_e09_1475277121920_0010_01_000001" , "YARN_CONTAINER_ALLOCATED_MEMORY" : 1024, "SYSTEM_INFO_PARENT_ENTITY" : { "type" : "YARN_APPLICATION_ATTEMPT" , "id" : "appattempt_1475277121920_0010_000001" }, "YARN_CONTAINER_ALLOCATED_PRIORITY" : "0" , "YARN_CONTAINER_ALLOCATED_HOST" : "ctr-e29-1471411959733-0412-01-000004.hwx.site" , "YARN_CONTAINER_ALLOCATED_HOST_HTTP_ADDRESS" : "ctr-e29-1471411959733-0412-01-000004.hwx.site:0" , "YARN_CONTAINER_ALLOCATED_VCORE" : 1 }, "configs" : {}, "isrelatedto" : {}, "relatesto" : {} } ] Container event publishing from RM is optional. I see. But I think we should make it mandatory to publish from RM since some information always get from RM like container_state. Otherwise we miss the this information else container_state info should be published from NM. YARN-5156 removes from NM publisher.
        Hide
        varun_saxena Varun Saxena added a comment -

        Rohith Sharma K S, thanks for the patch.
        In the patch I see you have removed container related info from NMTimelinePublisher. Is this intentional ?
        Container event publishing from RM is optional. And will be ideally be switched off due to the volume and impact on RM.

        Show
        varun_saxena Varun Saxena added a comment - Rohith Sharma K S , thanks for the patch. In the patch I see you have removed container related info from NMTimelinePublisher. Is this intentional ? Container event publishing from RM is optional. And will be ideally be switched off due to the volume and impact on RM.
        Hide
        rohithsharma Rohith Sharma K S added a comment -

        updating same patch to branch YARN-5355

        Show
        rohithsharma Rohith Sharma K S added a comment - updating same patch to branch YARN-5355
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 0s Docker mode activated.
        -1 patch 0m 5s YARN-5699 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help.



        Subsystem Report/Notes
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12831941/0001-YARN-5699.patch
        JIRA Issue YARN-5699
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/13305/console
        Powered by Apache Yetus 0.3.0 http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s Docker mode activated. -1 patch 0m 5s YARN-5699 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help. Subsystem Report/Notes JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12831941/0001-YARN-5699.patch JIRA Issue YARN-5699 Console output https://builds.apache.org/job/PreCommit-YARN-Build/13305/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
        Hide
        rohithsharma Rohith Sharma K S added a comment -

        Updated the patch with following changes.
        Application :

        1. YARN_APPLICATION_FINISHED event has some app level information, I have moved to entity level info.

        ApplicationAttempt :

        1. Events YARN_APPLICATION_ATTEMPT_FINISHED and YARN_APPLICATION_ATTEMPT_REGISTERED has application report specific info details. These have been moved to entity info level.

        Container :

        1. Events YARN_RM_CONTAINER_CREATED and YARN_RM_CONTAINER_FINISHED had container report specific info details. These have been moved to entity info level.
        2. Remove duplicated information which was published from NM publisher info.
        3. Added container created and container finished information in entity level info.
        Show
        rohithsharma Rohith Sharma K S added a comment - Updated the patch with following changes. Application : YARN_APPLICATION_FINISHED event has some app level information, I have moved to entity level info. ApplicationAttempt : Events YARN_APPLICATION_ATTEMPT_FINISHED and YARN_APPLICATION_ATTEMPT_REGISTERED has application report specific info details. These have been moved to entity info level. Container : Events YARN_RM_CONTAINER_CREATED and YARN_RM_CONTAINER_FINISHED had container report specific info details. These have been moved to entity info level. Remove duplicated information which was published from NM publisher info. Added container created and container finished information in entity level info.
        Hide
        rohithsharma Rohith Sharma K S added a comment -

        I think this is the time to decide should we support translation layer for YARN entities to get attempts-report, container-reports, application-report. It can be additional web service in ATS2 similar to /ws/v1/applicationhistory.

        Major concern of this JIRA is about deserializing attempt-report and container-report for Web UI. If we get much easier translation layer from java, it would be much easier. Other wise, it is tedious task for deserializing entity payload if any change in the publishing entity.

        Show
        rohithsharma Rohith Sharma K S added a comment - I think this is the time to decide should we support translation layer for YARN entities to get attempts-report, container-reports, application-report. It can be additional web service in ATS2 similar to /ws/v1/applicationhistory . Major concern of this JIRA is about deserializing attempt-report and container-report for Web UI. If we get much easier translation layer from java, it would be much easier. Other wise, it is tedious task for deserializing entity payload if any change in the publishing entity.
        Hide
        Naganarasimha Naganarasimha G R added a comment -

        Thanks Rohith Sharma K S & Li Lu for detailing it out.

        For example : container exist status, container state, createdTime, finished time.

        Pushing these kind of information both the places would be fine but IMO diagnostics kind of stuff need not be stored. Both the places to ensure existing clients need not be impacted.

        Thirdly, event RM_CONTAINER_CREATED holds diagnostics, state and status.

        IIUC you mean container finished right ?

        In between (not related to this jira ) One more thing i was noticing was how will user know what are the different entity types present which needs to be queried, do we need to capture in our documentation ? For example MR entities, what are the different entity ID's to query ? Where are the MR Job configs present? how to access the MR job counters?
        Similarly for YARN system entities to query the resource usage and the containers stats(number of containers for different priority, time taken to finish etc ...)
        These were some of the queries which i have encountered from others...

        Show
        Naganarasimha Naganarasimha G R added a comment - Thanks Rohith Sharma K S & Li Lu for detailing it out. For example : container exist status, container state, createdTime, finished time. Pushing these kind of information both the places would be fine but IMO diagnostics kind of stuff need not be stored. Both the places to ensure existing clients need not be impacted. Thirdly, event RM_CONTAINER_CREATED holds diagnostics, state and status. IIUC you mean container finished right ? In between (not related to this jira ) One more thing i was noticing was how will user know what are the different entity types present which needs to be queried, do we need to capture in our documentation ? For example MR entities, what are the different entity ID's to query ? Where are the MR Job configs present? how to access the MR job counters? Similarly for YARN system entities to query the resource usage and the containers stats(number of containers for different priority, time taken to finish etc ...) These were some of the queries which i have encountered from others...
        Hide
        varun_saxena Varun Saxena added a comment - - edited

        For containers, some of the event info should be published at container info level.

        Agree with the intention behind this JIRA Rohith Sharma K S . Some info which we may have to query and does not change per event i.e. this info is not repeated across events, we should have at entity info level. It will enable users to use info filters for it.
        Only info which is strictly specific to an event is what can be kept at event info level. This we can keep as the thumb rule for publishing info. However, we can also consider publishing it at both places on need basis (i.e. if and when we find a use case).

        Show
        varun_saxena Varun Saxena added a comment - - edited For containers, some of the event info should be published at container info level. Agree with the intention behind this JIRA Rohith Sharma K S . Some info which we may have to query and does not change per event i.e. this info is not repeated across events, we should have at entity info level. It will enable users to use info filters for it. Only info which is strictly specific to an event is what can be kept at event info level. This we can keep as the thumb rule for publishing info. However, we can also consider publishing it at both places on need basis (i.e. if and when we find a use case).
        Hide
        rohithsharma Rohith Sharma K S added a comment -

        When I raised JIRA, my concern was too as Li mentioned. We have lost ability to easily query yarn entities by adding few information in event info fields.

        Currently, ATS does not support querying for event-info filter. And even if ATS supports, it is again a burden for user to parse the information from different events. For example,YARN container has information 3 different places. Firstly, info field of entity about user, host detail, priority etc. Secondly, event RM_CONTAINER_CREATED holds information about container start time and other. Thirdly, event RM_CONTAINER_CREATED holds diagnostics, state and status.

        So, I think entity related information better to publish at entity info level. It is basically refactoring code.

        Show
        rohithsharma Rohith Sharma K S added a comment - When I raised JIRA, my concern was too as Li mentioned. We have lost ability to easily query yarn entities by adding few information in event info fields. Currently, ATS does not support querying for event-info filter. And even if ATS supports, it is again a burden for user to parse the information from different events. For example,YARN container has information 3 different places. Firstly, info field of entity about user, host detail, priority etc. Secondly, event RM_CONTAINER_CREATED holds information about container start time and other. Thirdly, event RM_CONTAINER_CREATED holds diagnostics, state and status. So, I think entity related information better to publish at entity info level. It is basically refactoring code.
        Hide
        gtCarrera9 Li Lu added a comment -

        I think the major concern is that when putting useful fields into event info field instead of entity info field, we lost the ability to easily query them. For example, if an user would like to list all containers with a specific exit code, or finished within a time window. This said, I think the key point is to put data related to the whole entity into the entity's info field. Ideally we should only put some quite lightweight data into the event's info fields (since they're not easily to query), but if we share a lot of code path we may want to either replicate those data in entity info, or refactor the code?

        Show
        gtCarrera9 Li Lu added a comment - I think the major concern is that when putting useful fields into event info field instead of entity info field, we lost the ability to easily query them. For example, if an user would like to list all containers with a specific exit code, or finished within a time window. This said, I think the key point is to put data related to the whole entity into the entity's info field. Ideally we should only put some quite lightweight data into the event's info fields (since they're not easily to query), but if we share a lot of code path we may want to either replicate those data in entity info, or refactor the code?
        Hide
        Naganarasimha Naganarasimha G R added a comment -

        Rohith Sharma K S,
        This i had tried to do mimic same as in ATSv1 so that the users of the ATS v1 are not impacted. Any particular reason to move it to info level ?

        Show
        Naganarasimha Naganarasimha G R added a comment - Rohith Sharma K S , This i had tried to do mimic same as in ATSv1 so that the users of the ATS v1 are not impacted. Any particular reason to move it to info level ?

          People

          • Assignee:
            rohithsharma Rohith Sharma K S
            Reporter:
            rohithsharma Rohith Sharma K S
          • Votes:
            0 Vote for this issue
            Watchers:
            7 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development