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

Nodemanager container crash after ext3 folder limit

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.9.0, 3.0.0-beta1, 2.8.2
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Configure umask as 027 for nodemanager service user
      and yarn.nodemanager.local-cache.max-files-per-directory as 40. After 4 private dir localization next directory will be 0/14

      Local Directory cache manager

      vm2:/opt/hadoop/release/data/nmlocal/usercache/mapred/filecache # l
      total 28
      drwx--x--- 7 mapred hadoop 4096 Jun 10 14:35 ./
      drwxr-s--- 4 mapred hadoop 4096 Jun 10 12:07 ../
      drwxr-x--- 3 mapred users  4096 Jun 10 14:36 0/
      drwxr-xr-x 3 mapred users  4096 Jun 10 12:15 10/
      drwxr-xr-x 3 mapred users  4096 Jun 10 12:22 11/
      drwxr-xr-x 3 mapred users  4096 Jun 10 12:27 12/
      drwxr-xr-x 3 mapred users  4096 Jun 10 12:31 13/
      
      

      drwxr-x--- 3 mapred users 4096 Jun 10 14:36 0/ is only 750
      Nodemanager user will not be able check for localization path exists or not.

      LocalResourcesTrackerImpl

          case REQUEST:
            if (rsrc != null && (!isResourcePresent(rsrc))) {
              LOG.info("Resource " + rsrc.getLocalPath()
                  + " is missing, localizing it again");
              removeResource(req);
              rsrc = null;
            }
            if (null == rsrc) {
              rsrc = new LocalizedResource(req, dispatcher);
              localrsrc.put(req, rsrc);
            }
            break;
      

      isResourcePresent will always return false and same resource will be localized to 0 to next unique number

      1. YARN-6708.007.patch
        9 kB
        Bibin A Chundatt
      2. YARN-6708.006.patch
        8 kB
        Bibin A Chundatt
      3. YARN-6708.005.patch
        8 kB
        Bibin A Chundatt
      4. YARN-6708.004.patch
        13 kB
        Bibin A Chundatt
      5. YARN-6708.003.patch
        15 kB
        Bibin A Chundatt
      6. YARN-6708.002.patch
        9 kB
        Bibin A Chundatt
      7. YARN-6708.001.patch
        9 kB
        Bibin A Chundatt

        Activity

        Hide
        bibinchundatt Bibin A Chundatt added a comment - - edited

        FS download takes cares for setting permission for localization directory

          Callable<Path> download(Path path, LocalResource rsrc,
              UserGroupInformation ugi) throws IOException {
            diskValidator.checkStatus(new File(path.toUri().getRawPath()));
            return new FSDownloadWrapper(lfs, ugi, conf, path, rsrc);
          }
        
        

        FSDownload#call createDir(destDirPath, cachePerms); only for 0/14 so for 0 rights are not set

         @Override
          public Path call() throws Exception {
            final Path sCopy;
            try {
              sCopy = resource.getResource().toPath();
            } catch (URISyntaxException e) {
              throw new IOException("Invalid resource", e);
            }
            createDir(destDirPath, cachePerms);
        

        + Varun Saxena

        Show
        bibinchundatt Bibin A Chundatt added a comment - - edited FS download takes cares for setting permission for localization directory Callable<Path> download(Path path, LocalResource rsrc, UserGroupInformation ugi) throws IOException { diskValidator.checkStatus( new File(path.toUri().getRawPath())); return new FSDownloadWrapper(lfs, ugi, conf, path, rsrc); } FSDownload#call createDir(destDirPath, cachePerms); only for 0/14 so for 0 rights are not set @Override public Path call() throws Exception { final Path sCopy; try { sCopy = resource.getResource().toPath(); } catch (URISyntaxException e) { throw new IOException( "Invalid resource" , e); } createDir(destDirPath, cachePerms); + Varun Saxena
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 16s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
        0 mvndep 0m 10s Maven dependency ordering for branch
        +1 mvninstall 15m 26s trunk passed
        +1 compile 9m 35s trunk passed
        +1 checkstyle 0m 53s trunk passed
        +1 mvnsite 1m 12s trunk passed
        -1 findbugs 0m 50s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager in trunk has 5 extant Findbugs warnings.
        +1 javadoc 1m 2s trunk passed
        0 mvndep 0m 11s Maven dependency ordering for patch
        +1 mvninstall 1m 0s the patch passed
        +1 compile 5m 26s the patch passed
        +1 javac 5m 26s the patch passed
        -0 checkstyle 0m 53s hadoop-yarn-project/hadoop-yarn: The patch generated 7 new + 60 unchanged - 5 fixed = 67 total (was 65)
        +1 mvnsite 1m 9s the patch passed
        -1 whitespace 0m 1s The patch has 4 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
        +1 findbugs 2m 29s the patch passed
        -1 javadoc 0m 36s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common generated 1 new + 4573 unchanged - 0 fixed = 4574 total (was 4573)
        +1 javadoc 0m 25s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager generated 0 new + 226 unchanged - 1 fixed = 226 total (was 227)
        +1 unit 2m 29s hadoop-yarn-common in the patch passed.
        +1 unit 13m 36s hadoop-yarn-server-nodemanager in the patch passed.
        +1 asflicense 0m 37s The patch does not generate ASF License warnings.
        67m 9s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:14b5c93
        JIRA Issue YARN-6708
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12873914/YARN-6708.001.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux d7181580bd85 3.13.0-107-generic #154-Ubuntu SMP Tue Dec 20 09:57:27 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / e806c6e
        Default Java 1.8.0_131
        findbugs v3.1.0-RC1
        findbugs https://builds.apache.org/job/PreCommit-YARN-Build/16212/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager-warnings.html
        checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16212/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
        whitespace https://builds.apache.org/job/PreCommit-YARN-Build/16212/artifact/patchprocess/whitespace-eol.txt
        javadoc https://builds.apache.org/job/PreCommit-YARN-Build/16212/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/16212/testReport/
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/16212/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 16s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. 0 mvndep 0m 10s Maven dependency ordering for branch +1 mvninstall 15m 26s trunk passed +1 compile 9m 35s trunk passed +1 checkstyle 0m 53s trunk passed +1 mvnsite 1m 12s trunk passed -1 findbugs 0m 50s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager in trunk has 5 extant Findbugs warnings. +1 javadoc 1m 2s trunk passed 0 mvndep 0m 11s Maven dependency ordering for patch +1 mvninstall 1m 0s the patch passed +1 compile 5m 26s the patch passed +1 javac 5m 26s the patch passed -0 checkstyle 0m 53s hadoop-yarn-project/hadoop-yarn: The patch generated 7 new + 60 unchanged - 5 fixed = 67 total (was 65) +1 mvnsite 1m 9s the patch passed -1 whitespace 0m 1s The patch has 4 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply +1 findbugs 2m 29s the patch passed -1 javadoc 0m 36s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common generated 1 new + 4573 unchanged - 0 fixed = 4574 total (was 4573) +1 javadoc 0m 25s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager generated 0 new + 226 unchanged - 1 fixed = 226 total (was 227) +1 unit 2m 29s hadoop-yarn-common in the patch passed. +1 unit 13m 36s hadoop-yarn-server-nodemanager in the patch passed. +1 asflicense 0m 37s The patch does not generate ASF License warnings. 67m 9s Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue YARN-6708 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12873914/YARN-6708.001.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux d7181580bd85 3.13.0-107-generic #154-Ubuntu SMP Tue Dec 20 09:57:27 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / e806c6e Default Java 1.8.0_131 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-YARN-Build/16212/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager-warnings.html checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16212/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt whitespace https://builds.apache.org/job/PreCommit-YARN-Build/16212/artifact/patchprocess/whitespace-eol.txt javadoc https://builds.apache.org/job/PreCommit-YARN-Build/16212/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/16212/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/16212/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 14s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
        0 mvndep 0m 10s Maven dependency ordering for branch
        +1 mvninstall 15m 3s trunk passed
        +1 compile 9m 50s trunk passed
        +1 checkstyle 0m 55s trunk passed
        +1 mvnsite 1m 15s trunk passed
        -1 findbugs 0m 55s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager in trunk has 5 extant Findbugs warnings.
        +1 javadoc 1m 3s trunk passed
        0 mvndep 0m 10s Maven dependency ordering for patch
        +1 mvninstall 1m 3s the patch passed
        +1 compile 6m 1s the patch passed
        +1 javac 6m 1s the patch passed
        -0 checkstyle 0m 55s hadoop-yarn-project/hadoop-yarn: The patch generated 7 new + 60 unchanged - 5 fixed = 67 total (was 65)
        +1 mvnsite 1m 18s the patch passed
        -1 whitespace 0m 0s The patch has 4 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
        +1 findbugs 2m 32s the patch passed
        -1 javadoc 0m 36s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common generated 1 new + 4573 unchanged - 0 fixed = 4574 total (was 4573)
        +1 javadoc 0m 26s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager generated 0 new + 226 unchanged - 1 fixed = 226 total (was 227)
        +1 unit 2m 39s hadoop-yarn-common in the patch passed.
        +1 unit 13m 21s hadoop-yarn-server-nodemanager in the patch passed.
        +1 asflicense 0m 33s The patch does not generate ASF License warnings.
        67m 55s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:14b5c93
        JIRA Issue YARN-6708
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12873998/YARN-6708.002.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux f8ba24e2ab3a 3.13.0-107-generic #154-Ubuntu SMP Tue Dec 20 09:57:27 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / c22cf00
        Default Java 1.8.0_131
        findbugs v3.1.0-RC1
        findbugs https://builds.apache.org/job/PreCommit-YARN-Build/16219/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager-warnings.html
        checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16219/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
        whitespace https://builds.apache.org/job/PreCommit-YARN-Build/16219/artifact/patchprocess/whitespace-eol.txt
        javadoc https://builds.apache.org/job/PreCommit-YARN-Build/16219/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/16219/testReport/
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/16219/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 14s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. 0 mvndep 0m 10s Maven dependency ordering for branch +1 mvninstall 15m 3s trunk passed +1 compile 9m 50s trunk passed +1 checkstyle 0m 55s trunk passed +1 mvnsite 1m 15s trunk passed -1 findbugs 0m 55s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager in trunk has 5 extant Findbugs warnings. +1 javadoc 1m 3s trunk passed 0 mvndep 0m 10s Maven dependency ordering for patch +1 mvninstall 1m 3s the patch passed +1 compile 6m 1s the patch passed +1 javac 6m 1s the patch passed -0 checkstyle 0m 55s hadoop-yarn-project/hadoop-yarn: The patch generated 7 new + 60 unchanged - 5 fixed = 67 total (was 65) +1 mvnsite 1m 18s the patch passed -1 whitespace 0m 0s The patch has 4 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply +1 findbugs 2m 32s the patch passed -1 javadoc 0m 36s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common generated 1 new + 4573 unchanged - 0 fixed = 4574 total (was 4573) +1 javadoc 0m 26s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager generated 0 new + 226 unchanged - 1 fixed = 226 total (was 227) +1 unit 2m 39s hadoop-yarn-common in the patch passed. +1 unit 13m 21s hadoop-yarn-server-nodemanager in the patch passed. +1 asflicense 0m 33s The patch does not generate ASF License warnings. 67m 55s Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue YARN-6708 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12873998/YARN-6708.002.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux f8ba24e2ab3a 3.13.0-107-generic #154-Ubuntu SMP Tue Dec 20 09:57:27 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / c22cf00 Default Java 1.8.0_131 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-YARN-Build/16219/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager-warnings.html checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16219/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt whitespace https://builds.apache.org/job/PreCommit-YARN-Build/16219/artifact/patchprocess/whitespace-eol.txt javadoc https://builds.apache.org/job/PreCommit-YARN-Build/16219/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/16219/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/16219/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        bibinchundatt Bibin A Chundatt added a comment -

        Attaching patch after following modification

        1. Testcase addition
        2. findbug correction
        3. whitespace correction

        Please review patch attached

        Show
        bibinchundatt Bibin A Chundatt added a comment - Attaching patch after following modification Testcase addition findbug correction whitespace correction Please review patch attached
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 13s Docker mode activated.
        +1 @author 0m 1s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
        0 mvndep 0m 12s Maven dependency ordering for branch
        +1 mvninstall 12m 37s trunk passed
        +1 compile 9m 16s trunk passed
        +1 checkstyle 0m 51s trunk passed
        +1 mvnsite 1m 11s 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 1m 1s trunk passed
        0 mvndep 0m 11s Maven dependency ordering for patch
        +1 mvninstall 0m 49s the patch passed
        +1 compile 5m 6s the patch passed
        +1 javac 5m 6s the patch passed
        -0 checkstyle 0m 49s hadoop-yarn-project/hadoop-yarn: The patch generated 10 new + 135 unchanged - 5 fixed = 145 total (was 140)
        +1 mvnsite 1m 9s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 2m 3s the patch passed
        +1 javadoc 0m 33s hadoop-yarn-common in the patch passed.
        +1 javadoc 0m 24s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager generated 0 new + 226 unchanged - 1 fixed = 226 total (was 227)
        +1 unit 2m 34s hadoop-yarn-common in the patch passed.
        +1 unit 13m 20s hadoop-yarn-server-nodemanager in the patch passed.
        +1 asflicense 0m 33s The patch does not generate ASF License warnings.
        62m 47s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:14b5c93
        JIRA Issue YARN-6708
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12874105/YARN-6708.003.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 7558ca6f45cf 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 / 8dbd53e
        Default Java 1.8.0_131
        findbugs v3.1.0-RC1
        findbugs https://builds.apache.org/job/PreCommit-YARN-Build/16223/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager-warnings.html
        checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16223/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/16223/testReport/
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/16223/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 13s Docker mode activated. +1 @author 0m 1s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files. 0 mvndep 0m 12s Maven dependency ordering for branch +1 mvninstall 12m 37s trunk passed +1 compile 9m 16s trunk passed +1 checkstyle 0m 51s trunk passed +1 mvnsite 1m 11s 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 1m 1s trunk passed 0 mvndep 0m 11s Maven dependency ordering for patch +1 mvninstall 0m 49s the patch passed +1 compile 5m 6s the patch passed +1 javac 5m 6s the patch passed -0 checkstyle 0m 49s hadoop-yarn-project/hadoop-yarn: The patch generated 10 new + 135 unchanged - 5 fixed = 145 total (was 140) +1 mvnsite 1m 9s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 2m 3s the patch passed +1 javadoc 0m 33s hadoop-yarn-common in the patch passed. +1 javadoc 0m 24s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager generated 0 new + 226 unchanged - 1 fixed = 226 total (was 227) +1 unit 2m 34s hadoop-yarn-common in the patch passed. +1 unit 13m 20s hadoop-yarn-server-nodemanager in the patch passed. +1 asflicense 0m 33s The patch does not generate ASF License warnings. 62m 47s Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue YARN-6708 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12874105/YARN-6708.003.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 7558ca6f45cf 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 / 8dbd53e Default Java 1.8.0_131 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-YARN-Build/16223/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager-warnings.html checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16223/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/16223/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/16223/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        bibinchundatt Bibin A Chundatt added a comment -

        cc:/ Jason Lowe.

        Show
        bibinchundatt Bibin A Chundatt added a comment - cc:/ Jason Lowe .
        Hide
        jlowe Jason Lowe added a comment -

        Thanks for the report and the patch!

        I'm not a fan of moving LocalCacheDirectoryManager to yarn-common. It is very specific to the peculiarities of how container localization works, and therefore isn't really a reusable component as something in yarn-common would imply.

        Instead I think it's more appropriate for the directory to be created before FSDownload tries to download it. Notice that FSDownload when it creates the directory is not expecting to create parents, because it is explicitly calling the mkdir form that should fail when parent directories do not exist. That made me wonder how this is actually working in practice, and I found that the place where the parents are getting auto-created is actually this code chunk in ContainerLocalizer:

          Callable<Path> download(Path path, LocalResource rsrc,
              UserGroupInformation ugi) throws IOException {
            diskValidator.checkStatus(new File(path.toUri().getRawPath()));
            return new FSDownloadWrapper(lfs, ugi, conf, path, rsrc);
          }
        

        The checkStatus call is calling checkDir which in turn calls mkdirsWithExistsCheck. That's creating the parent directories with default permissions. I'd rather see ContainerLocalizer setup the parent directories with proper permissions before calling FSDownload. ContainerLocalizer is already in the appropriate package to leverage LocalCacheDirectoryManager and seems like a more appropriate place to make this change.

        Show
        jlowe Jason Lowe added a comment - Thanks for the report and the patch! I'm not a fan of moving LocalCacheDirectoryManager to yarn-common. It is very specific to the peculiarities of how container localization works, and therefore isn't really a reusable component as something in yarn-common would imply. Instead I think it's more appropriate for the directory to be created before FSDownload tries to download it. Notice that FSDownload when it creates the directory is not expecting to create parents, because it is explicitly calling the mkdir form that should fail when parent directories do not exist. That made me wonder how this is actually working in practice, and I found that the place where the parents are getting auto-created is actually this code chunk in ContainerLocalizer: Callable<Path> download(Path path, LocalResource rsrc, UserGroupInformation ugi) throws IOException { diskValidator.checkStatus( new File(path.toUri().getRawPath())); return new FSDownloadWrapper(lfs, ugi, conf, path, rsrc); } The checkStatus call is calling checkDir which in turn calls mkdirsWithExistsCheck. That's creating the parent directories with default permissions. I'd rather see ContainerLocalizer setup the parent directories with proper permissions before calling FSDownload. ContainerLocalizer is already in the appropriate package to leverage LocalCacheDirectoryManager and seems like a more appropriate place to make this change.
        Hide
        bibinchundatt Bibin A Chundatt added a comment -

        Jason Lowe
        Thank you for looking into issue.Attaching patch with following modification.

        1. Moved folder creation to container Localizer.
        2. Testcase to verify folder permission are based on configuration.
        Show
        bibinchundatt Bibin A Chundatt added a comment - Jason Lowe Thank you for looking into issue.Attaching patch with following modification. Moved folder creation to container Localizer. Testcase to verify folder permission are based on configuration.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 8s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
        +1 mvninstall 13m 27s trunk passed
        +1 compile 0m 29s trunk passed
        +1 checkstyle 0m 18s trunk passed
        +1 mvnsite 0m 27s 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 18s trunk passed
        +1 mvninstall 0m 24s the patch passed
        +1 compile 0m 25s the patch passed
        +1 javac 0m 25s the patch passed
        -0 checkstyle 0m 16s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager: The patch generated 4 new + 54 unchanged - 1 fixed = 58 total (was 55)
        +1 mvnsite 0m 25s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 0m 48s the patch passed
        +1 javadoc 0m 15s the patch passed
        +1 unit 12m 46s hadoop-yarn-server-nodemanager in the patch passed.
        +1 asflicense 0m 20s The patch does not generate ASF License warnings.
        32m 44s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:14b5c93
        JIRA Issue YARN-6708
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12874715/YARN-6708.004.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 9619f3540737 3.13.0-119-generic #166-Ubuntu SMP Wed May 3 12:18:55 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 94e39c6
        Default Java 1.8.0_131
        findbugs v3.1.0-RC1
        findbugs https://builds.apache.org/job/PreCommit-YARN-Build/16255/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager-warnings.html
        checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16255/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/16255/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/16255/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 8s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 13m 27s trunk passed +1 compile 0m 29s trunk passed +1 checkstyle 0m 18s trunk passed +1 mvnsite 0m 27s 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 18s trunk passed +1 mvninstall 0m 24s the patch passed +1 compile 0m 25s the patch passed +1 javac 0m 25s the patch passed -0 checkstyle 0m 16s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager: The patch generated 4 new + 54 unchanged - 1 fixed = 58 total (was 55) +1 mvnsite 0m 25s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 48s the patch passed +1 javadoc 0m 15s the patch passed +1 unit 12m 46s hadoop-yarn-server-nodemanager in the patch passed. +1 asflicense 0m 20s The patch does not generate ASF License warnings. 32m 44s Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue YARN-6708 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12874715/YARN-6708.004.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 9619f3540737 3.13.0-119-generic #166-Ubuntu SMP Wed May 3 12:18:55 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 94e39c6 Default Java 1.8.0_131 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-YARN-Build/16255/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager-warnings.html checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16255/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/16255/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/16255/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        jlowe Jason Lowe added a comment -

        Thanks for updating the patch! This looks a lot cleaner.

        Is there a reason to use 755 permissions on the intermediate directories in the user cache? Note that we only allow 710 permissions on the final directly, and it seems intermediate directories should only require that as well, or 750 at the most. I don't see any reason to allow any "other" permissions on user-specific directories in the local cache.

        If the parent of destDirPath is the cache root then we won't set the permissions of destDirPath but otherwise we will? Seems like we could end up with inconsistent permissions on dest directories since sometimes the disk validator will end up creating it. I think we should just always do the while loop rather than special-case the no-intermediate-directories case – or am I missing something?

        Nit: It would improve readability if we moved the directory handling stuff to a utility method like createDirAndParents or something similar and pass the desired permissions for the dirs as an argument.

        There be an After method that deletes basedir so we don't leave cruft on the filesystem if a unit test fails.

        The AtomicLong use is overkill in the test since there's no thread contention on that object. Actually the test could be a lot simpler if we just told the container localizer to download a singl file to basedir/0/0/85/ and verify the permissions of the directories are correct. No need to download a bunch of times – we're not testing the LocalCacheDirectoryManager here, and if the LocalCacheDirectoryManager internals change then this test is going to fail since it knows a bit too much about it.

        Similarly I do not see the need to actually create a jar and copy it. We can mock out the downloading process (see the mockOutDownloads method) so we don't have to have a real file on disk to download. The localizer will still create the directories but the executor service won't actually call FSDownload, and that's fine for what we're trying to test.

        The test should not be using reflection to modify access to objects. The user cache permissions can be package private, and we can just replicate the FSDownload permissions for this test if really necessary.

        Show
        jlowe Jason Lowe added a comment - Thanks for updating the patch! This looks a lot cleaner. Is there a reason to use 755 permissions on the intermediate directories in the user cache? Note that we only allow 710 permissions on the final directly, and it seems intermediate directories should only require that as well, or 750 at the most. I don't see any reason to allow any "other" permissions on user-specific directories in the local cache. If the parent of destDirPath is the cache root then we won't set the permissions of destDirPath but otherwise we will? Seems like we could end up with inconsistent permissions on dest directories since sometimes the disk validator will end up creating it. I think we should just always do the while loop rather than special-case the no-intermediate-directories case – or am I missing something? Nit: It would improve readability if we moved the directory handling stuff to a utility method like createDirAndParents or something similar and pass the desired permissions for the dirs as an argument. There be an After method that deletes basedir so we don't leave cruft on the filesystem if a unit test fails. The AtomicLong use is overkill in the test since there's no thread contention on that object. Actually the test could be a lot simpler if we just told the container localizer to download a singl file to basedir/0/0/85/ and verify the permissions of the directories are correct. No need to download a bunch of times – we're not testing the LocalCacheDirectoryManager here, and if the LocalCacheDirectoryManager internals change then this test is going to fail since it knows a bit too much about it. Similarly I do not see the need to actually create a jar and copy it. We can mock out the downloading process (see the mockOutDownloads method) so we don't have to have a real file on disk to download. The localizer will still create the directories but the executor service won't actually call FSDownload, and that's fine for what we're trying to test. The test should not be using reflection to modify access to objects. The user cache permissions can be package private, and we can just replicate the FSDownload permissions for this test if really necessary.
        Hide
        bibinchundatt Bibin A Chundatt added a comment -

        Thank you Jason Lowe for review.

        Is there a reason to use 755 permissions on the intermediate directories in the user cache? Note that we only allow 710 permissions on the final directly, and it seems intermediate directories should only require that as well, or 750 at the most. I don't see any reason to allow any "other" permissions on user-specific directories in the local cache.

        755 is the existing directory permissions for cache folders in FsDownload#cacheperms. If Node manager service and users are in different group should be able to check the availability of existing cache folders during download and recovery. LocalResourcesTrackerImpl#handle

            case REQUEST:
              if (rsrc != null && (!isResourcePresent(rsrc))) {
                LOG.info("Resource " + rsrc.getLocalPath()
                    + " is missing, localizing it again");
                removeResource(req);
                rsrc = null;
              }
        

        It would improve readability if we moved the directory handling stuff to a utility method like createDirAndParents or something similar and pass the desired permissions for the dirs as an argument.

        Will update in next patch.

        vm2:/opt/hadoop/release/data/nmlocal/usercache/mapred/filecache # l
        total 28
        drwx--x--- 7 mapred hadoop 4096 Jun 10 14:35 ./
        drwxr-s--- 4 mapred hadoop 4096 Jun 10 12:07 ../
        drwxr-x--- 3 mapred users  4096 Jun 10 14:36 0/
        drwxr-xr-x 3 mapred users  4096 Jun 10 12:15 10/
        drwxr-xr-x 3 mapred users  4096 Jun 10 12:22 11/
        drwxr-xr-x 3 mapred users  4096 Jun 10 12:27 12/
        drwxr-xr-x 3 mapred users  4096 Jun 10 12:31 13/
        

        If the parent of destDirPath is the cache root then we won't set the permissions of destDirPath but otherwise we will?

        Already existing FSDownload code handles this case. FSDownload cacheperms sets directory permissions as 755.
        FSDownload should have been in nodemanager since its tightly coupled to the directoy permission wrt to localization . or am I missing something?

        AtomicLong use is overkill in the test since there's no thread contention on that object.

        Yes .. we dont require will change.

        In test tried to cover complete flow with multiple base directory, single base directory etc.. On second thought we really don't require. LocalCacheDirectoryManager part we could skip. Creating paths 12, 1/14 0/0/85 should be enough for current code change.

        FSDownload handles the final cache directory permissions. Even if 0/0/85 is created before download, in FSDownload for 85 the same could get reset rt??

        The directory permission is 755 and in jenkins the umask is 022 to validate directory rights for code change used reflection. Container localizer USERCACHE permission could be package private but the above point of FSDownload will set the rights to 0755 or we should be checking only 0/0??

        Show
        bibinchundatt Bibin A Chundatt added a comment - Thank you Jason Lowe for review. Is there a reason to use 755 permissions on the intermediate directories in the user cache? Note that we only allow 710 permissions on the final directly, and it seems intermediate directories should only require that as well, or 750 at the most. I don't see any reason to allow any "other" permissions on user-specific directories in the local cache. 755 is the existing directory permissions for cache folders in FsDownload#cacheperms . If Node manager service and users are in different group should be able to check the availability of existing cache folders during download and recovery. LocalResourcesTrackerImpl#handle case REQUEST: if (rsrc != null && (!isResourcePresent(rsrc))) { LOG.info( "Resource " + rsrc.getLocalPath() + " is missing, localizing it again" ); removeResource(req); rsrc = null ; } It would improve readability if we moved the directory handling stuff to a utility method like createDirAndParents or something similar and pass the desired permissions for the dirs as an argument. Will update in next patch. vm2:/opt/hadoop/release/data/nmlocal/usercache/mapred/filecache # l total 28 drwx--x--- 7 mapred hadoop 4096 Jun 10 14:35 ./ drwxr-s--- 4 mapred hadoop 4096 Jun 10 12:07 ../ drwxr-x--- 3 mapred users 4096 Jun 10 14:36 0/ drwxr-xr-x 3 mapred users 4096 Jun 10 12:15 10/ drwxr-xr-x 3 mapred users 4096 Jun 10 12:22 11/ drwxr-xr-x 3 mapred users 4096 Jun 10 12:27 12/ drwxr-xr-x 3 mapred users 4096 Jun 10 12:31 13/ If the parent of destDirPath is the cache root then we won't set the permissions of destDirPath but otherwise we will? Already existing FSDownload code handles this case. FSDownload cacheperms sets directory permissions as 755 . FSDownload should have been in nodemanager since its tightly coupled to the directoy permission wrt to localization . or am I missing something? AtomicLong use is overkill in the test since there's no thread contention on that object. Yes .. we dont require will change. In test tried to cover complete flow with multiple base directory, single base directory etc.. On second thought we really don't require. LocalCacheDirectoryManager part we could skip. Creating paths 12 , 1/14 0/0/85 should be enough for current code change. FSDownload handles the final cache directory permissions. Even if 0/0/85 is created before download, in FSDownload for 85 the same could get reset rt?? The directory permission is 755 and in jenkins the umask is 022 to validate directory rights for code change used reflection. Container localizer USERCACHE permission could be package private but the above point of FSDownload will set the rights to 0755 or we should be checking only 0/0 ??
        Hide
        jlowe Jason Lowe added a comment -

        If Node manager service and users are in different group should be able to check the availability of existing cache folders during download and recovery.

        Right, I was confusing this with HDFS group behavior where we'd setup the parent directory to ensure the group of child directories is always one the NM participates in. As you say, we need it to be more permissive (at least 0711) so the NM will always be able to stat the localized resource. 0755 should be fine, the parent directory has already locked the filecache down to 710 anyway so only the user and the NM's group can get in.

        Already existing FSDownload code handles this case.

        What I meant by that comment is it's weird that we sometimes create the destDirPath leaf directory before calling FSDownload. If the parent is the local cache root we do not and leave that for FSDownload to do, but if the parent isn't the root then we do create it. We should either always create it or never create it. For example, I think the code should be more like the following. Also note the simplified logic where the whole path object is pushed onto the stack which saves having to manipulate the path when we pop things off the stack:

              Path parent = destDirPath.getParent();
              Path cacheRoot = LocalCacheDirectoryManager.getCacheDirectoryRoot(parent);
              Stack<String> dirs = new Stack<String>();
              while (parent != null && !parent.equals(cacheRoot)) {
                dirs.push(parent);
                parent = parent.getParent();
              }
              // Create sub directories with same permission
              while (!dirs.isEmpty()) {
                createDir(lfs, dirs.pop(), USER_CACHE_FOLDER_PERMS);
              }
        

        That way we always leave it to FSDownload to create the leaf directory. If we'd rather always create it before calling FSDownload then we simply push destDirPath on the stack before the loop.

        FSDownload should have been in nodemanager since its tightly coupled to the directoy permission wrt to localization

        With respect to permissions on that directory, yes, although those directory permissions are a typical default and not necessarily specific to localization. It's currently used by code outside of the nodemanager and therefore a bit tricky to move without breaking things that may be using it.

        FSDownload handles the final cache directory permissions. Even if 0/0/85 is created before download, in FSDownload for 85 the same could get reset rt??

        FSDownload will reset an existing cache directory permissions to 0755 iff the umask is too restrictive to allow 0755 by default. Either way the directory is going to be 0755 after FSDownload is done with it, and that's all we need.

        The directory permission is 755 and in jenkins the umask is 022 to validate directory rights for code change used reflection. Container localizer USERCACHE permission could be package private but the above point of FSDownload will set the rights to 0755 or we should be checking only 0/0??

        We should only be checking 0/0. We already know the leaf directory will be 0755 because FSDownload does that for us. The unit test can go ahead and verify 0/0/85 is the right permissions, but it will be the case even before the patch. Still probably worth it in case somehow they're not correct. What's critical is to test the permissions of 0/ and 0/0/.

        As far as the umask is concerned, you should be able to override it by passing a FileContext to the ContainerLocalizer where we've explicitly called setUMask on it to the desired umask you need for the test. We should not modify constants in classes to execute this test, especially via reflection. That's very confusing and could break other tests in the same test suite that run afterwards since the classes have been scribbled upon.

        Please also address the checkstyle comments in the new patch. I'm guessing many of them will become moot as part of the unit test rewrite.

        Show
        jlowe Jason Lowe added a comment - If Node manager service and users are in different group should be able to check the availability of existing cache folders during download and recovery. Right, I was confusing this with HDFS group behavior where we'd setup the parent directory to ensure the group of child directories is always one the NM participates in. As you say, we need it to be more permissive (at least 0711) so the NM will always be able to stat the localized resource. 0755 should be fine, the parent directory has already locked the filecache down to 710 anyway so only the user and the NM's group can get in. Already existing FSDownload code handles this case. What I meant by that comment is it's weird that we sometimes create the destDirPath leaf directory before calling FSDownload. If the parent is the local cache root we do not and leave that for FSDownload to do, but if the parent isn't the root then we do create it. We should either always create it or never create it. For example, I think the code should be more like the following. Also note the simplified logic where the whole path object is pushed onto the stack which saves having to manipulate the path when we pop things off the stack: Path parent = destDirPath.getParent(); Path cacheRoot = LocalCacheDirectoryManager.getCacheDirectoryRoot(parent); Stack< String > dirs = new Stack< String >(); while (parent != null && !parent.equals(cacheRoot)) { dirs.push(parent); parent = parent.getParent(); } // Create sub directories with same permission while (!dirs.isEmpty()) { createDir(lfs, dirs.pop(), USER_CACHE_FOLDER_PERMS); } That way we always leave it to FSDownload to create the leaf directory. If we'd rather always create it before calling FSDownload then we simply push destDirPath on the stack before the loop. FSDownload should have been in nodemanager since its tightly coupled to the directoy permission wrt to localization With respect to permissions on that directory, yes, although those directory permissions are a typical default and not necessarily specific to localization. It's currently used by code outside of the nodemanager and therefore a bit tricky to move without breaking things that may be using it. FSDownload handles the final cache directory permissions. Even if 0/0/85 is created before download, in FSDownload for 85 the same could get reset rt?? FSDownload will reset an existing cache directory permissions to 0755 iff the umask is too restrictive to allow 0755 by default. Either way the directory is going to be 0755 after FSDownload is done with it, and that's all we need. The directory permission is 755 and in jenkins the umask is 022 to validate directory rights for code change used reflection. Container localizer USERCACHE permission could be package private but the above point of FSDownload will set the rights to 0755 or we should be checking only 0/0?? We should only be checking 0/0. We already know the leaf directory will be 0755 because FSDownload does that for us. The unit test can go ahead and verify 0/0/85 is the right permissions, but it will be the case even before the patch. Still probably worth it in case somehow they're not correct. What's critical is to test the permissions of 0/ and 0/0/. As far as the umask is concerned, you should be able to override it by passing a FileContext to the ContainerLocalizer where we've explicitly called setUMask on it to the desired umask you need for the test. We should not modify constants in classes to execute this test, especially via reflection. That's very confusing and could break other tests in the same test suite that run afterwards since the classes have been scribbled upon. Please also address the checkstyle comments in the new patch. I'm guessing many of them will become moot as part of the unit test rewrite.
        Hide
        bibinchundatt Bibin A Chundatt added a comment -

        As far as the umask is concerned, you should be able to override it by passing a FileContext to the ContainerLocalizer where we've explicitly called setUMask on it to the desired umask you need for the test.

        Tried this approach earlier didnt work out since DiskChecker#checkDir doesnt handle based on FileContext.
        We can't change either to

        checkDir(LocalFileSystem localFS, Path dir,
        FsPermission expected)

        Since DiskValidator.checkstatus dont have an interface with using FileContext

        Show
        bibinchundatt Bibin A Chundatt added a comment - As far as the umask is concerned, you should be able to override it by passing a FileContext to the ContainerLocalizer where we've explicitly called setUMask on it to the desired umask you need for the test. Tried this approach earlier didnt work out since DiskChecker#checkDir doesnt handle based on FileContext . We can't change either to checkDir(LocalFileSystem localFS, Path dir, FsPermission expected) Since DiskValidator.checkstatus dont have an interface with using FileContext
        Hide
        jlowe Jason Lowe added a comment -

        Tried this approach earlier didnt work out since DiskChecker#checkDir doesnt handle based on FileContext.

        Well if we choose to always create the leaf directory then that should solve that particular problem, since we can create it with FileContex to get the proper permissions. ContainerLocalizer is calling checkDir without an expected permission, and that doesn't create the directory if it already exists. Although as I mentioned before, I don't think it's really important to check the leaf directory permissions so much as the intermediate ones, since that's what's important for fixing this JIRA.

        Show
        jlowe Jason Lowe added a comment - Tried this approach earlier didnt work out since DiskChecker#checkDir doesnt handle based on FileContext. Well if we choose to always create the leaf directory then that should solve that particular problem, since we can create it with FileContex to get the proper permissions. ContainerLocalizer is calling checkDir without an expected permission, and that doesn't create the directory if it already exists. Although as I mentioned before, I don't think it's really important to check the leaf directory permissions so much as the intermediate ones, since that's what's important for fixing this JIRA.
        Hide
        bibinchundatt Bibin A Chundatt added a comment - - edited

        Thank you Jason Lowe for review.
        Attaching patch after handling comments.

        Show
        bibinchundatt Bibin A Chundatt added a comment - - edited Thank you Jason Lowe for review. Attaching patch after handling comments.
        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 appears to include 1 new or modified test files.
        +1 mvninstall 14m 2s trunk passed
        +1 compile 0m 30s trunk passed
        +1 checkstyle 0m 17s trunk passed
        +1 mvnsite 0m 28s trunk passed
        -1 findbugs 0m 44s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager in trunk has 5 extant Findbugs warnings.
        +1 javadoc 0m 18s trunk passed
        +1 mvninstall 0m 26s the patch passed
        +1 compile 0m 26s the patch passed
        +1 javac 0m 26s the patch passed
        -0 checkstyle 0m 16s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager: The patch generated 7 new + 54 unchanged - 0 fixed = 61 total (was 54)
        +1 mvnsite 0m 27s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 0m 50s the patch passed
        +1 javadoc 0m 16s the patch passed
        +1 unit 14m 8s hadoop-yarn-server-nodemanager in the patch passed.
        +1 asflicense 0m 18s The patch does not generate ASF License warnings.
        35m 4s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:14b5c93
        JIRA Issue YARN-6708
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12875230/YARN-6708.005.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux dc368714cd74 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 / 3be2659
        Default Java 1.8.0_131
        findbugs v3.1.0-RC1
        findbugs https://builds.apache.org/job/PreCommit-YARN-Build/16284/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager-warnings.html
        checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16284/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/16284/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/16284/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 appears to include 1 new or modified test files. +1 mvninstall 14m 2s trunk passed +1 compile 0m 30s trunk passed +1 checkstyle 0m 17s trunk passed +1 mvnsite 0m 28s trunk passed -1 findbugs 0m 44s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager in trunk has 5 extant Findbugs warnings. +1 javadoc 0m 18s trunk passed +1 mvninstall 0m 26s the patch passed +1 compile 0m 26s the patch passed +1 javac 0m 26s the patch passed -0 checkstyle 0m 16s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager: The patch generated 7 new + 54 unchanged - 0 fixed = 61 total (was 54) +1 mvnsite 0m 27s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 50s the patch passed +1 javadoc 0m 16s the patch passed +1 unit 14m 8s hadoop-yarn-server-nodemanager in the patch passed. +1 asflicense 0m 18s The patch does not generate ASF License warnings. 35m 4s Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue YARN-6708 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12875230/YARN-6708.005.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux dc368714cd74 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 / 3be2659 Default Java 1.8.0_131 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-YARN-Build/16284/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager-warnings.html checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16284/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/16284/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/16284/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 12s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
        +1 mvninstall 15m 10s trunk passed
        +1 compile 0m 33s trunk passed
        +1 checkstyle 0m 19s trunk passed
        +1 mvnsite 0m 32s trunk passed
        -1 findbugs 0m 49s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager in trunk has 5 extant Findbugs warnings.
        +1 javadoc 0m 20s trunk passed
        +1 mvninstall 0m 29s the patch passed
        +1 compile 0m 30s the patch passed
        +1 javac 0m 30s the patch passed
        -0 checkstyle 0m 17s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager: The patch generated 7 new + 55 unchanged - 0 fixed = 62 total (was 55)
        +1 mvnsite 0m 25s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 0m 57s the patch passed
        +1 javadoc 0m 17s the patch passed
        +1 unit 13m 14s hadoop-yarn-server-nodemanager in the patch passed.
        +1 asflicense 0m 18s The patch does not generate ASF License warnings.
        35m 40s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:14b5c93
        JIRA Issue YARN-6708
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12875230/YARN-6708.005.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 3da39720b96d 3.13.0-119-generic #166-Ubuntu SMP Wed May 3 12:18:55 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 3be2659
        Default Java 1.8.0_131
        findbugs v3.1.0-RC1
        findbugs https://builds.apache.org/job/PreCommit-YARN-Build/16285/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager-warnings.html
        checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16285/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/16285/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/16285/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 12s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 15m 10s trunk passed +1 compile 0m 33s trunk passed +1 checkstyle 0m 19s trunk passed +1 mvnsite 0m 32s trunk passed -1 findbugs 0m 49s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager in trunk has 5 extant Findbugs warnings. +1 javadoc 0m 20s trunk passed +1 mvninstall 0m 29s the patch passed +1 compile 0m 30s the patch passed +1 javac 0m 30s the patch passed -0 checkstyle 0m 17s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager: The patch generated 7 new + 55 unchanged - 0 fixed = 62 total (was 55) +1 mvnsite 0m 25s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 57s the patch passed +1 javadoc 0m 17s the patch passed +1 unit 13m 14s hadoop-yarn-server-nodemanager in the patch passed. +1 asflicense 0m 18s The patch does not generate ASF License warnings. 35m 40s Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue YARN-6708 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12875230/YARN-6708.005.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 3da39720b96d 3.13.0-119-generic #166-Ubuntu SMP Wed May 3 12:18:55 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 3be2659 Default Java 1.8.0_131 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-YARN-Build/16285/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager-warnings.html checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/16285/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/16285/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/16285/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 12s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
        +1 mvninstall 15m 24s trunk passed
        +1 compile 1m 29s trunk passed
        +1 checkstyle 0m 54s trunk passed
        +1 mvnsite 1m 26s trunk passed
        -1 findbugs 1m 27s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager in trunk has 5 extant Findbugs warnings.
        +1 javadoc 0m 18s trunk passed
        +1 mvninstall 0m 24s the patch passed
        +1 compile 0m 26s the patch passed
        +1 javac 0m 26s the patch passed
        +1 checkstyle 0m 16s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager: The patch generated 0 new + 54 unchanged - 1 fixed = 54 total (was 55)
        +1 mvnsite 0m 25s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 0m 48s the patch passed
        +1 javadoc 0m 15s the patch passed
        +1 unit 12m 49s hadoop-yarn-server-nodemanager in the patch passed.
        +1 asflicense 0m 17s The patch does not generate ASF License warnings.
        38m 8s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:14b5c93
        JIRA Issue YARN-6708
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12875276/YARN-6708.006.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 53209432018d 3.13.0-119-generic #166-Ubuntu SMP Wed May 3 12:18:55 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / fa1aaee
        Default Java 1.8.0_131
        findbugs v3.1.0-RC1
        findbugs https://builds.apache.org/job/PreCommit-YARN-Build/16292/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/16292/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/16292/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 12s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 15m 24s trunk passed +1 compile 1m 29s trunk passed +1 checkstyle 0m 54s trunk passed +1 mvnsite 1m 26s trunk passed -1 findbugs 1m 27s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager in trunk has 5 extant Findbugs warnings. +1 javadoc 0m 18s trunk passed +1 mvninstall 0m 24s the patch passed +1 compile 0m 26s the patch passed +1 javac 0m 26s the patch passed +1 checkstyle 0m 16s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager: The patch generated 0 new + 54 unchanged - 1 fixed = 54 total (was 55) +1 mvnsite 0m 25s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 48s the patch passed +1 javadoc 0m 15s the patch passed +1 unit 12m 49s hadoop-yarn-server-nodemanager in the patch passed. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 38m 8s Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue YARN-6708 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12875276/YARN-6708.006.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 53209432018d 3.13.0-119-generic #166-Ubuntu SMP Wed May 3 12:18:55 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / fa1aaee Default Java 1.8.0_131 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-YARN-Build/16292/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/16292/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/16292/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 12s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
        +1 mvninstall 13m 12s trunk passed
        +1 compile 0m 30s trunk passed
        +1 checkstyle 0m 19s trunk passed
        +1 mvnsite 0m 30s trunk passed
        -1 findbugs 0m 48s 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 27s the patch passed
        +1 compile 0m 29s the patch passed
        +1 javac 0m 29s the patch passed
        +1 checkstyle 0m 17s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager: The patch generated 0 new + 54 unchanged - 1 fixed = 54 total (was 55)
        +1 mvnsite 0m 28s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 0m 55s the patch passed
        +1 javadoc 0m 16s 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.
        33m 12s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:14b5c93
        JIRA Issue YARN-6708
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12875276/YARN-6708.006.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux ebda8fd0e78b 3.13.0-119-generic #166-Ubuntu SMP Wed May 3 12:18:55 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / fa1aaee
        Default Java 1.8.0_131
        findbugs v3.1.0-RC1
        findbugs https://builds.apache.org/job/PreCommit-YARN-Build/16294/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/16294/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/16294/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 12s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 13m 12s trunk passed +1 compile 0m 30s trunk passed +1 checkstyle 0m 19s trunk passed +1 mvnsite 0m 30s trunk passed -1 findbugs 0m 48s 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 27s the patch passed +1 compile 0m 29s the patch passed +1 javac 0m 29s the patch passed +1 checkstyle 0m 17s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager: The patch generated 0 new + 54 unchanged - 1 fixed = 54 total (was 55) +1 mvnsite 0m 28s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 55s the patch passed +1 javadoc 0m 16s 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. 33m 12s Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue YARN-6708 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12875276/YARN-6708.006.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux ebda8fd0e78b 3.13.0-119-generic #166-Ubuntu SMP Wed May 3 12:18:55 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / fa1aaee Default Java 1.8.0_131 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-YARN-Build/16294/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/16294/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/16294/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        Naganarasimha Naganarasimha G R added a comment -

        Jason Lowe,
        Findbugs is not related to the patch and is tracked in YARN-6515, Apart from it i feel the work in the latest patch is fine. Shall i go ahead and merge if you are held up with other work ?

        Show
        Naganarasimha Naganarasimha G R added a comment - Jason Lowe , Findbugs is not related to the patch and is tracked in YARN-6515 , Apart from it i feel the work in the latest patch is fine. Shall i go ahead and merge if you are held up with other work ?
        Hide
        jlowe Jason Lowe added a comment -

        Sorry for the delay, was out of the office for a bit. Please hold off on committing this, as I plan on reviewing the latest patch later today.

        Show
        jlowe Jason Lowe added a comment - Sorry for the delay, was out of the office for a bit. Please hold off on committing this, as I plan on reviewing the latest patch later today.
        Hide
        jlowe Jason Lowe added a comment -

        Thanks for updating the patch! We're almost there, just some cleanup needed in the unit test.

        A prior review comment was missed in the patch update:

        There be an After method that deletes basedir so we don't leave cruft on the filesystem if a unit test fails.

        On a related note, the unit test should be using basedir rather than making up its own path under target to benefit from that cleanup. Otherwise the unit test is leaving cruft around on the filesystem after it runs.

        Also the unit test is passing for me even without the code change. It will only fail if the umask of the user running the test is more restrictive than 022 which is a typical default. One way to work around that is to explicitly create one of the parent directories with the wrong permissions first, e.g.: filecache/0 with permissions 0700. Then we can call the localizer and verify the permissions were fixed afterwards.

        Show
        jlowe Jason Lowe added a comment - Thanks for updating the patch! We're almost there, just some cleanup needed in the unit test. A prior review comment was missed in the patch update: There be an After method that deletes basedir so we don't leave cruft on the filesystem if a unit test fails. On a related note, the unit test should be using basedir rather than making up its own path under target to benefit from that cleanup. Otherwise the unit test is leaving cruft around on the filesystem after it runs. Also the unit test is passing for me even without the code change. It will only fail if the umask of the user running the test is more restrictive than 022 which is a typical default. One way to work around that is to explicitly create one of the parent directories with the wrong permissions first, e.g.: filecache/0 with permissions 0700. Then we can call the localizer and verify the permissions were fixed afterwards.
        Hide
        bibinchundatt Bibin A Chundatt added a comment -

        Apologies for missing out comment.
        Following changes are made in latest patch

        1. After added to test class to delete basedir
        2. Precreate directory and set wrong permission for USERCACHE folder
        3. USER_CACHE_FOLDER_PERMS renamed to USERCACHE_FOLDER_PERMS
        Show
        bibinchundatt Bibin A Chundatt added a comment - Apologies for missing out comment. Following changes are made in latest patch After added to test class to delete basedir Precreate directory and set wrong permission for USERCACHE folder USER_CACHE_FOLDER_PERMS renamed to USERCACHE_FOLDER_PERMS
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 15m 28s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
        +1 mvninstall 16m 39s trunk passed
        +1 compile 0m 28s trunk passed
        +1 checkstyle 0m 19s trunk passed
        +1 mvnsite 0m 28s trunk passed
        -1 findbugs 0m 44s 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 24s the patch passed
        +1 compile 0m 25s the patch passed
        +1 javac 0m 25s the patch passed
        +1 checkstyle 0m 16s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager: The patch generated 0 new + 53 unchanged - 1 fixed = 53 total (was 54)
        +1 mvnsite 0m 24s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 0m 47s the patch passed
        +1 javadoc 0m 15s the patch passed
        +1 unit 12m 46s hadoop-yarn-server-nodemanager in the patch passed.
        +1 asflicense 0m 21s The patch does not generate ASF License warnings.
        51m 18s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:14b5c93
        JIRA Issue YARN-6708
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12875865/YARN-6708.007.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 83f5c5bceb4a 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 / 946dd25
        Default Java 1.8.0_131
        findbugs v3.1.0-RC1
        findbugs https://builds.apache.org/job/PreCommit-YARN-Build/16307/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/16307/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/16307/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 15m 28s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 16m 39s trunk passed +1 compile 0m 28s trunk passed +1 checkstyle 0m 19s trunk passed +1 mvnsite 0m 28s trunk passed -1 findbugs 0m 44s 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 24s the patch passed +1 compile 0m 25s the patch passed +1 javac 0m 25s the patch passed +1 checkstyle 0m 16s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager: The patch generated 0 new + 53 unchanged - 1 fixed = 53 total (was 54) +1 mvnsite 0m 24s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 47s the patch passed +1 javadoc 0m 15s the patch passed +1 unit 12m 46s hadoop-yarn-server-nodemanager in the patch passed. +1 asflicense 0m 21s The patch does not generate ASF License warnings. 51m 18s Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue YARN-6708 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12875865/YARN-6708.007.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 83f5c5bceb4a 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 / 946dd25 Default Java 1.8.0_131 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-YARN-Build/16307/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/16307/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/16307/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        jlowe Jason Lowe added a comment -

        +1 latest patch lgtm. Committing this.

        Show
        jlowe Jason Lowe added a comment - +1 latest patch lgtm. Committing this.
        Hide
        jlowe Jason Lowe added a comment -

        Thanks to Bibin A Chundatt for the contribution and to Naganarasimha G R for additional review! I committed this to trunk, branch-2, branch-2.8, and branch-2.8.2.

        Show
        jlowe Jason Lowe added a comment - Thanks to Bibin A Chundatt for the contribution and to Naganarasimha G R for additional review! I committed this to trunk, branch-2, branch-2.8, and branch-2.8.2.
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11971 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11971/)
        YARN-6708. Nodemanager container crash after ext3 folder limit. (jlowe: rev 7576a688ea84aed7206321b1f03594e43a5f216e)

        • (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/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/TestContainerLocalizer.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11971 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11971/ ) YARN-6708 . Nodemanager container crash after ext3 folder limit. (jlowe: rev 7576a688ea84aed7206321b1f03594e43a5f216e) (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/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/TestContainerLocalizer.java

          People

          • Assignee:
            bibinchundatt Bibin A Chundatt
            Reporter:
            bibinchundatt Bibin A Chundatt
          • Votes:
            0 Vote for this issue
            Watchers:
            12 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development