Ok, I'm good w/ leaving the IOE. We can wait 'till Lucene moves to Java 7 (2013 ?) and then we'll revisit this :).
Shai On Fri, Apr 23, 2010 at 1:47 PM, Earwin Burrfoot <[email protected]> wrote: > There's also place for alternate Directories, which can throw > readable-loggable exceptions without waiting for nio2. > > On Fri, Apr 23, 2010 at 14:20, Michael McCandless > <[email protected]> wrote: > > Deletion can conceivably fail for a number of interesting reasons :) > > File doesn't exist, permission is denied, file system corruption, some > > kind of temporary resource starvation problem, etc... > > > > And actually it looks like Java 7 (nio.2) has moved to throwing an > > IOException (Path.delete) instead of returning a boolean > > (File.delete): > > > > http://www.baptiste-wicht.com/2010/03/nio-2-path-api-java-7/ > > > > If only nio.2 exposed some way to madvise/posix_fadvise so segment > > merging wouldn't obliterate the IO cache... > > > > Mike > > > > On Thu, Apr 22, 2010 at 11:45 PM, Shai Erera <[email protected]> wrote: > >> I understand your point Mike, but I don't think that returning a > >> boolean will make the Dir API poor. Today boolean is as descriptive as > >> an exception, only much more efficient to handle - given the current > >> Dir impls in Lucene and that we don't think there are many impls out > >> there … > >> > >> Also, I think the Java API makes sense - there cannot be too many > >> reasons for failing to delete a file. So runtime SecurityException > >> (which must be rarerly thrown) + boolean seems fine to me. > >> > >> But really, if you don't think we should change the API, let's drop it. > >> > >> Shai > >> > >> On Thursday, April 22, 2010, Michael McCandless > >> <[email protected]> wrote: > >>> Actually they both seem consistent today? Ie, Directory.deleteFile > >>> returns nothing (void) and throws an IOE if the delete fails. > >>> > >>> Both FSDir and RAMDir throw IOE when the delete fails (yes, RAMDir > >>> throws FNFE, but that's an IOE subclass). > >>> > >>> Just because the java API is poor (returns true or false instead of > >>> throwing specific IOEs) doesn't mean we should make our Directory API > >>> poor? > >>> > >>> And, how IFD deals with files that refuse to be deleted, seems > >>> orthogonal to how Directory.deleteFile conveys the fact that the file > >>> cannot be deleted... > >>> > >>> Mike > >>> > >>> On Wed, Apr 21, 2010 at 1:02 PM, Shai Erera <[email protected]> wrote: > >>>> I think today it's simply inconsistent - RAMDir throws FNFE if the > file does > >>>> not exist (and no other IOE) while FSDir throws IOE for whatever > reason > >>>> File.delete() returned false (not adding any information as to the > cause). > >>>> FSDir cannot do much more than what it does, because File.delete() > does not > >>>> throw any exception, except for the runtime SecurityException, which > is > >>>> ignored (propagated) by FSDir. I've never seen a SecurityException > thrown by > >>>> File.delete() ... > >>>> > >>>> And one can still (like IFD does) call dir.fileExist in case delete > returned > >>>> false to differentiate between "file exists and could not be deleted" > to > >>>> "file does not exist". As one can do w/ File. And then throw a more > >>>> descriptive exception. > >>>> > >>>> Also, I think that given all the current impls don't add any > (concrete) > >>>> information as to why the file was not deleted, I think we should > define the > >>>> proper semantics: "Returns true iff the file was successfully deleted. > If > >>>> false is returned, it is advised to call #fileExists(String) to > >>>> differentiate between a delete failure because the file does not > exist, to > >>>> another failure". > >>>> > >>>> We can keep the 'throws IOException' for "whatever other bad things > >>>> happened", but I'd hate to do that, especially as there are probably > not so > >>>> many Directory implementations out there which can return more > meaningful > >>>> info then true/false. And if we keep the IOE, I think we should do > something > >>>> in IFD rather than swallow the exception and retry over if the file > exists > >>>> ... currently the code assumes the exception is temporary, which may > not be > >>>> the case w/ external Dir impls. And if we fix that ... well we need to > fix > >>>> 'deletePendingFiles' as well ... this becomes a mess. > >>>> > >>>> What do you think? > >>>> > >>>> Shai > >>>> > >>>> On Wed, Apr 21, 2010 at 7:01 PM, Michael McCandless > >>>> <[email protected]> wrote: > >>>>> > >>>>> I agree clarity is important so we should tighten up this spec. > >>>>> > >>>>> But, don't we potentially lose information if we just return true or > >>>>> false? (Ie why the deletion failed). Failing because of FNFE vs a > >>>>> "permission denied" or filesystem corruption are very different. > But, > >>>>> then, our hands are tied anyway since File.delete returns boolean... > >>>>> so maybe we should simply return a boolean...? > >>>>> > >>>>> Mike > >>>>> > >>>>> On Wed, Apr 21, 2010 at 11:32 AM, Shai Erera <[email protected]> > wrote: > >>>>> > Hi > >>>>> > > >>>>> > While working on LUCENE-2402, I've noticed what I think is a > confusing > >>>>> > behavior of Dir.deleteFile. Its signature declares throwing an > >>>>> > IOException, > >>>>> > but w/ no documentation to when and why will this be thrown. And > then of > >>>>> > course there are the two different implementations by FSDir and > RAMDir: > >>>>> > FSDir throws an IOException if File.delete() returns false while > RAMDir > >>>>> > throws FNFE if the file does not exist. > >>>>> > In either case, an IOE is not thrown from the lower-level API (File > in > >>>>> > FSDir > >>>>> > case). > >>>>> > > >>>>> > Then, IFD.deleteFile declares "throws IOException", but never > really > >>>>> > throws > >>>>> > it. Instead, it calls directory.deleteFile(), catches IOE, and > calls > >>>>> > dir.fileExists. If the latter returns true it adds the file to the > list > >>>>> > of > >>>>> > pending to delete files, otherwise simply ignores the exception > (!?). > >>>>> > > >>>>> > My feeling is that this exception should not be declared on > Directory, > >>>>> > but > >>>>> > rather have deleteFile return true or false (like Java's File). In > both > >>>>> > current implementations, it's not a real IO error, and if there is > any > >>>>> > custom Dir impl out there, which really throws IOE, then IFD > clearly > >>>>> > ignores > >>>>> > it, and will try to delete the file again next time. > >>>>> > > >>>>> > So it's more that Dir.deleteFile is confusing, than IFD is broken. > And I > >>>>> > think clarity is important. > >>>>> > > >>>>> > What do you think? > >>>>> > > >>> > >> > >> --------------------------------------------------------------------- > >> To unsubscribe, e-mail: [email protected] > >> For additional commands, e-mail: [email protected] > >> > >> > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: [email protected] > > For additional commands, e-mail: [email protected] > > > > > > > > -- > Kirill Zakharenko/Кирилл Захаренко ([email protected]) > Home / Mobile: +7 (495) 683-567-4 / +7 (903) 5-888-423 > ICQ: 104465785 > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > >
