|
[
Permlink
| « Hide
]
dhruba borthakur added a comment - 28/Mar/08 06:57 PM
This does not seem to be a regression and I propose that we fix it for 0.17.
Is it likely to be fixed in 0.18.x ?
– This seems already fixed in trunk. At least, the behavior should be different since I could not find "Could not get listing " in the ls codes. I will check this in details.
> This seems already fixed in trunk...
Unfortunately, we still have this bug but the error message have been changed. "lsr /", then remove /a right before lsr printing the result of /a bash-3.2$ ./bin/hadoop fs -lsr / drwxr-xr-x - tsz supergroup 0 2008-11-05 14:01 /a lsr: hdfs://namenode9000/a: No such file or directory. I plan to change lsr such that lsr prints an error message and then keep listing other items. 3121_20081105.patch: print an error message and then keep listing other items, instead of throwing an exception and quit.
Same as before: "lsr /", then remove /a right before lsr printing the result of /a bash-3.2$ ./bin/hadoop fs -lsr / drwxr-xr-x - tsz supergroup 0 2008-11-05 14:18 /a hdfs://namenode:9000/a: No such file or directory. drwxr-xr-x - tsz supergroup 0 2008-11-05 14:18 /b ... Shouldn't it be the same for other exceptions? What if dir '/a' does not have permissions for this user? > Shouldn't it be the same for other exceptions? What if dir '/a' does not have permissions for this user?
Since this issue is target for 0.18, I don't want to introduce big changes. How about we fix other exceptions in a separated issue? Then I don't think it is a proper fix for the real issue. It fixes just one symptom reported.
> How about we fix other exceptions in a separated issue? Raghu, I see your point now. Here is a new patch.
3121_20081106.patch: try-catch all IOException. Couple of comments:
I suggest renaming it to some thing like 'shellListStatus()' 3121_20081106b.patch: renamed getFileStatus(...) to shellListStatus(...)
Thanks Nicholas. looks good. One nit:
instead of ls throwing exception with "There were totally..." message, I think it is better to make ls return -1 and replace ls(..) with ' exitCode = ls() ' in doall(). > instead of ls throwing exception with "There were totally..." message, I think it is better to make ls return -1 and replace ls(..) with ' exitCode = ls() ' in doall().
I am not sure what do you mean. Could you post a patch for this? I hope attached patch clarifies what I mean. It behaves same as before and does not add extra line of output that is unecessary.
For e.g. compare 'bin/hadoop fs -ls /other' :
I don't think most command print extra lines like 'ls: There were totaly 1 exception(s)' at the end..' +1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12393479/HADOOP-3121.patch against trunk revision 712033. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 Eclipse classpath. The patch retains Eclipse classpath integrity. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3545/testReport/ This message is automatically generated. > I don't think most command print extra lines like 'ls: There were totaly 1 exception(s)' at the end..'
I agree that the extra line is odd. Thanks. 3121_20081107.patch: We should also change FsShell.run(...) to update exitCode. ant test-patch
[exec] +1 overall.
[exec] +1 @author. The patch does not contain any @author tags.
[exec] +1 tests included. The patch appears to include 3 new or modified tests.
[exec] +1 javadoc. The javadoc tool did not generate any warning messages.
[exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings.
[exec] +1 findbugs. The patch does not introduce any new Findbugs warnings.
[exec] +1 Eclipse classpath. The patch retains Eclipse classpath integrity.
The patch passed all unit tests locally.
3121_20081107_0.18.patch: for 0.18. It also passed all unit tests locally.
I just committed this.
Integrated in Hadoop-trunk #655 (See http://hudson.zones.apache.org/hudson/job/Hadoop-trunk/655/
. lsr should keep listing the remaining items but not terminate if there is any IOException. (szetszwo) |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||