|
[
Permlink
| « Hide
]
dhruba borthakur added a comment - 16/May/08 11:46 PM
This is a first version of the patch that still has some debugging messages. Early code review comments welcome. This introduces a new FileSystem API.
Incorporated Nicholas's comments. For now, I purposely kept the name deleteOnExit while making the javadoc explicitly state that the file will be deleted when either the JVM exits or the FileSystem is closed. I did this to maintain some sort of uniformity with the java File.deleteOnExit() API. But, if more people feel strongly about renaming this API as deleteOnClose, i will be happy do do that change.
I also kept the default behaviour to be recursive. I enhanced the unit test to test for cases when deleteOnExit is called before/after the file is closed. Also tests the case when the filesystem is closed without closing the file. Also, test on LocalFileSystem as well as HDFS. +1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12382258/deleteOnExit.patch against trunk revision 656939. +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 release audit. The applied patch does not increase the total number of release audit warnings. +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/2497/testReport/ This message is automatically generated. +1 patch looks good. Two minor comments:
Incorporated Nicholas' comments,
Incorporated Nicholas' code review comments.
Incorporated Nicholas' code review comments.
While useful, this code is built on the assumptions that GC/shutdown happens cleanly,. and that when it does, the filesystem is visible. A long-lived filesystem will still pick up lots of orphan temp files.
What may be (eventually) useful would be the ability to give files an expiry time in their metadata, and have something intermittently walk the FS, deleting files that have expired. > What may be (eventually) useful would be the ability to give files an expiry time in their metadata, and have something intermittently walk the FS, deleting files that have expired.
The trash mechanism already supports that. If you move something to a directory named .Trash/0807040000 in your home directory then HDFS (if configured with a non-zero fs.trash.interval) will remove it after the 4th of July. The API doesn't currently provide explicit support for this, but it would be easy to add a removeAfter(Path, Date) method. Re-submitting patch to trigger Hudson QA tests.
Re-submitting patch to trigger Hudson QA tests.
Responding to Steve' comments: I agree that deleteOnExit helps only if the JVM shuts down cleanly and the filesystem is visible. A big percentage of our use cases actually fall in this category. I would like the deleteOnExit API to be part of the 0.18 release and following that, start discussion on the removeAfter() API proposed by Doug.
+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12382350/deleteOnExit.patch against trunk revision 658862. +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 release audit. The applied patch does not increase the total number of release audit warnings. +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/2510/testReport/ This message is automatically generated. Integrated in Hadoop-trunk #499 (See http://hudson.zones.apache.org/hudson/job/Hadoop-trunk/499/
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||