You are viewing a plain text version of this content. The canonical link for it is here.
Posted to hdfs-commits@hadoop.apache.org by el...@apache.org on 2011/11/21 08:26:57 UTC

svn commit: r1204378 - in /hadoop/common/branches/branch-0.22/hdfs: ./ src/java/org/apache/hadoop/hdfs/protocol/ src/java/org/apache/hadoop/hdfs/server/namenode/ src/test/hdfs/org/apache/hadoop/fs/

Author: eli
Date: Mon Nov 21 07:26:56 2011
New Revision: 1204378

URL: http://svn.apache.org/viewvc?rev=1204378&view=rev
Log:
HDFS-2514. svn merge -c 1204370 from trunk

Modified:
    hadoop/common/branches/branch-0.22/hdfs/CHANGES.txt
    hadoop/common/branches/branch-0.22/hdfs/src/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java
    hadoop/common/branches/branch-0.22/hdfs/src/java/org/apache/hadoop/hdfs/protocol/UnresolvedPathException.java
    hadoop/common/branches/branch-0.22/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
    hadoop/common/branches/branch-0.22/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/INode.java
    hadoop/common/branches/branch-0.22/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java
    hadoop/common/branches/branch-0.22/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java
    hadoop/common/branches/branch-0.22/hdfs/src/test/hdfs/org/apache/hadoop/fs/TestFcHdfsSymlink.java

Modified: hadoop/common/branches/branch-0.22/hdfs/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.22/hdfs/CHANGES.txt?rev=1204378&r1=1204377&r2=1204378&view=diff
==============================================================================
--- hadoop/common/branches/branch-0.22/hdfs/CHANGES.txt (original)
+++ hadoop/common/branches/branch-0.22/hdfs/CHANGES.txt Mon Nov 21 07:26:56 2011
@@ -663,6 +663,9 @@ Release 0.22.0 - Unreleased
 
     HDFS-2573. TestFiDataXceiverServer is failing, not testing OOME (cos)
 
+    HDFS-2514. Link resolution bug for intermediate symlinks with
+    relative targets. (eli)
+
 Release 0.21.1 - Unreleased
 
   IMPROVEMENTS

Modified: hadoop/common/branches/branch-0.22/hdfs/src/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.22/hdfs/src/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java?rev=1204378&r1=1204377&r2=1204378&view=diff
==============================================================================
--- hadoop/common/branches/branch-0.22/hdfs/src/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java (original)
+++ hadoop/common/branches/branch-0.22/hdfs/src/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java Mon Nov 21 07:26:56 2011
@@ -780,9 +780,9 @@ public interface ClientProtocol extends 
 
   /**
    * Create symlink to a file or directory.
-   * @param target The pathname of the destination that the
+   * @param target The path of the destination that the
    *               link points to.
-   * @param link The pathname of the link being created.
+   * @param link The path of the link being created.
    * @param dirPerm permissions to use when creating parent directories
    * @param createParent - if true then missing parent dirs are created
    *                       if false then parent must exist
@@ -803,14 +803,16 @@ public interface ClientProtocol extends 
       IOException;
 
   /**
-   * Resolve the first symbolic link on the specified path.
-   * @param path The pathname that needs to be resolved
-   * 
-   * @return The pathname after resolving the first symbolic link if any.
-   * 
+   * Return the target of the given symlink. If there is an intermediate
+   * symlink in the path (ie a symlink leading up to the final path component)
+   * then the given path is returned with this symlink resolved.
+   *
+   * @param path The path with a link that needs resolution.
+   * @return The path after resolving the first symbolic link in the path.
    * @throws AccessControlException permission denied
    * @throws FileNotFoundException If <code>path</code> does not exist
-   * @throws IOException If an I/O error occurred
+   * @throws IOException If the given path does not refer to a symlink
+   *           or an I/O error occurred
    */
   public String getLinkTarget(String path) throws AccessControlException,
       FileNotFoundException, IOException; 

