Details
-
Type:
Bug
-
Status: Closed
-
Priority:
Critical
-
Resolution: Fixed
-
Affects Version/s: 0.23.0, 2.0.0-alpha, 3.0.0-alpha1
-
Fix Version/s: 2.0.3-alpha, 0.23.5
-
Component/s: fs
-
Labels:None
-
Target Version/s:
Description
Let's say we have have a structure of "$date/$user/stuff/file". Multiple globs are unreliable unless every directory in the structure exists.
These work:
date*/user
date*/user/stuff
date*/user/stuff/file
These fail:
date*/user/*
date*/user//
date*/user/stu*
date*/user/stu*/*
date*/user/stu*/file
date*/user/stuff/*
date*/user/stuff/f*
Issue Links
- breaks
-
MAHOUT-1142
TestDistributedLanczosSolverCLI.testDistributedLanczosSolverEVJCLI is broken with Hadoop 0.23.5
-
- Closed
-
- is duplicated by
-
HADOOP-8845
When looking for parent paths info, globStatus must filter out non-directory elements to avoid an AccessControlException
-
- Resolved
-
Activity
- All
- Comments
- Work Log
- History
- Activity
- Transitions
There's more multi-glob bugginess. Given /file, and /dir/file:
- /*/file will fail with a permission denied because it tries to treat /file as a directory since it matches the first "*".
- Worse yet, /*/file* will return BOTH /file and /dir/file!
This appears to be the unfortunate fallout of the change to make listStatus throw an exception when the path doesn't exist.
Here's what I think will work for multi-level globs. Haven't checked the other issues in the comments yet.
It doesn't fix all cases though... There's another place.
The current behavior does seem kind of... odd.
cmccabe@keter:/h> ./bin/hadoop fs -mkdir -p /a/b/c cmccabe@keter:/h> ./bin/hadoop fs -ls '/*' Found 1 items drwxr-xr-x - cmccabe supergroup 0 2012-10-10 10:43 /a/b
I think we need a good unit test for this more than anything else.
The current behavior does seem kind of... odd.
Agreed it does some odd things, but actually the example you gave is exactly what I would expect since UN*X shells do the same thing. The wildcard expands to match 'a' and then the 'a' directory is passed as an argument to the 'ls' command.
$ ls ./* ls: cannot access ./*: No such file or directory $ mkdir -p a/b/c $ ls ./* b
The UNIX ls command has a syntax that's more familiar:
ls -l /*
/usr:
total 276
drwxr-xr-x 2 root root 77824 Oct 9 11:14 bin
drwxr-xr-x 92 root root 20480 Sep 25 10:49 lib
drwxr-xr-x 165 root root 118784 Oct 9 11:14 lib64
...
/var:
total 56
...
But basically, you're right... this behavior is fine and expected (unlike the behavior described in this JIRA.)
I fixed all the cited issues. I don't recommend trying to read the raw diffs. Look the old and revised methods side by side.
Yes, FsShell's ls is working as expected, but it's output format leaves quite a bit to be desired. It's a hybrid of "ls" and "find" commands. One of the many things I intend to do is add a "-l" flag that mimics the real ls, but never seem to find the time in part because of nasty bugs like this. 
Putting this in Patch Available for Jenkins run.
The changes look good, but I think there's a lingering bug wrt. how GlobExpander's results are handled. For example, I would expect the following test to pass:
assertEquals(0, fs.globStatus(new Path("/{nonexistent1/a,nonexistent2/b}")).length);
However it fails because globStatus is returning null instead of an empty array. FsShell relies on the fact that globStatus will never return null for paths that contain pattern characters, and we could end up creating paths with pattern characters. I checked trunk, and it turns out this test crashes globStatus internally. Not crashing is nice, but we shouldn't be reporting null for paths with pattern characters. Fortunately the fix is pretty easy. We just need to check if GlobExpander returned more than one result which implies patterns were used and we should convert a result of null into an empty array in that case.
Good catch, Jason. I now return an empty list (glob) if there are no matches but the glob expander returned multiple patterns.
While testing, I discovered that I "accidentally" fixed a NPE in the old code when the expander returned multiple parts, but one of them doesn't match. The old code tried to iterate over the null.
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12548616/HADOOP-8906.patch
against trunk revision .
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 1 new or modified test files.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 eclipse:eclipse. The patch built with eclipse:eclipse.
-1 findbugs. The patch appears to introduce 1 new Findbugs (version 1.3.9) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
-1 core tests. The patch failed these unit tests in hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs:
org.apache.hadoop.fs.TestFsShellCopy
org.apache.hadoop.hdfs.TestDFSShell
org.apache.hadoop.cli.TestHDFSCLI
+1 contrib tests. The patch passed contrib unit tests.
Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1601//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/1601//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-common.html
Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1601//console
This message is automatically generated.
Not sure if Jason's comment is addressed in the new patch. +1 with that addressed.
BTW the findbugs seems to be related to this patch.
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12548634/HADOOP-8906.patch
against trunk revision .
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 1 new or modified test files.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 eclipse:eclipse. The patch built with eclipse:eclipse.
-1 findbugs. The patch appears to introduce 1 new Findbugs (version 1.3.9) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
-1 core tests. The patch failed these unit tests in hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs:
org.apache.hadoop.fs.TestFsShellCopy
org.apache.hadoop.hdfs.TestDFSShell
org.apache.hadoop.cli.TestHDFSCLI
+1 contrib tests. The patch passed contrib unit tests.
Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1603//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/1603//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-common.html
Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1603//console
This message is automatically generated.
A Path from Path.CUR_DIR is internally an empty string, which the Path ctors won't allow so had to ensure to use Path.CUR_DIR when the path is empty.
Fixed issue where globbing failed unexpectedly when the starting directory doesn't exist - ex. relative path and home dir doesn't exist. Added exception handler. Had to pre-compute if globs are present before attempting lookup in order to return the correct value.
Thanks for the updates, Daryn. I'm eager to see the Jenkins results, although it seems the Jenkins build is stuck right now.
After a closer look, I'm wondering if there's one more subtle difference between the old and new versions, this time when the user specifies a filter. I think the old version will return null in the case of an non-globbed path that found a file but it didn't pass the specified filter. It applies the filter as it searches before it checks for the empty-array-should-return-null case. In the new version, it applies the specified filter after it checks for whether there are matches, and in this scenario there will be a match since the filter hasn't been applied yet. I'm wondering if we should apply the filter before checking for an empty match array to align with the old behavior. Granted it's odd to provide a non-globbed path that wouldn't pass the filter, so it's a corner-case.
Otherwise everything looks great, pending Jenkins since it found some things in the last run that we missed.
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12548681/HADOOP-8906.patch
against trunk revision .
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 1 new or modified test files.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 eclipse:eclipse. The patch built with eclipse:eclipse.
+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
-1 core tests. The patch failed these unit tests in hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs:
org.apache.hadoop.hdfs.server.namenode.ha.TestStandbyCheckpoints
+1 contrib tests. The patch passed contrib unit tests.
Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1607//testReport/
Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1607//console
This message is automatically generated.
Address corner case where a custom filter of a can return the wrong result. If the glob has no pattern, do just one stat instead of all the components.
patch for branch 0.23
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12548747/HADOOP-8906-branch_0.23.patch
against trunk revision .
-1 patch. The patch command could not apply the patch.
Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1612//console
This message is automatically generated.
The corner case is now handled, but the new tests added don't test for it. There should be a test for a non-globbed path for an existing file with the false filter, and all the false filter tests check for either a globbed path or non-existent files.
I tried adding a test locally with the false filter for "/" and noticed that it didn't return null. Instead it returned "/" because the filter isn't applied in the special cases of "/" and "", which seems wrong. It turns out that the existing code also had this bug, so I suppose it's at least consistent with the previous version's behavior.
Yes, I had that case but somehow accidentally removed in the final patch.
The case of a non-glob path with a user-supplied filter is an interesting one. "null" means the path isn't a glob AND doesn't exist. When an existing non-glob path is removed by the filter, then arguably maybe it should return empty array since it's not that the path doesn't exist but that the filter had no matches. In essence, perhaps a user filter means the query is always a glob? I can see it going either way.
In essence, perhaps a user filter means the query is always a glob? I can see it going either way.
Yes, I thought about that as well. Maybe it would be more consistent to return empty instead of null in that case, but I was erring on the side of caution to maintain compatibility with the previous version's behavior. It all comes down to what a result of null really means. If it's being used to check for globs in the path then arguably we should continue to return null because someone could be using/abusing globStatus(path, falseFilter) to check for globs in a path even if the path exists in the filesystem.
+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12548745/HADOOP-8906.patch
against trunk revision .
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 1 new or modified test files.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 eclipse:eclipse. The patch built with eclipse:eclipse.
+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
+1 core tests. The patch passed unit tests in hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs.
+1 contrib tests. The patch passed contrib unit tests.
Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1611//testReport/
Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1611//console
This message is automatically generated.
Add more tests. As cited by Jason, return null for non-glob queries that filter out all results.
+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12548911/HADOOP-8906.patch
against trunk revision .
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 1 new or modified test files.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 eclipse:eclipse. The patch built with eclipse:eclipse.
+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
+1 core tests. The patch passed unit tests in hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs.
+1 contrib tests. The patch passed contrib unit tests.
Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1622//testReport/
Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1622//console
This message is automatically generated.
+1, thanks Daryn. I'll commit this shortly.
Integrated in Hadoop-Hdfs-trunk-Commit #2920 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2920/)
HADOOP-8906. paths with multiple globs are unreliable. Contributed by Daryn Sharp. (Revision 1397704)
Result = SUCCESS
jlowe : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1397704
Files :
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java
- /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/TestGlobPaths.java
Integrated in Hadoop-Common-trunk-Commit #2858 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/2858/)
HADOOP-8906. paths with multiple globs are unreliable. Contributed by Daryn Sharp. (Revision 1397704)
Result = SUCCESS
jlowe : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1397704
Files :
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java
- /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/TestGlobPaths.java
I've committed this to trunk, branch-2, and branch-0.23.
Integrated in Hadoop-Mapreduce-trunk-Commit #2881 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2881/)
HADOOP-8906. paths with multiple globs are unreliable. Contributed by Daryn Sharp. (Revision 1397704)
Result = FAILURE
jlowe : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1397704
Files :
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java
- /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/TestGlobPaths.java
Integrated in Hadoop-Hdfs-0.23-Build #403 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/403/)
HADOOP-8906. paths with multiple globs are unreliable. Contributed by Daryn Sharp. (Revision 1397709)
Result = SUCCESS
jlowe : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1397709
Files :
- /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt
- /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java
- /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/TestGlobPaths.java
Integrated in Hadoop-Hdfs-trunk #1194 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1194/)
HADOOP-8906. paths with multiple globs are unreliable. Contributed by Daryn Sharp. (Revision 1397704)
Result = SUCCESS
jlowe : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1397704
Files :
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java
- /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/TestGlobPaths.java
Integrated in Hadoop-Mapreduce-trunk #1225 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1225/)
HADOOP-8906. paths with multiple globs are unreliable. Contributed by Daryn Sharp. (Revision 1397704)
Result = SUCCESS
jlowe : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1397704
Files :
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java
- /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/TestGlobPaths.java
It appears that after a glob, all the intervening path components before the next glob must exist.