Grokbase Groups Lucene dev June 2009
FAQ
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

Search Discussions

  • Shai Erera at Jun 25, 2009 at 7:34 pm
    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
    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
  • Michael McCandless at Jun 25, 2009 at 7:51 pm

    On Thu, Jun 25, 2009 at 3:34 PM, Shai Ererawrote:

    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
  • Yonik Seeley at Jun 25, 2009 at 7:56 pm

    On Thu, Jun 25, 2009 at 3:34 PM, Shai Ererawrote:
    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
  • Michael McCandless at Jun 25, 2009 at 7:47 pm
    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 Seeleywrote:
    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
  • Yonik Seeley at Jun 25, 2009 at 7:52 pm

    On Thu, Jun 25, 2009 at 3:47 PM, Michael McCandlesswrote:
    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

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupdev @
categorieslucene
postedJun 25, '09 at 7:09p
activeJun 25, '09 at 7:56p
posts6
users3
websitelucene.apache.org

People

Translate

site design / logo © 2019 Grokbase