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 Wed, 21 Apr 2010 17:02:37 GMT
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?
> >
> > Shai
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: dev-help@lucene.apache.org
>
>

Mime
View raw message