[ 
https://issues.apache.org/jira/browse/HDFS-245?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Eli Collins updated HDFS-245:
-----------------------------

    Attachment: symlink35-hdfs.patch

Hey Sanjay,

Thanks for the review. Latest patch attached, addresses your feedback and 
merges in trunk.

bq. File Jira to resolve dot relative and volume-root relative names locally in 
HDFS (this Jira can be completed later)

Filed HDFS-932.

bq. explicitly declare the unresolvedLinkException  in methods that throw them- 
we are moving towards declaring all exceptions (see HDFS-717). ClientProtocol, 
Namenode, FSNamesystem, etc

Done. I had to modify quite a few functions both in hdfs and common since the 
UnresolvedLinkException flows all the way up to FileContext. I left the tests 
as is, I also didn't modify FileSystem.

bq. UnresolvedPathException, suggestion: adding a method called 
"getUnresolvedPath for completeness.

Done.

bq. createSymlinkXXX -don't pass in permission object as parameter - it  
suggests that permissions matter for symlinks

Done. I left the permissions as default since INodes do not support null 
PermissionStatus or PermissionStatus' created with a null FsPermission. 
Currently links are rwx everyone so access is checked on the link target. And 
like files accessed to the links themselves should be subject to the containing 
directory's permissions. I started to modify TestDFSPermission to cover this, 
which requires converting the test to use FileContext, and just converting it 
to use FileContext caused some of the tests to fail, which I'm looking into, if 
you don't mind will address in a follow-on jira. I filed HDFS-876 for porting 
tests over to FileContext a while back and HDFS-934 to extend those for 
additional coverage after they've been ported. 

bq. FSDirectory#addToParent - if not a symlink, then pass null instead of a 
null string.

The symlink value needs to be a string since we serialize it to image/edit log, 
ie readString and writeString don't support nulls.

bq. getExistingPathINodes() typo: javadoc for param resolveLink is missing the 
param name

Fixed.

bq. Add comment that exception is thrown for symlinks occurs in the middle of 
the path.

Done.

bq. The multiple if statements with same or negation of condition - can be 
confusing and error prone in future

I rewrote this to use just two if that read like the logic: "If the current 
node is a symlink then throw an UnresolvedPathException, unconditionally if we 
are not on the last path component, and conditionally if we are on the last 
path component and we want to resolve it." Think  this is more clear.

{code}
if (curNode.isLink() && (!lastComp || (lastComp && resolveLink))) {
  throw new UnresolvedPathException(...)
}
if (lastComp || !curNode.isDirectory()) {
  break;
}
{code}

bq. FSDirectory#getFileBlocks add: if (symlink) return null.

Fixed. Shouldn't be executed in practice since we always complete a resolved 
path.

bq. FSDirectory#getListing() calls getNode(srcs, false) // shouldn't 
resolveLink be true

Fixed. FileContext#listStatus was not resolving links, fixed that. I added 
FileContextSymlinkBaseTest#testListStatusUsingLink to test this case.

bq.  FSDirectory#getPreferredBlockSize calls getNode(filename, false ) // 
shouldn't resolveLink be true

Fixed. Looks like DFSClient#getBlockSize is the only caller and isn't covered 
by any unit tests (you can remove the method and compile hdfs). Filed HDFS-929 
to either remove it or add a test.

I replaced the throw of UnresolvedPathException in FSNameSystem, it wasn't 
necessary as the call to getFileINode resolves the links. There's now only a 
single place we throw UnresolvedPathException which was my original intent.

Latest patch attached here and to HADOOP-6421.

Thanks,
Eli 

> Create symbolic links in HDFS
> -----------------------------
>
>                 Key: HDFS-245
>                 URL: https://issues.apache.org/jira/browse/HDFS-245
>             Project: Hadoop HDFS
>          Issue Type: New Feature
>            Reporter: dhruba borthakur
>            Assignee: Eli Collins
>         Attachments: 4044_20081030spi.java, designdocv1.txt, designdocv2.txt, 
> designdocv3.txt, HADOOP-4044-strawman.patch, symlink-0.20.0.patch, 
> symlink-25-hdfs.patch, symlink-26-hdfs.patch, symlink-26-hdfs.patch, 
> symLink1.patch, symLink1.patch, symLink11.patch, symLink12.patch, 
> symLink13.patch, symLink14.patch, symLink15.txt, symLink15.txt, 
> symlink16-common.patch, symlink16-hdfs.patch, symlink16-mr.patch, 
> symlink17-common.txt, symlink17-hdfs.txt, symlink18-common.txt, 
> symlink19-common-delta.patch, symlink19-common.txt, symlink19-common.txt, 
> symlink19-hdfs-delta.patch, symlink19-hdfs.txt, symlink20-common.patch, 
> symlink20-hdfs.patch, symlink21-common.patch, symlink21-hdfs.patch, 
> symlink22-common.patch, symlink22-hdfs.patch, symlink23-common.patch, 
> symlink23-hdfs.patch, symlink24-hdfs.patch, symlink27-hdfs.patch, 
> symlink28-hdfs.patch, symlink29-hdfs.patch, symlink29-hdfs.patch, 
> symlink30-hdfs.patch, symlink31-hdfs.patch, symlink33-hdfs.patch, 
> symlink35-hdfs.patch, symLink4.patch, symLink5.patch, symLink6.patch, 
> symLink8.patch, symLink9.patch
>
>
> HDFS should support symbolic links. A symbolic link is a special type of file 
> that contains a reference to another file or directory in the form of an 
> absolute or relative path and that affects pathname resolution. Programs 
> which read or write to files named by a symbolic link will behave as if 
> operating directly on the target file. However, archiving utilities can 
> handle symbolic links specially and manipulate them directly.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to