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

Fix warnings from Spotbugs in hadoop-yarn-server-nodemanager

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.0.0-beta1
    • Component/s: nodemanager
    • Labels:
      None

      Description

      5 find bugs issue was reported NM project as part of the YARN-4166 build
      Issue 1:
      org.apache.hadoop.yarn.server.nodemanager.containermanager.monitor.ContainerMetrics.usageMetrics is a mutable collection which should be package protected
      Bug type MS_MUTABLE_COLLECTION_PKGPROTECT (click for details)
      In class org.apache.hadoop.yarn.server.nodemanager.containermanager.monitor.ContainerMetrics
      Field org.apache.hadoop.yarn.server.nodemanager.containermanager.monitor.ContainerMetrics.usageMetrics
      At ContainerMetrics.java:[line 134]

      Issue 2:
      org.apache.hadoop.yarn.server.nodemanager.containermanager.localizer.ContainerLocalizer.createStatus() makes inefficient use of keySet iterator instead of entrySet iterator
      Bug type WMI_WRONG_MAP_ITERATOR (click for details)
      In class org.apache.hadoop.yarn.server.nodemanager.containermanager.localizer.ContainerLocalizer
      In method org.apache.hadoop.yarn.server.nodemanager.containermanager.localizer.ContainerLocalizer.createStatus()
      Field org.apache.hadoop.yarn.server.nodemanager.containermanager.localizer.ContainerLocalizer.pendingResources
      At ContainerLocalizer.java:[line 334]

      Issue 3:
      org.apache.hadoop.yarn.server.nodemanager.NodeStatusUpdaterImpl.removeVeryOldStoppedContainersFromCache() makes inefficient use of keySet iterator instead of entrySet iterator
      Bug type WMI_WRONG_MAP_ITERATOR (click for details)
      In class org.apache.hadoop.yarn.server.nodemanager.NodeStatusUpdaterImpl
      In method org.apache.hadoop.yarn.server.nodemanager.NodeStatusUpdaterImpl.removeVeryOldStoppedContainersFromCache()
      Field org.apache.hadoop.yarn.server.nodemanager.NodeStatusUpdaterImpl.recentlyStoppedContainers
      At NodeStatusUpdaterImpl.java:[line 721]

      Issue 4:
      Hard coded reference to an absolute pathname in org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.runtime.DockerLinuxContainerRuntime.launchContainer(ContainerRuntimeContext)
      Bug type DMI_HARDCODED_ABSOLUTE_FILENAME (click for details)
      In class org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.runtime.DockerLinuxContainerRuntime
      In method org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.runtime.DockerLinuxContainerRuntime.launchContainer(ContainerRuntimeContext)
      File name /sys/fs/cgroup
      At DockerLinuxContainerRuntime.java:[line 455]

      Useless object stored in variable removedNullContainers of method org.apache.hadoop.yarn.server.nodemanager.NodeStatusUpdaterImpl.removeOrTrackCompletedContainersFromContext(List)
      Bug type UC_USELESS_OBJECT (click for details)
      In class org.apache.hadoop.yarn.server.nodemanager.NodeStatusUpdaterImpl
      In method org.apache.hadoop.yarn.server.nodemanager.NodeStatusUpdaterImpl.removeOrTrackCompletedContainersFromContext(List)
      Value removedNullContainers
      Type java.util.HashSet
      At NodeStatusUpdaterImpl.java:[line 644]

      1. YARN-6515.001.patch
        12 kB
        Naganarasimha G R
      2. YARN-6515.002.patch
        6 kB
        Naganarasimha G R

        Issue Links

          Activity

          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks for the review and commit Akira Ajisaka and reviews from Weiwei Yang, Miklos Szegedi, Shane Kumpf

          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks for the review and commit Akira Ajisaka and reviews from Weiwei Yang , Miklos Szegedi , Shane Kumpf
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #12152 (See https://builds.apache.org/job/Hadoop-trunk-Commit/12152/)
          YARN-6515. Fix warnings from Spotbugs in hadoop-yarn-server-nodemanager. (aajisaka: rev 1a18d5e514d13aa3a88e9b6089394a27296d6bc3)

          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/ContainerMetrics.java
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/ContainerLocalizer.java
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeStatusUpdaterImpl.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #12152 (See https://builds.apache.org/job/Hadoop-trunk-Commit/12152/ ) YARN-6515 . Fix warnings from Spotbugs in hadoop-yarn-server-nodemanager. (aajisaka: rev 1a18d5e514d13aa3a88e9b6089394a27296d6bc3) (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/ContainerMetrics.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/ContainerLocalizer.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeStatusUpdaterImpl.java
          Hide
          ajisakaa Akira Ajisaka added a comment -

          Committed this to trunk. Thanks all who contributed to this issue.

          Show
          ajisakaa Akira Ajisaka added a comment - Committed this to trunk. Thanks all who contributed to this issue.
          Hide
          miklos.szegedi@cloudera.com Miklos Szegedi added a comment -

          Thanks Naganarasimha G R, I created YARN-6968 for the Docker issue.

          Show
          miklos.szegedi@cloudera.com Miklos Szegedi added a comment - Thanks Naganarasimha G R , I created YARN-6968 for the Docker issue.
          Hide
          shanekumpf@gmail.com Shane Kumpf added a comment -

          Thanks Naganarasimha G R for the updated patch. With regard to the hard coded cgroup path in DLCR, I am fine with leaving it off this patch. The hard coded mount (and path) will be removed after YARN-5534 is in. The rest looks good to me, +1.

          Show
          shanekumpf@gmail.com Shane Kumpf added a comment - Thanks Naganarasimha G R for the updated patch. With regard to the hard coded cgroup path in DLCR, I am fine with leaving it off this patch. The hard coded mount (and path) will be removed after YARN-5534 is in. The rest looks good to me, +1.
          Hide
          ajisakaa Akira Ajisaka added a comment -

          Thanks Naganarasimha G R for the comment and the patch. +1.

          Show
          ajisakaa Akira Ajisaka added a comment - Thanks Naganarasimha G R for the comment and the patch. +1.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 18s Docker mode activated.
                Prechecks
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
                trunk Compile Tests
          +1 mvninstall 17m 18s trunk passed
          +1 compile 0m 35s trunk passed
          +1 checkstyle 0m 24s trunk passed
          +1 mvnsite 0m 35s trunk passed
          -1 findbugs 0m 57s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager in trunk has 5 extant Findbugs warnings.
          +1 javadoc 0m 22s trunk passed
                Patch Compile Tests
          +1 mvninstall 0m 27s the patch passed
          +1 compile 0m 29s the patch passed
          +1 javac 0m 29s the patch passed
          +1 checkstyle 0m 16s the patch passed
          +1 mvnsite 0m 26s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 54s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager generated 0 new + 1 unchanged - 4 fixed = 1 total (was 5)
          +1 javadoc 0m 15s the patch passed
                Other Tests
          +1 unit 13m 6s hadoop-yarn-server-nodemanager in the patch passed.
          +1 asflicense 0m 15s The patch does not generate ASF License warnings.
          37m 56s



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

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 18s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.       trunk Compile Tests +1 mvninstall 17m 18s trunk passed +1 compile 0m 35s trunk passed +1 checkstyle 0m 24s trunk passed +1 mvnsite 0m 35s trunk passed -1 findbugs 0m 57s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager in trunk has 5 extant Findbugs warnings. +1 javadoc 0m 22s trunk passed       Patch Compile Tests +1 mvninstall 0m 27s the patch passed +1 compile 0m 29s the patch passed +1 javac 0m 29s the patch passed +1 checkstyle 0m 16s the patch passed +1 mvnsite 0m 26s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 54s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager generated 0 new + 1 unchanged - 4 fixed = 1 total (was 5) +1 javadoc 0m 15s the patch passed       Other Tests +1 unit 13m 6s hadoop-yarn-server-nodemanager in the patch passed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 37m 56s Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue YARN-6515 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12880569/YARN-6515.002.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux d490047816fa 3.13.0-116-generic #163-Ubuntu SMP Fri Mar 31 14:13:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 024c3ec Default Java 1.8.0_131 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-YARN-Build/16727/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager-warnings.html Test Results https://builds.apache.org/job/PreCommit-YARN-Build/16727/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/16727/console Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Sorry for keeping this jira pending since a long time, was not sure whether to go ahead with the fix for the find bug report in DockerLinuxContainerRuntime. To proceed for other issues i have uploaded a patch excluding it.
          Miklos Szegedi, would you mind handling the findbug issue in DockerLinuxContainerRuntime in YARN-6757 or new jira?
          cc / Akira Ajisaka

          Show
          Naganarasimha Naganarasimha G R added a comment - Sorry for keeping this jira pending since a long time, was not sure whether to go ahead with the fix for the find bug report in DockerLinuxContainerRuntime. To proceed for other issues i have uploaded a patch excluding it. Miklos Szegedi , would you mind handling the findbug issue in DockerLinuxContainerRuntime in YARN-6757 or new jira? cc / Akira Ajisaka
          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-6515 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help.



          Subsystem Report/Notes
          JIRA Issue YARN-6515
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12864692/YARN-6515.001.patch
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/16665/console
          Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s Docker mode activated. -1 patch 0m 5s YARN-6515 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help. Subsystem Report/Notes JIRA Issue YARN-6515 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12864692/YARN-6515.001.patch Console output https://builds.apache.org/job/PreCommit-YARN-Build/16665/console Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          miklos.szegedi@cloudera.com Miklos Szegedi added a comment -

          Thank you for the response Shane Kumpf. In this case I think I would also add this as a comment to the mount code, if we keep it for now.

          Show
          miklos.szegedi@cloudera.com Miklos Szegedi added a comment - Thank you for the response Shane Kumpf . In this case I think I would also add this as a comment to the mount code, if we keep it for now.
          Hide
          shanekumpf@gmail.com Shane Kumpf added a comment -

          what is the point overriding the cgroups directory mounted by Docker in the worker containers?

          Thanks for bringing that up Miklos Szegedi. At the time, I believe it was required to allow systemd to run inside the containers. That overlap is confusing and likely not the behavior wanted/anticipated.

          Show
          shanekumpf@gmail.com Shane Kumpf added a comment - what is the point overriding the cgroups directory mounted by Docker in the worker containers? Thanks for bringing that up Miklos Szegedi . At the time, I believe it was required to allow systemd to run inside the containers. That overlap is confusing and likely not the behavior wanted/anticipated.
          Hide
          miklos.szegedi@cloudera.com Miklos Szegedi added a comment -

          Thank you, Shane Kumpf for the explanation.

          Also, the location inside the container might still need to be hard coded or configured by a separate entry to support isolation better.

          Naganarasimha Garla, yes I am talking about the Docker container here.
          A little bit off topic but I am now curious, what is the point overriding the cgroups directory mounted by Docker in the worker containers?
          If I create a centos6.7 Docker container on centos7, I see overlapping IDs in the two cgroups folders, that does no good I think and it will cause confusion in the future:
          host:

          [root@miklos-dev2 ~]# cat /sys/fs/cgroup/cpu/tasks
          1
          2
          3
          5
          7
          8
          9
          ...
          26938
          root         1     0  0 09:55 ?        00:00:03 /usr/lib/systemd/systemd --system --deserialize 22
          

          container:

          / # cat /sys/fs/cgroup/cpu/tasks 
          1
          9
              1 root       0:00 sh
          
          Show
          miklos.szegedi@cloudera.com Miklos Szegedi added a comment - Thank you, Shane Kumpf for the explanation. Also, the location inside the container might still need to be hard coded or configured by a separate entry to support isolation better. Naganarasimha Garla , yes I am talking about the Docker container here. A little bit off topic but I am now curious, what is the point overriding the cgroups directory mounted by Docker in the worker containers? If I create a centos6.7 Docker container on centos7, I see overlapping IDs in the two cgroups folders, that does no good I think and it will cause confusion in the future: host: [root@miklos-dev2 ~]# cat /sys/fs/cgroup/cpu/tasks 1 2 3 5 7 8 9 ... 26938 root 1 0 0 09:55 ? 00:00:03 /usr/lib/systemd/systemd --system --deserialize 22 container: / # cat /sys/fs/cgroup/cpu/tasks 1 9 1 root 0:00 sh
          Hide
          shanekumpf@gmail.com Shane Kumpf added a comment -

          Thank you Naganarasimha G R and Akira Ajisaka for bringing up the cgroup mount path hardcode issue.

          I agree with Miklos Szegedi that using YarnConfiguration.NM_LINUX_CONTAINER_CGROUPS_MOUNT_PATH will require a documentation update due to the explicit wording of when it used. However, the setting does seem to be appropriate for this use and I think introducing a new configuration would be superfluous.

          It would be good to remove the CGROUPS_ROOT_DIRECTORY variable all together and add a default value to YarnConfiguration ("/sys/fs/cgroup" to maintain backward compatibility). This will require an update to TestDockerContainerRuntime as well.

          Also, the location inside the container might still need to be hard coded or configured by a separate entry to support isolation better.

          IMO, I don't think we should change behavior here and allow the mount point in the container to differ from the source (i.e. /sys/fs/cgroup should be bind mounted at /sys/fs/cgroup in the container, as we do today). I believe we can remove this automatic cgroup mount and address the need for different src/dest paths once YARN-5534 is in.

          Show
          shanekumpf@gmail.com Shane Kumpf added a comment - Thank you Naganarasimha G R and Akira Ajisaka for bringing up the cgroup mount path hardcode issue. I agree with Miklos Szegedi that using YarnConfiguration.NM_LINUX_CONTAINER_CGROUPS_MOUNT_PATH will require a documentation update due to the explicit wording of when it used. However, the setting does seem to be appropriate for this use and I think introducing a new configuration would be superfluous. It would be good to remove the CGROUPS_ROOT_DIRECTORY variable all together and add a default value to YarnConfiguration ("/sys/fs/cgroup" to maintain backward compatibility). This will require an update to TestDockerContainerRuntime as well. Also, the location inside the container might still need to be hard coded or configured by a separate entry to support isolation better. IMO, I don't think we should change behavior here and allow the mount point in the container to differ from the source (i.e. /sys/fs/cgroup should be bind mounted at /sys/fs/cgroup in the container, as we do today). I believe we can remove this automatic cgroup mount and address the need for different src/dest paths once YARN-5534 is in.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks Miklos Szegedi for pitching in,
          Agree may be my fix is not correct and requires update in multiple places or even cause change in behavior. IIUC it is not correct to use hard coded path in DockerLinuxContainerRuntime, but not sure will it cause any impact.

          Also, the location inside the container might still need to be hard coded or configured by a separate entry to support isolation better.

          you mean docker container here right ? May be i am not completely aware of how this "CGROUPS_ROOT_DIRECTORY" is used by the docker container. So please suggest should we need to handle as a separate configuration entry ?

          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks Miklos Szegedi for pitching in, Agree may be my fix is not correct and requires update in multiple places or even cause change in behavior. IIUC it is not correct to use hard coded path in DockerLinuxContainerRuntime , but not sure will it cause any impact. Also, the location inside the container might still need to be hard coded or configured by a separate entry to support isolation better. you mean docker container here right ? May be i am not completely aware of how this "CGROUPS_ROOT_DIRECTORY" is used by the docker container. So please suggest should we need to handle as a separate configuration entry ?
          Hide
          miklos.szegedi@cloudera.com Miklos Szegedi added a comment -

          Thank you, Akira Ajisaka for raising this issue. Naganarasimha G R, if we do this cgroups change, I think we need to update the documentation in NodemanagerCgroups.md. So far this configuration was used just in the context of yarn.nodemanager.linux-container-executor.cgroups.mount=true. yarn-default.xml is another candidate. Also, the location inside the container might still need to be hard coded or configured by a separate entry to support isolation better.

          Show
          miklos.szegedi@cloudera.com Miklos Szegedi added a comment - Thank you, Akira Ajisaka for raising this issue. Naganarasimha G R , if we do this cgroups change, I think we need to update the documentation in NodemanagerCgroups.md. So far this configuration was used just in the context of yarn.nodemanager.linux-container-executor.cgroups.mount=true. yarn-default.xml is another candidate. Also, the location inside the container might still need to be hard coded or configured by a separate entry to support isolation better.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks Akira Ajisaka,
          but the point which is made by the find bugs is valid and we should not go by fixed cgroups mount folder which is currently being used. Wanted some clarification from the owners of DockerLinuxContainerRuntime. cc/ Sidharta Seethana, hope to get some clarification on this

          One alternative is to open separate ticket for the same, but anyways would like to wait till he responds.

          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks Akira Ajisaka , but the point which is made by the find bugs is valid and we should not go by fixed cgroups mount folder which is currently being used. Wanted some clarification from the owners of DockerLinuxContainerRuntime . cc/ Sidharta Seethana , hope to get some clarification on this One alternative is to open separate ticket for the same, but anyways would like to wait till he responds.
          Hide
          ajisakaa Akira Ajisaka added a comment -

          Mostly looks good to me.

          +    cgroupsRootDir =
          +        conf.get(YarnConfiguration.NM_LINUX_CONTAINER_CGROUPS_MOUNT_PATH,
          +            CGROUPS_ROOT_DIRECTORY);
          
          -        .addMountLocation(CGROUPS_ROOT_DIRECTORY,
          -            CGROUPS_ROOT_DIRECTORY + ":ro", false);
          +        .addMountLocation(cgroupsRootDir,
          +            cgroupsRootDir + ":ro", false);
          

          I'm thinking this patch changes the existing behavior. What do you think?

          Show
          ajisakaa Akira Ajisaka added a comment - Mostly looks good to me. + cgroupsRootDir = + conf.get(YarnConfiguration.NM_LINUX_CONTAINER_CGROUPS_MOUNT_PATH, + CGROUPS_ROOT_DIRECTORY); - .addMountLocation(CGROUPS_ROOT_DIRECTORY, - CGROUPS_ROOT_DIRECTORY + ":ro" , false ); + .addMountLocation(cgroupsRootDir, + cgroupsRootDir + ":ro" , false ); I'm thinking this patch changes the existing behavior. What do you think?
          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.
          +1 mvninstall 11m 59s trunk passed
          +1 compile 0m 27s trunk passed
          +1 checkstyle 0m 15s trunk passed
          +1 mvnsite 0m 25s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          -1 findbugs 0m 43s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager in trunk has 5 extant Findbugs warnings.
          +1 javadoc 0m 16s trunk passed
          +1 mvninstall 0m 23s the patch passed
          +1 compile 0m 24s the patch passed
          +1 javac 0m 24s the patch passed
          +1 checkstyle 0m 13s the patch passed
          +1 mvnsite 0m 22s the patch passed
          +1 mvneclipse 0m 13s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 45s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager generated 0 new + 0 unchanged - 5 fixed = 0 total (was 5)
          +1 javadoc 0m 14s the patch passed
          +1 unit 12m 40s hadoop-yarn-server-nodemanager in the patch passed.
          +1 asflicense 0m 15s The patch does not generate ASF License warnings.
          31m 22s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ac17dc
          JIRA Issue YARN-6515
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12864692/YARN-6515.001.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 84b0eef219f9 4.4.0-43-generic #63-Ubuntu SMP Wed Oct 12 13:48:03 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 93fa48f
          Default Java 1.8.0_121
          findbugs v3.1.0-RC1
          findbugs https://builds.apache.org/job/PreCommit-YARN-Build/15743/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager-warnings.html
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/15743/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/15743/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 21s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 mvninstall 11m 59s trunk passed +1 compile 0m 27s trunk passed +1 checkstyle 0m 15s trunk passed +1 mvnsite 0m 25s trunk passed +1 mvneclipse 0m 13s trunk passed -1 findbugs 0m 43s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager in trunk has 5 extant Findbugs warnings. +1 javadoc 0m 16s trunk passed +1 mvninstall 0m 23s the patch passed +1 compile 0m 24s the patch passed +1 javac 0m 24s the patch passed +1 checkstyle 0m 13s the patch passed +1 mvnsite 0m 22s the patch passed +1 mvneclipse 0m 13s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 45s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager generated 0 new + 0 unchanged - 5 fixed = 0 total (was 5) +1 javadoc 0m 14s the patch passed +1 unit 12m 40s hadoop-yarn-server-nodemanager in the patch passed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 31m 22s Subsystem Report/Notes Docker Image:yetus/hadoop:0ac17dc JIRA Issue YARN-6515 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12864692/YARN-6515.001.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 84b0eef219f9 4.4.0-43-generic #63-Ubuntu SMP Wed Oct 12 13:48:03 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 93fa48f Default Java 1.8.0_121 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-YARN-Build/15743/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager-warnings.html Test Results https://builds.apache.org/job/PreCommit-YARN-Build/15743/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/15743/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          cheersyang Weiwei Yang added a comment -

          Revised the title a bit for consistency

          Show
          cheersyang Weiwei Yang added a comment - Revised the title a bit for consistency
          Hide
          cheersyang Weiwei Yang added a comment -

          Thanks Naganarasimha G R and Akira Ajisaka, I am going to create several JIRAs per yarn component (like this one), then split the patch in HADOOP-14338 and attach to each of them. Thank you all for the comments.

          Show
          cheersyang Weiwei Yang added a comment - Thanks Naganarasimha G R and Akira Ajisaka , I am going to create several JIRAs per yarn component (like this one), then split the patch in HADOOP-14338 and attach to each of them. Thank you all for the comments.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          As discussed with Weiwei Yang and Akira Ajisaka, we will split YARN find bugs issue into multiple per project yarn jiras. As this would be easier in tracking and discussion.
          Weiwei Yang, please raise and upload patches for other YARN and MR projects.

          Show
          Naganarasimha Naganarasimha G R added a comment - As discussed with Weiwei Yang and Akira Ajisaka , we will split YARN find bugs issue into multiple per project yarn jiras. As this would be easier in tracking and discussion. Weiwei Yang , please raise and upload patches for other YARN and MR projects.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks for the confirmation Akira Ajisaka, will updated it as related to HADOOP-14336 .

          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks for the confirmation Akira Ajisaka , will updated it as related to HADOOP-14336 .
          Hide
          ajisakaa Akira Ajisaka added a comment -

          These find bugs jira should have been under YARN jira, so IIUC we can not make a YARN jira a sub task under HADOOP Jira. If possible i am fine with it, else we can make this jira related to HADOOP-14336 .

          I prefer making this type of jiras to be in YARN project and related to HADOOP-14336. Thanks.

          Show
          ajisakaa Akira Ajisaka added a comment - These find bugs jira should have been under YARN jira, so IIUC we can not make a YARN jira a sub task under HADOOP Jira. If possible i am fine with it, else we can make this jira related to HADOOP-14336 . I prefer making this type of jiras to be in YARN project and related to HADOOP-14336 . Thanks.
          Hide
          cheersyang Weiwei Yang added a comment -

          Hi Naganarasimha G R

          Can you reply the email sent by Akira Ajisaka in which he mentioned the umbrella jiar? I am OK with either way, as long as there is a place for me to attach my patches .

          Show
          cheersyang Weiwei Yang added a comment - Hi Naganarasimha G R Can you reply the email sent by Akira Ajisaka in which he mentioned the umbrella jiar? I am OK with either way, as long as there is a place for me to attach my patches .
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Hi Weiwei Yang,

          I have a patch for all components, but I can split them up to fix rest of yarn components if you like

          Agree you can split and hope you dont mind as i have already raised and fixed for NM, As I was mentioning earlier its better to fix per project, as the reports are generated per project.

          I personally have no problem with that. But since there is an umbrella JIRA, could you please move this one under that?

          These find bugs jira should have been under YARN jira, so IIUC we can not make a YARN jira a sub task under HADOOP Jira. If possible i am fine with it, else we can make this jira related to HADOOP-14336 . Thoughts ?

          Show
          Naganarasimha Naganarasimha G R added a comment - Hi Weiwei Yang , I have a patch for all components, but I can split them up to fix rest of yarn components if you like Agree you can split and hope you dont mind as i have already raised and fixed for NM, As I was mentioning earlier its better to fix per project, as the reports are generated per project. I personally have no problem with that. But since there is an umbrella JIRA, could you please move this one under that? These find bugs jira should have been under YARN jira, so IIUC we can not make a YARN jira a sub task under HADOOP Jira. If possible i am fine with it, else we can make this jira related to HADOOP-14336 . Thoughts ?
          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 doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
          +1 mvninstall 14m 24s trunk passed
          +1 compile 0m 30s trunk passed
          +1 checkstyle 0m 19s trunk passed
          +1 mvnsite 0m 29s trunk passed
          +1 mvneclipse 0m 21s trunk passed
          -1 findbugs 0m 47s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager in trunk has 5 extant Findbugs warnings.
          +1 javadoc 0m 19s trunk passed
          +1 mvninstall 0m 26s the patch passed
          +1 compile 0m 27s the patch passed
          +1 javac 0m 27s the patch passed
          +1 checkstyle 0m 16s the patch passed
          +1 mvnsite 0m 25s the patch passed
          +1 mvneclipse 0m 13s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 46s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager generated 0 new + 0 unchanged - 5 fixed = 0 total (was 5)
          +1 javadoc 0m 15s the patch passed
          +1 unit 13m 2s hadoop-yarn-server-nodemanager in the patch passed.
          +1 asflicense 0m 17s The patch does not generate ASF License warnings.
          35m 0s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ac17dc
          JIRA Issue YARN-6515
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12864692/YARN-6515.001.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 116325ec8848 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / fda86ef
          Default Java 1.8.0_121
          findbugs v3.1.0-RC1
          findbugs https://builds.apache.org/job/PreCommit-YARN-Build/15717/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager-warnings.html
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/15717/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/15717/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 18s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 mvninstall 14m 24s trunk passed +1 compile 0m 30s trunk passed +1 checkstyle 0m 19s trunk passed +1 mvnsite 0m 29s trunk passed +1 mvneclipse 0m 21s trunk passed -1 findbugs 0m 47s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager in trunk has 5 extant Findbugs warnings. +1 javadoc 0m 19s trunk passed +1 mvninstall 0m 26s the patch passed +1 compile 0m 27s the patch passed +1 javac 0m 27s the patch passed +1 checkstyle 0m 16s the patch passed +1 mvnsite 0m 25s the patch passed +1 mvneclipse 0m 13s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 46s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager generated 0 new + 0 unchanged - 5 fixed = 0 total (was 5) +1 javadoc 0m 15s the patch passed +1 unit 13m 2s hadoop-yarn-server-nodemanager in the patch passed. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 35m 0s Subsystem Report/Notes Docker Image:yetus/hadoop:0ac17dc JIRA Issue YARN-6515 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12864692/YARN-6515.001.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 116325ec8848 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / fda86ef Default Java 1.8.0_121 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-YARN-Build/15717/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager-warnings.html Test Results https://builds.apache.org/job/PreCommit-YARN-Build/15717/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/15717/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          cheersyang Weiwei Yang added a comment -

          Hi Naganarasimha G R

          I personally have no problem with that. But since there is an umbrella JIRA, could you please move this one under that? I have a patch for all components, but I can split them up to fix rest of yarn components if you like. Thank you for the quick response.

          Show
          cheersyang Weiwei Yang added a comment - Hi Naganarasimha G R I personally have no problem with that. But since there is an umbrella JIRA, could you please move this one under that? I have a patch for all components, but I can split them up to fix rest of yarn components if you like. Thank you for the quick response.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Hi Weiwei Yang,
          IMO i would suggest to split based on project so that its simpler to handle (and refer when there is findbugs failures elsewhere) and also it should have been YARN jira instead of HADOOP jira and may be we could have linked it under the umbrella HADOOP-14336 . Thoughts ?

          Show
          Naganarasimha Naganarasimha G R added a comment - Hi Weiwei Yang , IMO i would suggest to split based on project so that its simpler to handle (and refer when there is findbugs failures elsewhere) and also it should have been YARN jira instead of HADOOP jira and may be we could have linked it under the umbrella HADOOP-14336 . Thoughts ?
          Hide
          cheersyang Weiwei Yang added a comment -

          Hi Naganarasimha G R

          There is an umbrella jira HADOOP-14336 to fix all findbugs warnings since switched to spotbugs, I was working on the one for yarn HADOOP-14338. There were total 18 warnings in yarn components. Do you want to track them all in HADOOP-14338 or split them for each component like this one?

          Show
          cheersyang Weiwei Yang added a comment - Hi Naganarasimha G R There is an umbrella jira HADOOP-14336 to fix all findbugs warnings since switched to spotbugs, I was working on the one for yarn HADOOP-14338 . There were total 18 warnings in yarn components. Do you want to track them all in HADOOP-14338 or split them for each component like this one?
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Attaching a patch with the fixes.
          Issue #3 : Not sure why DockerContainerImpl is using hard coded cgroup mount path, hope Sidharta Seethana can have a look at it.
          Issue #5 : Seems to be introduced in YARN-4597, Arun Suresh can you take a look at it, though IMO it can be safely removed.

          Show
          Naganarasimha Naganarasimha G R added a comment - Attaching a patch with the fixes. Issue #3 : Not sure why DockerContainerImpl is using hard coded cgroup mount path, hope Sidharta Seethana can have a look at it. Issue #5 : Seems to be introduced in YARN-4597 , Arun Suresh can you take a look at it, though IMO it can be safely removed.

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development