Browse Source

HADOOP-9877. Fix listing of snapshot directories in globStatus. (Binglin Chang via Andrew Wang)

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/branch-2@1516236 13f79535-47bb-0310-9956-ffa450edef68
Andrew Wang 11 years ago
parent
commit
1bc892472c

+ 2 - 0
hadoop-common-project/hadoop-common/CHANGES.txt

@@ -67,6 +67,8 @@ Release 2.3.0 - UNRELEASED
     HADOOP-9865.  FileContext#globStatus has a regression with respect to
     HADOOP-9865.  FileContext#globStatus has a regression with respect to
     relative path.  (Chuan Lin via Colin Patrick McCabe)
     relative path.  (Chuan Lin via Colin Patrick McCabe)
 
 
+    HADOOP-9877. Fix listing of snapshot directories in globStatus.
+    (Binglin Chang via Andrew Wang)
 
 
 Release 2.1.1-beta - UNRELEASED
 Release 2.1.1-beta - UNRELEASED
 
 

+ 48 - 8
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Globber.java

@@ -62,6 +62,18 @@ class Globber {
     }
     }
   }
   }
 
 
+  private FileStatus getFileLinkStatus(Path path) {
+    try {
+      if (fs != null) {
+        return fs.getFileLinkStatus(path);
+      } else {
+        return fc.getFileLinkStatus(path);
+      }
+    } catch (IOException e) {
+      return null;
+    }
+  }
+
   private FileStatus[] listStatus(Path path) {
   private FileStatus[] listStatus(Path path) {
     try {
     try {
       if (fs != null) {
       if (fs != null) {
@@ -122,6 +134,18 @@ class Globber {
     return authority ;
     return authority ;
   }
   }
 
 
+  /**
+   * The glob filter builds a regexp per path component.  If the component
+   * does not contain a shell metachar, then it falls back to appending the
+   * raw string to the list of built up paths.  This raw path needs to have
+   * the quoting removed.  Ie. convert all occurrences of "\X" to "X"
+   * @param name of the path component
+   * @return the unquoted path component
+   */
+  private static String unquotePathComponent(String name) {
+    return name.replaceAll("\\\\(.)", "$1");
+  }
+
   public FileStatus[] glob() throws IOException {
   public FileStatus[] glob() throws IOException {
     // First we get the scheme and authority of the pattern that was passed
     // First we get the scheme and authority of the pattern that was passed
     // in.
     // in.
@@ -176,14 +200,30 @@ class Globber {
               resolvedCandidate.isDirectory() == false) {
               resolvedCandidate.isDirectory() == false) {
             continue;
             continue;
           }
           }
-          FileStatus[] children = listStatus(candidate.getPath());
-          for (FileStatus child : children) {
-            // Set the child path based on the parent path.
-            // This keeps the symlinks in our path.
-            child.setPath(new Path(candidate.getPath(),
-                    child.getPath().getName()));
-            if (globFilter.accept(child.getPath())) {
-              newCandidates.add(child);
+          // For components without pattern, we get its FileStatus directly
+          // using getFileLinkStatus for two reasons:
+          // 1. It should be faster to only get FileStatus needed rather than
+          //    get all children.
+          // 2. Some special filesystem directories (e.g. HDFS snapshot
+          //    directories) are not returned by listStatus, but do exist if
+          //    checked explicitly via getFileLinkStatus.
+          if (globFilter.hasPattern()) {
+            FileStatus[] children = listStatus(candidate.getPath());
+            for (FileStatus child : children) {
+              // Set the child path based on the parent path.
+              // This keeps the symlinks in our path.
+              child.setPath(new Path(candidate.getPath(),
+                      child.getPath().getName()));
+              if (globFilter.accept(child.getPath())) {
+                newCandidates.add(child);
+              }
+            }
+          } else {
+            Path p = new Path(candidate.getPath(), unquotePathComponent(component));
+            FileStatus s = getFileLinkStatus(p);
+            if (s != null) {
+              s.setPath(p);
+              newCandidates.add(s);
             }
             }
           }
           }
         }
         }

+ 15 - 0
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileContextMainOperationsBaseTest.java

@@ -30,6 +30,7 @@ import org.apache.hadoop.fs.Options.Rename;
 import org.apache.hadoop.fs.permission.FsPermission;
 import org.apache.hadoop.fs.permission.FsPermission;
 import org.junit.After;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Assert;
+import org.junit.Assume;
 import org.junit.Before;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.Test;
 
 
@@ -632,6 +633,20 @@ public abstract class FileContextMainOperationsBaseTest  {
         filteredPaths));
         filteredPaths));
   }
   }
   
   
