lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael Busch (JIRA)" <>
Subject [jira] Updated: (LUCENE-743) IndexReader.reopen()
Date Wed, 31 Oct 2007 07:03:50 GMT


Michael Busch updated LUCENE-743:

    Attachment: lucene-743-take4.patch

Patch looks great!  I'm still working through it but found a few small
Thanks Mike! Very good review and feedback!

It might be good to put a "assert refCount > 0" at various places like...
Agreed. I added those asserts to incRef() and decRef(). I didn't add it
to ensureOpen(), because it throws an AlreadyClosedException anyway, and
some testcases check if this exception is thrown.

I'm seeing a failure in contrib/memory testcase:
Oups, I fixed this already. I changed the (deprecated) ctr 
IndexReader.IndexReader(Directory) to call this() which sets the refCount 
to 1. The test passes then. I made this fix yesterday, I think I just 
forgot to update the patch file before I submitted it, sorry about this.

  - In multiple places you catch an IOException and undo the attempted
    re-open, but shouldn't this be a try/finally instead so you also
    clean up on hitting any unchecked exceptions?
Yes of course! Changed it.

  - I think you need an explicit refCount for the Norm class in
OK I see. I made this change as well. I also made the change that
there is no chain, but one starting SegmentReader which all re-opened 
ones reference (see below). Now this starting SegmentReader won't close 
its norms until all other readers that reference it are closed (RC=0),
because only then doClose() is called, which calls closeNorms().
Do you see an easy way how to improve this?
Hmm, probably I have to definalize IndexReader.incRef() and decRef()
and overwrite them in SegmentReader. Then SegmentReader.incRef() would
also incRef the norms, SegmentReader.decref() would decref the norms,
and somehow a clone that shares references the reader but not the norms
(because they changed) would only incref the reader itself, but not
the norms... Or do you see an easier way?

  - If you have a long series of reopens, then, all SegmentReaders in
    the chain will remain alive.  So this is a [small] memory leak
    with time.  I think if you changed referencedSegmentReader to
    always be the *starting* SegmentReader then this chain is broken
Good point. Ok I changed this and also the test cases that check the refCount

> IndexReader.reopen()
> --------------------
>                 Key: LUCENE-743
>                 URL:
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>            Reporter: Otis Gospodnetic
>            Assignee: Michael Busch
>            Priority: Minor
>             Fix For: 2.3
>         Attachments:, lucene-743-take2.patch, lucene-743-take3.patch,
lucene-743-take4.patch, lucene-743.patch, lucene-743.patch, lucene-743.patch,,, varient-no-isCloneSupported.BROKEN.patch
> This is Robert Engels' implementation of IndexReader.reopen() functionality, as a set
of 3 new classes (this was easier for him to implement, but should probably be folded into
the core, if this looks good).

This message is automatically generated by JIRA.
You can reply to this email to add a comment to the issue online.

To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message