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

Remove unused variables in ContainersMonitorImpl and add debug log for overall resource usage by all containers

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Trivial
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.8.0, 3.0.0-alpha1
    • Component/s: nodemanager
    • Labels:
    • Target Version/s:
    • Hadoop Flags:
      Reviewed

      Description

      Some local variables in MonitoringThread.run() : vmemStillInUsage and pmemStillInUsage are not used and just updated.
      Instead we need to add debug log for overall resource usage by all containers

      1. YARN-3513.20150421-1.patch
        3 kB
        Naganarasimha G R
      2. YARN-3513.20150503-1.patch
        2 kB
        Naganarasimha G R
      3. YARN-3513.20150506-1.patch
        3 kB
        Naganarasimha G R
      4. YARN-3513.20150507-1.patch
        3 kB
        Naganarasimha G R
      5. YARN-3513.20150508-1.patch
        3 kB
        Naganarasimha G R
      6. YARN-3513.20150508-1.patch
        3 kB
        Naganarasimha G R
      7. YARN-3513.20150511-1.patch
        3 kB
        Naganarasimha G R

        Activity

        Hide
        Naganarasimha Naganarasimha G R added a comment -

        attaching a patch with removing the unused variables

        Show
        Naganarasimha Naganarasimha G R added a comment - attaching a patch with removing the unused variables
        Hide
        hadoopqa Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12726632/YARN-3513.20150421-1.patch
        against trunk revision f967fd2.

        +1 @author. The patch does not contain any @author tags.

        -1 tests included. 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 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. There were no new javadoc warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager.

        Test results: https://builds.apache.org/job/PreCommit-YARN-Build/7402//testReport/
        Console output: https://builds.apache.org/job/PreCommit-YARN-Build/7402//console

        This message is automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12726632/YARN-3513.20150421-1.patch against trunk revision f967fd2. +1 @author . The patch does not contain any @author tags. -1 tests included . 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 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager. Test results: https://builds.apache.org/job/PreCommit-YARN-Build/7402//testReport/ Console output: https://builds.apache.org/job/PreCommit-YARN-Build/7402//console This message is automatically generated.
        Hide
        gtCarrera9 Li Lu added a comment -

        Hi Naganarasimha G R, thanks for the patch. +1 for removing vmemStillInUsage and pmemStillInUsage. However, I noticed that we're using content in the 2928 branch. Since we're planning for a branch merge, potentially soon, maybe it's fine to leave it there?

        Show
        gtCarrera9 Li Lu added a comment - Hi Naganarasimha G R , thanks for the patch. +1 for removing vmemStillInUsage and pmemStillInUsage . However, I noticed that we're using content in the 2928 branch. Since we're planning for a branch merge, potentially soon, maybe it's fine to leave it there?
        Hide
        Naganarasimha Naganarasimha G R added a comment -

        Thanks for commenting on this Li Lu (inactive), I found out this when i was referring to YARN-3334 (2928 subjira) patch modifications after it was committed, and as per the code in 2928 branch, vmemStillInUsage and pmemStillInUsage has not been made use and other variables currentPmemUsage and cpuUsageTotalCoresPercentage have been used to publish the container metrics to ATS.

        Show
        Naganarasimha Naganarasimha G R added a comment - Thanks for commenting on this Li Lu (inactive) , I found out this when i was referring to YARN-3334 (2928 subjira) patch modifications after it was committed, and as per the code in 2928 branch, vmemStillInUsage and pmemStillInUsage has not been made use and other variables currentPmemUsage and cpuUsageTotalCoresPercentage have been used to publish the container metrics to ATS.
        Hide
        gtCarrera9 Li Lu added a comment -

        No I was talking about these lines in YARN-3334:

        +            try {
        +              TimelineClient timelineClient = context.getApplications().get(
        +                  containerId.getApplicationAttemptId().getApplicationId()).
        +                      getTimelineClient();
        +              putEntityWithoutBlocking(timelineClient, entity);
        +            }
        

        which refs context and will have problems with

        -  private final Context context;
        

        This may be fine in trunk, but since YARN-2928 needs to merge back in near future, we may not want to make the change on content for now. We need to consider code clean up comprehensively when we're doing the branch merge.

        Show
        gtCarrera9 Li Lu added a comment - No I was talking about these lines in YARN-3334 : + try { + TimelineClient timelineClient = context.getApplications().get( + containerId.getApplicationAttemptId().getApplicationId()). + getTimelineClient(); + putEntityWithoutBlocking(timelineClient, entity); + } which refs context and will have problems with - private final Context context; This may be fine in trunk, but since YARN-2928 needs to merge back in near future, we may not want to make the change on content for now. We need to consider code clean up comprehensively when we're doing the branch merge.
        Hide
        vinodkv Vinod Kumar Vavilapalli added a comment -

        Removing fix-version, use the target-version field for specifying your intent.

        Show
        vinodkv Vinod Kumar Vavilapalli added a comment - Removing fix-version, use the target-version field for specifying your intent.
        Hide
        Naganarasimha Naganarasimha G R added a comment -

        Hi Li Lu

        we're using content in the 2928 branch

        earlier you had mentioned as content hence could not get it. you are right context is made use of and earlier missed to catch it. have uploaded the patch with removing context

        Removing fix-version, use the target-version field for specifying your intent.

        By mistake mentioned it as fix-version instead of target-version. correcting it

        Show
        Naganarasimha Naganarasimha G R added a comment - Hi Li Lu we're using content in the 2928 branch earlier you had mentioned as content hence could not get it. you are right context is made use of and earlier missed to catch it. have uploaded the patch with removing context Removing fix-version, use the target-version field for specifying your intent. By mistake mentioned it as fix-version instead of target-version. correcting it
        Hide
        hadoopqa Hadoop QA added a comment -



        -1 overall



        Vote Subsystem Runtime Comment
        0 pre-patch 14m 38s Pre-patch trunk compilation is healthy.
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 tests included 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 javac 7m 32s There were no new javac warning messages.
        +1 javadoc 9m 34s There were no new javadoc warning messages.
        +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings.
        -1 checkstyle 0m 36s The applied patch generated 1 new checkstyle issues (total was 28, now 28).
        +1 whitespace 0m 0s The patch has no lines that end in whitespace.
        +1 install 1m 35s mvn install still works.
        +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse.
        +1 findbugs 1m 3s The patch does not introduce any new Findbugs (version 2.0.3) warnings.
        +1 yarn tests 5m 51s Tests passed in hadoop-yarn-server-nodemanager.
            41m 47s  



        Subsystem Report/Notes
        Patch URL http://issues.apache.org/jira/secure/attachment/12730024/YARN-3513.20150503-1.patch
        Optional Tests javadoc javac unit findbugs checkstyle
        git revision trunk / a319771
        checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/7677/artifact/patchprocess/diffcheckstylehadoop-yarn-server-nodemanager.txt
        hadoop-yarn-server-nodemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/7677/artifact/patchprocess/testrun_hadoop-yarn-server-nodemanager.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/7677/testReport/
        Java 1.7.0_55
        uname Linux asf902.gq1.ygridcore.net 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
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/7677/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 14m 38s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. -1 tests included 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 javac 7m 32s There were no new javac warning messages. +1 javadoc 9m 34s There were no new javadoc warning messages. +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings. -1 checkstyle 0m 36s The applied patch generated 1 new checkstyle issues (total was 28, now 28). +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 35s mvn install still works. +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse. +1 findbugs 1m 3s The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 yarn tests 5m 51s Tests passed in hadoop-yarn-server-nodemanager.     41m 47s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12730024/YARN-3513.20150503-1.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / a319771 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/7677/artifact/patchprocess/diffcheckstylehadoop-yarn-server-nodemanager.txt hadoop-yarn-server-nodemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/7677/artifact/patchprocess/testrun_hadoop-yarn-server-nodemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/7677/testReport/ Java 1.7.0_55 uname Linux asf902.gq1.ygridcore.net 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 Console output https://builds.apache.org/job/PreCommit-YARN-Build/7677/console This message was automatically generated.
        Hide
        vinodkv Vinod Kumar Vavilapalli added a comment -

        This may be fine in trunk, but since YARN-2928 needs to merge back in near future, we may not want to make the change on content for now. We need to consider code clean up comprehensively when we're doing the branch merge.

        Is there agreement on this? If so, let's commit this (and any other change required) to YARN-2928 first?

        Show
        vinodkv Vinod Kumar Vavilapalli added a comment - This may be fine in trunk, but since YARN-2928 needs to merge back in near future, we may not want to make the change on content for now. We need to consider code clean up comprehensively when we're doing the branch merge. Is there agreement on this? If so, let's commit this (and any other change required) to YARN-2928 first?
        Hide
        djp Junping Du added a comment -

        Sorry guys for coming with late comments. I have different opinion that we should make use of these unused variables instead of removing it - e.g. log in debug level for understanding the overall memory resource consumption across all running containers inside of NM. Thoughts?

        Show
        djp Junping Du added a comment - Sorry guys for coming with late comments. I have different opinion that we should make use of these unused variables instead of removing it - e.g. log in debug level for understanding the overall memory resource consumption across all running containers inside of NM. Thoughts?
        Hide
        Naganarasimha Naganarasimha G R added a comment -

        Thanks for the comments Junping Du,
        +1 for logging, i think initially too they might have intended for the same reason and some how missed it... Have attached the patch with additional logging for CPU too, please review and if its ok will update the title and the type of the jira.

        Show
        Naganarasimha Naganarasimha G R added a comment - Thanks for the comments Junping Du , +1 for logging, i think initially too they might have intended for the same reason and some how missed it... Have attached the patch with additional logging for CPU too, please review and if its ok will update the title and the type of the jira.
        Hide
        hadoopqa Hadoop QA added a comment -



        -1 overall



        Vote Subsystem Runtime Comment
        0 pre-patch 15m 0s Pre-patch trunk compilation is healthy.
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 tests included 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 javac 7m 38s There were no new javac warning messages.
        +1 javadoc 9m 52s There were no new javadoc warning messages.
        +1 release audit 0m 23s The applied patch does not increase the total number of release audit warnings.
        -1 checkstyle 0m 38s The applied patch generated 1 new checkstyle issues (total was 27, now 27).
        +1 whitespace 0m 0s The patch has no lines that end in whitespace.
        +1 install 1m 33s mvn install still works.
        +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse.
        +1 findbugs 1m 3s The patch does not introduce any new Findbugs (version 2.0.3) warnings.
        +1 yarn tests 6m 2s Tests passed in hadoop-yarn-server-nodemanager.
            42m 46s  



        Subsystem Report/Notes
        Patch URL http://issues.apache.org/jira/secure/attachment/12730656/YARN-3513.20150506-1.patch
        Optional Tests javadoc javac unit findbugs checkstyle
        git revision trunk / 90b3845
        checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/7718/artifact/patchprocess/diffcheckstylehadoop-yarn-server-nodemanager.txt
        hadoop-yarn-server-nodemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/7718/artifact/patchprocess/testrun_hadoop-yarn-server-nodemanager.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/7718/testReport/
        Java 1.7.0_55
        uname Linux asf903.gq1.ygridcore.net 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
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/7718/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 15m 0s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. -1 tests included 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 javac 7m 38s There were no new javac warning messages. +1 javadoc 9m 52s There were no new javadoc warning messages. +1 release audit 0m 23s The applied patch does not increase the total number of release audit warnings. -1 checkstyle 0m 38s The applied patch generated 1 new checkstyle issues (total was 27, now 27). +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 33s mvn install still works. +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse. +1 findbugs 1m 3s The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 yarn tests 6m 2s Tests passed in hadoop-yarn-server-nodemanager.     42m 46s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12730656/YARN-3513.20150506-1.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / 90b3845 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/7718/artifact/patchprocess/diffcheckstylehadoop-yarn-server-nodemanager.txt hadoop-yarn-server-nodemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/7718/artifact/patchprocess/testrun_hadoop-yarn-server-nodemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/7718/testReport/ Java 1.7.0_55 uname Linux asf903.gq1.ygridcore.net 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 Console output https://builds.apache.org/job/PreCommit-YARN-Build/7718/console This message was automatically generated.
        Hide
        Naganarasimha Naganarasimha G R added a comment -

        Hi Junping Du,Li Lu & Vinod Kumar Vavilapalli,
        Hope one of you can review my last uploaded patch as per Junping Du's comment.

        Show
        Naganarasimha Naganarasimha G R added a comment - Hi Junping Du , Li Lu & Vinod Kumar Vavilapalli , Hope one of you can review my last uploaded patch as per Junping Du 's comment.
        Hide
        gtCarrera9 Li Lu added a comment -

        Hi Naganarasimha G R, thanks for working on this. After looking at this patch I have a few things to verify:

        1. Are we only referencing the four variables when debug is enabled? If so does it harm to only update those four variables on debug only? I understand it may not be quite "branch-predictor friendly", but maybe in this way we can make the usage of those four variables more clear?
        2. I think we're changing the meaning of vmemStillInUsage and pmemStillInUsage. Would you please elaborate a little bit more the current way makes more sense than the previous way? I think the previous way only accounts for alive containers.
        Show
        gtCarrera9 Li Lu added a comment - Hi Naganarasimha G R , thanks for working on this. After looking at this patch I have a few things to verify: Are we only referencing the four variables when debug is enabled? If so does it harm to only update those four variables on debug only? I understand it may not be quite "branch-predictor friendly", but maybe in this way we can make the usage of those four variables more clear? I think we're changing the meaning of vmemStillInUsage and pmemStillInUsage . Would you please elaborate a little bit more the current way makes more sense than the previous way? I think the previous way only accounts for alive containers.
        Hide
        Naganarasimha Naganarasimha G R added a comment -

        Thanks for reviewing Li Lu

        If so does it harm to only update those four variables on debug only

        +1 will do these modifications which will remove unwanted computations when debug is not enabled

        I think we're changing the meaning of vmemStillInUsage and pmemStillInUsage. Would you please elaborate a little bit more the current way makes more sense than the previous way?

        Well the names may be debatable, but my intention was to actually capture the total usage when the monitors ran, so that it includes the usage of containers which are about to be killled as part of monitor's run. Well can keep it as earlier but felt this will be more useful to get the actual usage... thoughts ?

        Show
        Naganarasimha Naganarasimha G R added a comment - Thanks for reviewing Li Lu If so does it harm to only update those four variables on debug only +1 will do these modifications which will remove unwanted computations when debug is not enabled I think we're changing the meaning of vmemStillInUsage and pmemStillInUsage. Would you please elaborate a little bit more the current way makes more sense than the previous way? Well the names may be debatable, but my intention was to actually capture the total usage when the monitors ran, so that it includes the usage of containers which are about to be killled as part of monitor's run. Well can keep it as earlier but felt this will be more useful to get the actual usage... thoughts ?
        Hide
        gtCarrera9 Li Lu added a comment -

        my intention was to actually capture the total usage when the monitors ran, so that it includes the usage of containers which are about to be killled as part of monitor's run.

        OK, that sounds reasonable to me, but I'd certainly appreciate if any committers can help verify this.

        Show
        gtCarrera9 Li Lu added a comment - my intention was to actually capture the total usage when the monitors ran, so that it includes the usage of containers which are about to be killled as part of monitor's run. OK, that sounds reasonable to me, but I'd certainly appreciate if any committers can help verify this.
        Hide
        Naganarasimha Naganarasimha G R added a comment -

        Attaching a patch incorporating suggestions from Li Lu.

        Show
        Naganarasimha Naganarasimha G R added a comment - Attaching a patch incorporating suggestions from Li Lu .
        Hide
        hadoopqa Hadoop QA added a comment -



        -1 overall



        Vote Subsystem Runtime Comment
        0 pre-patch 14m 45s Pre-patch trunk compilation is healthy.
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 tests included 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 javac 7m 35s There were no new javac warning messages.
        +1 javadoc 9m 32s There were no new javadoc warning messages.
        +1 release audit 0m 23s The applied patch does not increase the total number of release audit warnings.
        -1 checkstyle 0m 35s The applied patch generated 2 new checkstyle issues (total was 27, now 28).
        +1 whitespace 0m 0s The patch has no lines that end in whitespace.
        +1 install 1m 34s mvn install still works.
        +1 eclipse:eclipse 0m 32s The patch built with eclipse:eclipse.
        +1 findbugs 1m 7s The patch does not introduce any new Findbugs (version 2.0.3) warnings.
        +1 yarn tests 6m 4s Tests passed in hadoop-yarn-server-nodemanager.
            42m 10s  



        Subsystem Report/Notes
        Patch URL http://issues.apache.org/jira/secure/attachment/12731066/YARN-3513.20150507-1.patch
        Optional Tests javadoc javac unit findbugs checkstyle
        git revision trunk / 918af8e
        checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/7752/artifact/patchprocess/diffcheckstylehadoop-yarn-server-nodemanager.txt
        hadoop-yarn-server-nodemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/7752/artifact/patchprocess/testrun_hadoop-yarn-server-nodemanager.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/7752/testReport/
        Java 1.7.0_55
        uname Linux asf903.gq1.ygridcore.net 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
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/7752/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 14m 45s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. -1 tests included 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 javac 7m 35s There were no new javac warning messages. +1 javadoc 9m 32s There were no new javadoc warning messages. +1 release audit 0m 23s The applied patch does not increase the total number of release audit warnings. -1 checkstyle 0m 35s The applied patch generated 2 new checkstyle issues (total was 27, now 28). +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 34s mvn install still works. +1 eclipse:eclipse 0m 32s The patch built with eclipse:eclipse. +1 findbugs 1m 7s The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 yarn tests 6m 4s Tests passed in hadoop-yarn-server-nodemanager.     42m 10s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12731066/YARN-3513.20150507-1.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / 918af8e checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/7752/artifact/patchprocess/diffcheckstylehadoop-yarn-server-nodemanager.txt hadoop-yarn-server-nodemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/7752/artifact/patchprocess/testrun_hadoop-yarn-server-nodemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/7752/testReport/ Java 1.7.0_55 uname Linux asf903.gq1.ygridcore.net 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 Console output https://builds.apache.org/job/PreCommit-YARN-Build/7752/console This message was automatically generated.
        Hide
        Naganarasimha Naganarasimha G R added a comment -

        Checkstyle is not completely related to this patch modifications and test cases are not there as modifications are as part of logging.
        Can Junping Du or Vinod Kumar Vavilapalli further review this patch ?

        Show
        Naganarasimha Naganarasimha G R added a comment - Checkstyle is not completely related to this patch modifications and test cases are not there as modifications are as part of logging. Can Junping Du or Vinod Kumar Vavilapalli further review this patch ?
        Hide
        Naganarasimha Naganarasimha G R added a comment -

        Correcting 1 check style issue related to the patch

        Show
        Naganarasimha Naganarasimha G R added a comment - Correcting 1 check style issue related to the patch
        Hide
        Naganarasimha Naganarasimha G R added a comment -

        Oops induced white spaces, uploading new patch correcting !

        Show
        Naganarasimha Naganarasimha G R added a comment - Oops induced white spaces, uploading new patch correcting !
        Hide
        hadoopqa Hadoop QA added a comment -



        -1 overall



        Vote Subsystem Runtime Comment
        0 pre-patch 14m 46s Pre-patch trunk compilation is healthy.
        +1 @author 0m 1s The patch does not contain any @author tags.
        -1 tests included 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 javac 7m 47s There were no new javac warning messages.
        +1 javadoc 9m 49s There were no new javadoc warning messages.
        +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings.
        -1 checkstyle 0m 38s The applied patch generated 1 new checkstyle issues (total was 27, now 27).
        +1 whitespace 0m 0s The patch has no lines that end in whitespace.
        +1 install 1m 35s mvn install still works.
        +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse.
        +1 findbugs 1m 3s The patch does not introduce any new Findbugs (version 2.0.3) warnings.
        +1 yarn tests 6m 11s Tests passed in hadoop-yarn-server-nodemanager.
            42m 49s  



        Subsystem Report/Notes
        Patch URL http://issues.apache.org/jira/secure/attachment/12731425/YARN-3513.20150508-1.patch
        Optional Tests javadoc javac unit findbugs checkstyle
        git revision trunk / 7b1ea9c
        checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/7803/artifact/patchprocess/diffcheckstylehadoop-yarn-server-nodemanager.txt
        hadoop-yarn-server-nodemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/7803/artifact/patchprocess/testrun_hadoop-yarn-server-nodemanager.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/7803/testReport/
        Java 1.7.0_55
        uname Linux asf903.gq1.ygridcore.net 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
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/7803/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 14m 46s Pre-patch trunk compilation is healthy. +1 @author 0m 1s The patch does not contain any @author tags. -1 tests included 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 javac 7m 47s There were no new javac warning messages. +1 javadoc 9m 49s There were no new javadoc warning messages. +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings. -1 checkstyle 0m 38s The applied patch generated 1 new checkstyle issues (total was 27, now 27). +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 35s mvn install still works. +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse. +1 findbugs 1m 3s The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 yarn tests 6m 11s Tests passed in hadoop-yarn-server-nodemanager.     42m 49s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12731425/YARN-3513.20150508-1.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / 7b1ea9c checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/7803/artifact/patchprocess/diffcheckstylehadoop-yarn-server-nodemanager.txt hadoop-yarn-server-nodemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/7803/artifact/patchprocess/testrun_hadoop-yarn-server-nodemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/7803/testReport/ Java 1.7.0_55 uname Linux asf903.gq1.ygridcore.net 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 Console output https://builds.apache.org/job/PreCommit-YARN-Build/7803/console This message was automatically generated.
        Hide
        Naganarasimha Naganarasimha G R added a comment -

        Devaraj K/Junping Du, can any one of you take a look at it now as valid check style and whitespace issues has been fixed.

        Show
        Naganarasimha Naganarasimha G R added a comment - Devaraj K / Junping Du , can any one of you take a look at it now as valid check style and whitespace issues has been fixed.
        Hide
        devaraj.k Devaraj K added a comment -

        Thanks Naganarasimha G R for the patch.

        +            if (LOG.isDebugEnabled()) {
        +              // Accounting the total memory in usage for all containers
        +              vmemUsageByAllContainers += currentVmemUsage;
        +              pmemByAllContainers += currentPmemUsage;
        +              // Accounting the total cpu usage for all containers
        +              cpuUsagePercentPerCoreByAllContainers += cpuUsagePercentPerCore;
        +              cpuUsageTotalCoresByAllContainers += cpuUsagePercentPerCore;
        +            }
        

        Here resource usage calculations happen only when the debug log is enabled, users can change the log level any time using the below command.

        [devaraj@server2 bin]$ ./yarn daemonlog
        
        Usage: General options are:
                [-getlevel <host:httpPort> <classname>]
                [-setlevel <host:httpPort> <classname> <level>]
        

        These values may go wrong when the user changes the log level while Node Manager running.

        Show
        devaraj.k Devaraj K added a comment - Thanks Naganarasimha G R for the patch. + if (LOG.isDebugEnabled()) { + // Accounting the total memory in usage for all containers + vmemUsageByAllContainers += currentVmemUsage; + pmemByAllContainers += currentPmemUsage; + // Accounting the total cpu usage for all containers + cpuUsagePercentPerCoreByAllContainers += cpuUsagePercentPerCore; + cpuUsageTotalCoresByAllContainers += cpuUsagePercentPerCore; + } Here resource usage calculations happen only when the debug log is enabled, users can change the log level any time using the below command. [devaraj@server2 bin]$ ./yarn daemonlog Usage: General options are: [-getlevel <host:httpPort> <classname> ] [-setlevel <host:httpPort> <classname> <level> ] These values may go wrong when the user changes the log level while Node Manager running.
        Hide
        Naganarasimha Naganarasimha G R added a comment -

        Thanks for the comment Devaraj K,
        its a good catch but IMHO, i would like to retain the current approach as

        • even if values go wrong its only for one run of the monitor and doesn't fail anything.
        • change of log level is rare operation
        • better to save unnecessary computations for each run of monitoring than compared to single invalid log.

        Open to remove this check if you still feel necessary

        Show
        Naganarasimha Naganarasimha G R added a comment - Thanks for the comment Devaraj K , its a good catch but IMHO, i would like to retain the current approach as even if values go wrong its only for one run of the monitor and doesn't fail anything. change of log level is rare operation better to save unnecessary computations for each run of monitoring than compared to single invalid log. Open to remove this check if you still feel necessary
        Hide
        devaraj.k Devaraj K added a comment -

        Thanks Naganarasimha G R for the details. I understand your intention to avoid the calculations when the log level is not debug. Here we have variables declared irrespective of the log level and incrementing those variables would not make a big difference compared to the invalid log. IMO, It would be better to have the correct details even if it is a single log.

        Show
        devaraj.k Devaraj K added a comment - Thanks Naganarasimha G R for the details. I understand your intention to avoid the calculations when the log level is not debug. Here we have variables declared irrespective of the log level and incrementing those variables would not make a big difference compared to the invalid log. IMO, It would be better to have the correct details even if it is a single log.
        Hide
        Naganarasimha Naganarasimha G R added a comment -

        Ok Devaraj K, updated the patch as per your suggestion.

        Show
        Naganarasimha Naganarasimha G R added a comment - Ok Devaraj K , updated the patch as per your suggestion.
        Hide
        hadoopqa Hadoop QA added a comment -



        -1 overall



        Vote Subsystem Runtime Comment
        0 pre-patch 14m 33s Pre-patch trunk compilation is healthy.
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 tests included 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 javac 7m 29s There were no new javac warning messages.
        +1 javadoc 9m 32s There were no new javadoc warning messages.
        +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings.
        -1 checkstyle 0m 36s The applied patch generated 1 new checkstyle issues (total was 27, now 27).
        +1 whitespace 0m 0s The patch has no lines that end in whitespace.
        +1 install 1m 38s mvn install still works.
        +1 eclipse:eclipse 0m 32s The patch built with eclipse:eclipse.
        +1 findbugs 1m 1s The patch does not introduce any new Findbugs (version 2.0.3) warnings.
        +1 yarn tests 5m 57s Tests passed in hadoop-yarn-server-nodemanager.
            41m 44s  



        Subsystem Report/Notes
        Patch URL http://issues.apache.org/jira/secure/attachment/12731901/YARN-3513.20150511-1.patch
        Optional Tests javadoc javac unit findbugs checkstyle
        git revision trunk / 3fa2efc
        checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/7861/artifact/patchprocess/diffcheckstylehadoop-yarn-server-nodemanager.txt
        hadoop-yarn-server-nodemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/7861/artifact/patchprocess/testrun_hadoop-yarn-server-nodemanager.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/7861/testReport/
        Java 1.7.0_55
        uname Linux asf906.gq1.ygridcore.net 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
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/7861/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 14m 33s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. -1 tests included 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 javac 7m 29s There were no new javac warning messages. +1 javadoc 9m 32s There were no new javadoc warning messages. +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings. -1 checkstyle 0m 36s The applied patch generated 1 new checkstyle issues (total was 27, now 27). +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 38s mvn install still works. +1 eclipse:eclipse 0m 32s The patch built with eclipse:eclipse. +1 findbugs 1m 1s The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 yarn tests 5m 57s Tests passed in hadoop-yarn-server-nodemanager.     41m 44s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12731901/YARN-3513.20150511-1.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / 3fa2efc checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/7861/artifact/patchprocess/diffcheckstylehadoop-yarn-server-nodemanager.txt hadoop-yarn-server-nodemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/7861/artifact/patchprocess/testrun_hadoop-yarn-server-nodemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/7861/testReport/ Java 1.7.0_55 uname Linux asf906.gq1.ygridcore.net 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 Console output https://builds.apache.org/job/PreCommit-YARN-Build/7861/console This message was automatically generated.
        Hide
        devaraj.k Devaraj K added a comment -

        Thanks Naganarasimha G R for the updated patch.

        +1, Latest patch looks good to me, will commit it shortly.

        Show
        devaraj.k Devaraj K added a comment - Thanks Naganarasimha G R for the updated patch. +1, Latest patch looks good to me, will commit it shortly.
        Hide
        devaraj.k Devaraj K added a comment -

        Committed to trunk and branch-2

        Thanks Naganarasimha G R.

        Show
        devaraj.k Devaraj K added a comment - Committed to trunk and branch-2 Thanks Naganarasimha G R .
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Hadoop-trunk-Commit #7804 (See https://builds.apache.org/job/Hadoop-trunk-Commit/7804/)
        YARN-3513. Remove unused variables in ContainersMonitorImpl and add debug (devaraj: rev 8badd82ce256e4dc8c234961120d62a88358ab39)

        • hadoop-yarn-project/CHANGES.txt
        • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/ContainersMonitorImpl.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-trunk-Commit #7804 (See https://builds.apache.org/job/Hadoop-trunk-Commit/7804/ ) YARN-3513 . Remove unused variables in ContainersMonitorImpl and add debug (devaraj: rev 8badd82ce256e4dc8c234961120d62a88358ab39) hadoop-yarn-project/CHANGES.txt hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/ContainersMonitorImpl.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Mapreduce-trunk #2141 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2141/)
        YARN-3513. Remove unused variables in ContainersMonitorImpl and add debug (devaraj: rev 8badd82ce256e4dc8c234961120d62a88358ab39)

        • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/ContainersMonitorImpl.java
        • hadoop-yarn-project/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk #2141 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2141/ ) YARN-3513 . Remove unused variables in ContainersMonitorImpl and add debug (devaraj: rev 8badd82ce256e4dc8c234961120d62a88358ab39) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/ContainersMonitorImpl.java hadoop-yarn-project/CHANGES.txt
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Hdfs-trunk #2123 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2123/)
        YARN-3513. Remove unused variables in ContainersMonitorImpl and add debug (devaraj: rev 8badd82ce256e4dc8c234961120d62a88358ab39)

        • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/ContainersMonitorImpl.java
        • hadoop-yarn-project/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk #2123 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2123/ ) YARN-3513 . Remove unused variables in ContainersMonitorImpl and add debug (devaraj: rev 8badd82ce256e4dc8c234961120d62a88358ab39) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/ContainersMonitorImpl.java hadoop-yarn-project/CHANGES.txt
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #183 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/183/)
        YARN-3513. Remove unused variables in ContainersMonitorImpl and add debug (devaraj: rev 8badd82ce256e4dc8c234961120d62a88358ab39)

        • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/ContainersMonitorImpl.java
        • hadoop-yarn-project/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #183 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/183/ ) YARN-3513 . Remove unused variables in ContainersMonitorImpl and add debug (devaraj: rev 8badd82ce256e4dc8c234961120d62a88358ab39) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/ContainersMonitorImpl.java hadoop-yarn-project/CHANGES.txt
        Hide
        Naganarasimha Naganarasimha G R added a comment -

        Thanks for reviewing and committing the patch Devaraj K, Li Lu & Junping Du

        Show
        Naganarasimha Naganarasimha G R added a comment - Thanks for reviewing and committing the patch Devaraj K , Li Lu & Junping Du
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Hadoop-Mapreduce-trunk-Java8 #193 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/193/)
        YARN-3513. Remove unused variables in ContainersMonitorImpl and add debug (devaraj: rev 8badd82ce256e4dc8c234961120d62a88358ab39)

        • hadoop-yarn-project/CHANGES.txt
        • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/ContainersMonitorImpl.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Mapreduce-trunk-Java8 #193 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/193/ ) YARN-3513 . Remove unused variables in ContainersMonitorImpl and add debug (devaraj: rev 8badd82ce256e4dc8c234961120d62a88358ab39) hadoop-yarn-project/CHANGES.txt hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/ContainersMonitorImpl.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #195 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/195/)
        YARN-3513. Remove unused variables in ContainersMonitorImpl and add debug (devaraj: rev 8badd82ce256e4dc8c234961120d62a88358ab39)

        • hadoop-yarn-project/CHANGES.txt
        • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/ContainersMonitorImpl.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #195 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/195/ ) YARN-3513 . Remove unused variables in ContainersMonitorImpl and add debug (devaraj: rev 8badd82ce256e4dc8c234961120d62a88358ab39) hadoop-yarn-project/CHANGES.txt hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/ContainersMonitorImpl.java
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Hadoop-Yarn-trunk #926 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/926/)
        YARN-3513. Remove unused variables in ContainersMonitorImpl and add debug (devaraj: rev 8badd82ce256e4dc8c234961120d62a88358ab39)

        • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/ContainersMonitorImpl.java
        • hadoop-yarn-project/CHANGES.txt
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Yarn-trunk #926 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/926/ ) YARN-3513 . Remove unused variables in ContainersMonitorImpl and add debug (devaraj: rev 8badd82ce256e4dc8c234961120d62a88358ab39) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/ContainersMonitorImpl.java hadoop-yarn-project/CHANGES.txt

          People

          • Assignee:
            Naganarasimha Naganarasimha G R
            Reporter:
            Naganarasimha Naganarasimha G R
          • Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development