You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by Yonik Seeley <yo...@lucidimagination.com> on 2009/06/25 21:09:32 UTC
Is close() correct?
Are the semantics of close() really correct when reopen() is used?
public final synchronized void close() throws IOException {
if (!closed) {
decRef();
closed = true;
}
}
Solr has the following code:
IndexReader newReader = currentReader.reopen();
if (newReader == currentReader) {
currentReader.incRef();
}
reopen() used to return the same instance if no changes had been made
- that makes sense.
But then of you do a close on both the currentReader and newReader,
only one decRef() will be called!
The reopen() javadoc suggests this has not changed:
* If the index has not changed since this instance was (re)opened, then this
* call is a NOOP and returns this instance.
Seems like the "closed" variable just be eliminated completely?
Throwing an exception on too many closes (rather than silently
ignoring) would probably be doing people a favor.
-Yonik
http://www.lucidimagination.com
---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
Re: Is close() correct?
Posted by Yonik Seeley <yo...@lucidimagination.com>.
On Thu, Jun 25, 2009 at 3:47 PM, Michael
McCandless<lu...@mikemccandless.com> wrote:
> Since close() is just a protected ("only once") call to decRef(), you
> could simply always call decRef() and never close()?
Yep, just re-read the javadoc to decRef()... as long as it stays that
way, solr will just avoid close().
I'll open a solr issue to fix.
-Yonik
htp://www.lucidimagination.com
---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
Re: Is close() correct?
Posted by Michael McCandless <lu...@mikemccandless.com>.
This came up a while back and we decided (then) that double-closing
should be allowed:
http://issues.apache.org/jira/browse/LUCENE-818?focusedCommentId=12477399&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#action_12477399
I think if we changed this ("every close counts") we'd break a number
of apps.
Since close() is just a protected ("only once") call to decRef(), you
could simply always call decRef() and never close()?
Mike
On Thu, Jun 25, 2009 at 3:09 PM, Yonik Seeley<yo...@lucidimagination.com> wrote:
> Are the semantics of close() really correct when reopen() is used?
>
> public final synchronized void close() throws IOException {
> if (!closed) {
> decRef();
> closed = true;
> }
> }
>
> Solr has the following code:
> IndexReader newReader = currentReader.reopen();
> if (newReader == currentReader) {
> currentReader.incRef();
> }
>
> reopen() used to return the same instance if no changes had been made
> - that makes sense.
> But then of you do a close on both the currentReader and newReader,
> only one decRef() will be called!
>
> The reopen() javadoc suggests this has not changed:
> * If the index has not changed since this instance was (re)opened, then this
> * call is a NOOP and returns this instance.
>
> Seems like the "closed" variable just be eliminated completely?
> Throwing an exception on too many closes (rather than silently
> ignoring) would probably be doing people a favor.
>
> -Yonik
> http://www.lucidimagination.com
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: java-dev-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
Re: Is close() correct?
Posted by Michael McCandless <lu...@mikemccandless.com>.
On Thu, Jun 25, 2009 at 3:34 PM, Shai Erera<se...@gmail.com> wrote:
> Doesn't reopen increase the refCount already? why do you call incRef()?
If you get the same reader back, the refCount is unchanged. If you
get a new reader back, it returns a reference (ie, refCount is 1).
Mike
---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
Re: Is close() correct?
Posted by Yonik Seeley <yo...@lucidimagination.com>.
On Thu, Jun 25, 2009 at 3:34 PM, Shai Erera<se...@gmail.com> wrote:
> Funny, we're just having a similar discussion on LUCENE-1707.
Not a coincidence ;-) I attempt to at least skim all the threads I
have no bandwidth to keep up with... LUCENE-1707 jiggled a neuron,
but this didn't seem to be part of that issue - hence the separate
email.
-Yonik
http://www.lucidimagination.com
---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
Re: Is close() correct?
Posted by Shai Erera <se...@gmail.com>.
Funny, we're just having a similar discussion on LUCENE-1707. I think the
purpose of closed is to save against calling close() twice w/o "knowing any
better". I.e, IndexWriter.ReadersPool's release() both call decRef() (to
decrease its own ref) and close() to decrease the caller's. But then the
caller might want to call close() too, since it doesn't know the reader has
been closed. I can imagine other situations like that. TestIndexReaderReopen
will hit it, and fail (that's what I tried in 1707).
Doesn't reopen increase the refCount already? why do you call incRef()?
Shai
On Thu, Jun 25, 2009 at 10:09 PM, Yonik Seeley
<yo...@lucidimagination.com>wrote:
> Are the semantics of close() really correct when reopen() is used?
>
> public final synchronized void close() throws IOException {
> if (!closed) {
> decRef();
> closed = true;
> }
> }
>
> Solr has the following code:
> IndexReader newReader = currentReader.reopen();
> if (newReader == currentReader) {
> currentReader.incRef();
> }
>
> reopen() used to return the same instance if no changes had been made
> - that makes sense.
> But then of you do a close on both the currentReader and newReader,
> only one decRef() will be called!
>
> The reopen() javadoc suggests this has not changed:
> * If the index has not changed since this instance was (re)opened, then
> this
> * call is a NOOP and returns this instance.
>
> Seems like the "closed" variable just be eliminated completely?
> Throwing an exception on too many closes (rather than silently
> ignoring) would probably be doing people a favor.
>
> -Yonik
> http://www.lucidimagination.com
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: java-dev-help@lucene.apache.org
>
>