lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael McCandless" <luc...@mikemccandless.com>
Subject Fwd: Re: - lock improvement suggestion
Date Fri, 09 Nov 2007 16:47:17 GMT
Are there any objections to adding "throws LockReleaseFailedException"
to Lock.release, and subclassing that exception from IOException?

I think this is a very minor break in backwards compatibility, because
all places that call Lock.release in Lucene already throw IOException
themselves.

The only other option I can see, as Nikolay suggests below, is to
subclass from RuntimeException, but I don't think that's very clean
and really the source of the issue (failed to delete the lock file)
is really most like an IOException.

Mike

"Nikolay Diakov" <nikolay.diakov@fredhopper.com> wrote:
> I see you do the wrapping in a RuntimeException trick. Perhaps you can 
> introduce a special exception derived from RuntimeException that you 
> would throw in that case. It would basically mean "The underlying FS 
> does something we cannot tolerate so we fail fast."
> 
> --Nikolay
> 
> Michael McCandless wrote:
> > I agree, we should not ignore the return value here.  I think throwing an
> > exception if it returns false is the right thing to do?  Though, if it's
> > a checked exception, that's not a backwards compatible change...
> > 
> > Mike
> > 
> > "Nikolay Diakov" <nikolay.diakov@fredhopper.com> wrote:
> >> I have briefly reviewed the SimpleFSLock of Lucene 2.1 and 2.2. I see 
> >> that the lock release mechanism does not check the return value of
> >> delete:
> >>
> >>    public void release() {
> >>      lockFile.delete();
> >>    }
> >>
> >> On most linux-es this can never return false, however under some windows 
> >> FS if someone (a virus scanner) touches the file at the proper 
> >> (improper) time, one may get a delete failure and get a false value. In 
> >> the original code this means that the directory stays locked forever 
> >> (unless someone does double unlocking or until a clearLock from the lock 
> >> factory). For diagnosting purposes, it may be a good idea to throw an 
> >> exception in that case. Alternatively, release() may return a boolean up 
> >> the chain, however this may require more changes in the code using the 
> >> release(). Just a suggestion.
> >>
> >> Cheers,
> >>    Nikolay
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: java-user-unsubscribe@lucene.apache.org
> >> For additional commands, e-mail: java-user-help@lucene.apache.org
> >>
> > 
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: java-user-unsubscribe@lucene.apache.org
> > For additional commands, e-mail: java-user-help@lucene.apache.org
> > 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: java-user-unsubscribe@lucene.apache.org
> For additional commands, e-mail: java-user-help@lucene.apache.org
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org


Mime
View raw message