You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by Shai Erera <se...@gmail.com> on 2010/04/21 17:32:48 UTC
Directory.deleteFile confusingly throws IOException
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
Re: Directory.deleteFile confusingly throws IOException
Posted by Shai Erera <se...@gmail.com>.
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 <ea...@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
> <lu...@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 <se...@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
> >> <lu...@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 <se...@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
> >>>> <lu...@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 <se...@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
>
>
Re: Directory.deleteFile confusingly throws IOException
Posted by Earwin Burrfoot <ea...@gmail.com>.
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
<lu...@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 <se...@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
>> <lu...@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 <se...@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
>>>> <lu...@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 <se...@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
Re: Directory.deleteFile confusingly throws IOException
Posted by Michael McCandless <lu...@mikemccandless.com>.
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 <se...@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
> <lu...@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 <se...@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
>>> <lu...@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 <se...@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
Re: Directory.deleteFile confusingly throws IOException
Posted by Shai Erera <se...@gmail.com>.
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
<lu...@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 <se...@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
>> <lu...@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 <se...@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
Re: Directory.deleteFile confusingly throws IOException
Posted by Michael McCandless <lu...@mikemccandless.com>.
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 <se...@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
> <lu...@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 <se...@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
>>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
Re: Directory.deleteFile confusingly throws IOException
Posted by Shai Erera <se...@gmail.com>.
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 <se...@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
>
>
Re: Directory.deleteFile confusingly throws IOException
Posted by Michael McCandless <lu...@mikemccandless.com>.
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 <se...@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