+  protected Path getHiddenPathForTest() {
+    return null;
+  }
+  
+  @Test
+  public void testGlobStatusFilterWithHiddenPathTrivialFilter()
+      throws Exception {
+    Path hidden = getHiddenPathForTest();
+    Assume.assumeNotNull(hidden);
+    FileStatus[] filteredPaths = fc.util().globStatus(hidden, DEFAULT_FILTER);
+    Assert.assertNotNull(filteredPaths);
+    Assert.assertEquals(1, filteredPaths.length);
+  }
+
   @Test
   @Test
   public void testWriteReadAndDeleteEmptyFile() throws Exception {
   public void testWriteReadAndDeleteEmptyFile() throws Exception {
     writeReadAndDelete(0);
     writeReadAndDelete(0);

+ 19 - 0
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellReturnCode.java

@@ -488,6 +488,25 @@ public class TestFsShellReturnCode {
       }
       }
       return stat;
       return stat;
     }
     }
+
+    @Override
+    public FileStatus getFileLinkStatus(Path p) throws IOException {
+      String f = makeQualified(p).toString();
+      FileStatus stat = super.getFileLinkStatus(p);
+      
+      stat.getPermission();
+      if (owners.containsKey(f)) {
+        stat.setOwner("STUB-"+owners.get(f));      
+      } else {
+        stat.setOwner("REAL-"+stat.getOwner());
+      }
+      if (groups.containsKey(f)) {
+        stat.setGroup("STUB-"+groups.get(f));      
+      } else {
+        stat.setGroup("REAL-"+stat.getGroup());
+      }
+      return stat;
+    }
   }
   }
 
 
   /**
   /**

+ 11 - 0
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/TestHDFSFileContextMainOperations.java

@@ -59,6 +59,9 @@ public class TestHDFSFileContextMainOperations extends
     defaultWorkingDirectory = fc.makeQualified( new Path("/user/" + 
     defaultWorkingDirectory = fc.makeQualified( new Path("/user/" + 
         UserGroupInformation.getCurrentUser().getShortUserName()));
         UserGroupInformation.getCurrentUser().getShortUserName()));
     fc.mkdir(defaultWorkingDirectory, FileContext.DEFAULT_PERM, true);
     fc.mkdir(defaultWorkingDirectory, FileContext.DEFAULT_PERM, true);
+    // Make defaultWorkingDirectory snapshottable to enable 
+    // testGlobStatusFilterWithHiddenPathTrivialFilter
+    cluster.getFileSystem().allowSnapshot(defaultWorkingDirectory);
   }
   }
 
 
   private static void restartCluster() throws IOException, LoginException {
   private static void restartCluster() throws IOException, LoginException {
@@ -73,6 +76,9 @@ public class TestHDFSFileContextMainOperations extends
     defaultWorkingDirectory = fc.makeQualified( new Path("/user/" + 
     defaultWorkingDirectory = fc.makeQualified( new Path("/user/" + 
         UserGroupInformation.getCurrentUser().getShortUserName()));
         UserGroupInformation.getCurrentUser().getShortUserName()));
     fc.mkdir(defaultWorkingDirectory, FileContext.DEFAULT_PERM, true);
     fc.mkdir(defaultWorkingDirectory, FileContext.DEFAULT_PERM, true);
+    // Make defaultWorkingDirectory snapshottable to enable 
+    // testGlobStatusFilterWithHiddenPathTrivialFilter
+    cluster.getFileSystem().allowSnapshot(defaultWorkingDirectory);
   }
   }
       
       
   @AfterClass
   @AfterClass
@@ -92,6 +98,11 @@ public class TestHDFSFileContextMainOperations extends
     super.tearDown();
     super.tearDown();
   }
   }
 
 
+  @Override
+  protected Path getHiddenPathForTest() {
+    return new Path(defaultWorkingDirectory, ".snapshot");
+  }
+  
   @Override
   @Override
   protected Path getDefaultWorkingDirectory() {
   protected Path getDefaultWorkingDirectory() {
     return defaultWorkingDirectory;
     return defaultWorkingDirectory;