[
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,
