lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Shai Erera <ser...@gmail.com>
Subject Re: Directory.deleteFile confusingly throws IOException
Date Fri, 23 Apr 2010 10:50:38 GMT
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 <earwin@gmail.com> 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
> <lucene@mikemccandless.com> 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 <serera@gmail.com> 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
> >> <lucene@mikemccandless.com> 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 <serera@gmail.com> 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
> >>>> <lucene@mikemccandless.com> 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 <serera@gmail.com>
> 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: dev-unsubscribe@lucene.apache.org
> >> For additional commands, e-mail: dev-help@lucene.apache.org
> >>
> >>
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> > For additional commands, e-mail: dev-help@lucene.apache.org
> >
> >
>
>
>
> --
> Kirill Zakharenko/Кирилл Захаренко (earwin@gmail.com)
> Home / Mobile: +7 (495) 683-567-4 / +7 (903) 5-888-423
> ICQ: 104465785
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: dev-help@lucene.apache.org
>
>

Mime
View raw message