Modified: hadoop/common/branches/branch-0.22/hdfs/src/java/org/apache/hadoop/hdfs/protocol/UnresolvedPathException.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.22/hdfs/src/java/org/apache/hadoop/hdfs/protocol/UnresolvedPathException.java?rev=1204378&r1=1204377&r2=1204378&view=diff
==============================================================================
--- hadoop/common/branches/branch-0.22/hdfs/src/java/org/apache/hadoop/hdfs/protocol/UnresolvedPathException.java (original)
+++ hadoop/common/branches/branch-0.22/hdfs/src/java/org/apache/hadoop/hdfs/protocol/UnresolvedPathException.java Mon Nov 21 07:26:56 2011
@@ -32,10 +32,10 @@ import org.apache.hadoop.fs.Path;
 @InterfaceStability.Evolving
 public final class UnresolvedPathException extends UnresolvedLinkException {
   private static final long serialVersionUID = 1L;
-  private String originalPath;  // The original path containing the link
-  private String linkTarget;    // The target of the link 
-  private String remainingPath; // The path part following the link
-  
+  private String path;        // The path containing the link
+  private String preceding;   // The path part preceding the link
+  private String remainder;   // The path part following the link
+  private String linkTarget;  // The link's target
 
   /**
    * Used by RemoteException to instantiate an UnresolvedPathException.
@@ -44,22 +44,30 @@ public final class UnresolvedPathExcepti
     super(msg);
   }
   
-  public UnresolvedPathException(String originalPath, String remainingPath, 
-      String linkTarget) {
-    this.originalPath  = originalPath;
-    this.remainingPath = remainingPath;
-    this.linkTarget    = linkTarget;
+  public UnresolvedPathException(String path, String preceding,
+      String remainder, String linkTarget) {
+    this.path = path;
+    this.preceding = preceding;
+    this.remainder = remainder;
+    this.linkTarget = linkTarget;
   }
 
-  public Path getUnresolvedPath() throws IOException {
-    return new Path(originalPath);
-  }
-  
+  /**
+   * Return a path with the link resolved with the target.
+   */
   public Path getResolvedPath() throws IOException {
-    if (remainingPath == null || "".equals(remainingPath)) {
-      return new Path(linkTarget);
+    // If the path is absolute we cam throw out the preceding part and
+    // just append the remainder to the target, otherwise append each
+    // piece to resolve the link in path.
+    boolean noRemainder = (remainder == null || "".equals(remainder));
+    Path target = new Path(linkTarget);
+    if (target.isUriPathAbsolute()) {
+      return noRemainder ? target : new Path(target, remainder);
+    } else {
+      return noRemainder
+        ? new Path(preceding, target)
+        : new Path(new Path(preceding, linkTarget), remainder);
     }
-    return new Path(linkTarget, remainingPath);
   }
 
   @Override
@@ -68,7 +76,7 @@ public final class UnresolvedPathExcepti
     if (msg != null) {
       return msg;
     }
-    String myMsg = "Unresolved path " + originalPath;
+    String myMsg = "Unresolved path " + path;
     try {
       return getResolvedPath().toString();
     } catch (IOException e) {

Modified: hadoop/common/branches/branch-0.22/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.22/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java?rev=1204378&r1=1204377&r2=1204378&view=diff
==============================================================================
--- hadoop/common/branches/branch-0.22/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java (original)
+++ hadoop/common/branches/branch-0.22/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java Mon Nov 21 07:26:56 2011
@@ -2095,10 +2095,12 @@ public class FSNamesystem implements FSC
     }
   }
 
-  /** Get the file info for a specific file.
+  /**
+   * Get the file info for a specific file.
+   *
    * @param src The string representation of the path to the file
    * @param resolveLink whether to throw UnresolvedLinkException 
-   *        if src refers to a symlinks
+   *        if src refers to a symlink
    *
    * @throws AccessControlException if access is denied
    * @throws UnresolvedLinkException if a symlink is encountered.

Modified: hadoop/common/branches/branch-0.22/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/INode.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.22/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/INode.java?rev=1204378&r1=1204377&r2=1204378&view=diff
==============================================================================
--- hadoop/common/branches/branch-0.22/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/INode.java (original)
+++ hadoop/common/branches/branch-0.22/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/INode.java Mon Nov 21 07:26:56 2011
@@ -363,14 +363,16 @@ abstract class INode implements Comparab
 
   /**
    * Given some components, create a path name.
-   * @param components
+   * @param components The path components
+   * @param start index
+   * @param end index
    * @return concatenated path
    */
-  static String constructPath(byte[][] components, int start) {
+  static String constructPath(byte[][] components, int start, int end) {
     StringBuilder buf = new StringBuilder();
-    for (int i = start; i < components.length; i++) {
+    for (int i = start; i < end; i++) {
       buf.append(DFSUtil.bytes2String(components[i]));
-      if (i < components.length - 1) {
+      if (i < end - 1) {
         buf.append(Path.SEPARATOR);
       }
     }

Modified: hadoop/common/branches/branch-0.22/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.22/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java?rev=1204378&r1=1204377&r2=1204378&view=diff
==============================================================================
--- hadoop/common/branches/branch-0.22/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java (original)
+++ hadoop/common/branches/branch-0.22/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java Mon Nov 21 07:26:56 2011
@@ -191,18 +191,19 @@ class INodeDirectory extends INode {
         existing[index] = curNode;
       }
       if (curNode.isLink() && (!lastComp || (lastComp && resolveLink))) {
-        if(NameNode.stateChangeLog.isDebugEnabled()) {
+        final String path = constructPath(components, 0, components.length);
+        final String preceding = constructPath(components, 0, count);
+        final String remainder =
+          constructPath(components, count + 1, components.length);
+        final String link = DFSUtil.bytes2String(components[count]);
+        final String target = ((INodeSymlink)curNode).getLinkValue();
+        if (NameNode.stateChangeLog.isDebugEnabled()) {
           NameNode.stateChangeLog.debug("UnresolvedPathException " +
-              " count: " + count +
-              " componenent: " + DFSUtil.bytes2String(components[count]) +
-              " full path: " + constructPath(components, 0) +
-              " remaining path: " + constructPath(components, count+1) +
-              " symlink: " + ((INodeSymlink)curNode).getLinkValue());
+            " path: " + path + " preceding: " + preceding +
+            " count: " + count + " link: " + link + " target: " + target +
+            " remainder: " + remainder);
         }
-        final String linkTarget = ((INodeSymlink)curNode).getLinkValue();
-        throw new UnresolvedPathException(constructPath(components, 0),
-                                          constructPath(components, count+1),
-                                          linkTarget);
+        throw new UnresolvedPathException(path, preceding, remainder, target);
       }
       if (lastComp || !curNode.isDirectory()) {
         break;

Modified: hadoop/common/branches/branch-0.22/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.22/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java?rev=1204378&r1=1204377&r2=1204378&view=diff
==============================================================================
--- hadoop/common/branches/branch-0.22/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java (original)
+++ hadoop/common/branches/branch-0.22/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java Mon Nov 21 07:26:56 2011
@@ -1191,10 +1191,6 @@ public class NameNode implements Namenod
   /** @inheritDoc */
   public String getLinkTarget(String path) throws IOException {
     myMetrics.numgetLinkTargetOps.inc();
-    /* Resolves the first symlink in the given path, returning a
-     * new path consisting of the target of the symlink and any 
-     * remaining path components from the original path.
-     */
     try {
       HdfsFileStatus stat = namesystem.getFileInfo(path, false);
       if (stat != null) {

Modified: hadoop/common/branches/branch-0.22/hdfs/src/test/hdfs/org/apache/hadoop/fs/TestFcHdfsSymlink.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.22/hdfs/src/test/hdfs/org/apache/hadoop/fs/TestFcHdfsSymlink.java?rev=1204378&r1=1204377&r2=1204378&view=diff
==============================================================================
--- hadoop/common/branches/branch-0.22/hdfs/src/test/hdfs/org/apache/hadoop/fs/TestFcHdfsSymlink.java (original)
+++ hadoop/common/branches/branch-0.22/hdfs/src/test/hdfs/org/apache/hadoop/fs/TestFcHdfsSymlink.java Mon Nov 21 07:26:56 2011
@@ -20,6 +20,9 @@ package org.apache.hadoop.fs;
 import java.io.*;
 import java.net.URI;
 
+import org.apache.commons.logging.impl.Log4JLogger;
+import org.apache.log4j.Level;
+
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.FileStatus;
 import org.apache.hadoop.fs.FileContext;
@@ -28,9 +31,11 @@ import org.apache.hadoop.fs.permission.F
 import org.apache.hadoop.hdfs.DFSConfigKeys;
 import org.apache.hadoop.hdfs.HdfsConfiguration;
 import org.apache.hadoop.hdfs.MiniDFSCluster;
+import org.apache.hadoop.hdfs.server.namenode.NameNode;
 import org.apache.hadoop.hdfs.protocol.FSConstants;
 import static org.apache.hadoop.fs.FileContextTestHelper.*;
 import org.apache.hadoop.ipc.RemoteException;
+
 import static org.junit.Assert.*;
 import org.junit.Test;
 import org.junit.BeforeClass;
@@ -41,6 +46,10 @@ import org.junit.AfterClass;
  */
 public class TestFcHdfsSymlink extends FileContextSymlinkBaseTest {
 
+  {
+    ((Log4JLogger)NameNode.stateChangeLog).getLogger().setLevel(Level.ALL);
+  }
+
   private static MiniDFSCluster cluster;
   
   protected String getScheme() {
@@ -250,8 +259,8 @@ public class TestFcHdfsSymlink extends F
     Path link = new Path(testBaseDir1(), "symlinkToFile");
     createAndWriteFile(file);
     fc.createSymlink(file, link, false);
-    FileStatus stat_file = fc.getFileStatus(file);
-    FileStatus stat_link = fc.getFileStatus(link);
-    assertEquals(stat_link.getOwner(), stat_file.getOwner());
+    FileStatus statFile = fc.getFileStatus(file);
+    FileStatus statLink = fc.getFileStatus(link);
+    assertEquals(statLink.getOwner(), statFile.getOwner());
   }
 }