Apologies for a long silence - have been fighting a nasty bug of a non-computer 
variety.

We are using 3.0.3. I was trying to upgrade to 3.1 and clear all deprecation 
warnings. What I described was an attempt to switch from MultiSearcher to 
MultiReader. We had a simple delegating IndexSearcher wrapper for cached/reused 
IndexSearchers that was doing reference counting. Its close() method was 
overridden to perform a real close() only if ref count was 0. I could have any 
combination of single-use IndexSearchers and shared/cached IndexSearchers in a 
MultiSearcher and everything worked beautifully on close().

So, my first choice was to use the incRef()/close() in IndexReader, but 
IndexReader.close() behavior prevents that.

The second thought was creating a delegating wrapper for IndexReader similarly 
to what I had for IndexSearcher but there are so many methods, and half of them 
are final including close(). So, that doesn't work either.

I don't think I can let MultiReader call both incRef() and decRef() on sub 
readers for two reasons: single-use (non-shared) readers need to just be 
closed; and for shared readers - there is a race condition between getting a 
shared reader from cache and calling incRef(). There is a potential that during 
that time the reader may get decRef()'ed and closed due to, e.g. a new version 
of an index or cache expiration. So, a shared reader has to be incRef()'ed at 
the time I get it from cache.

Finally, I ended up creating a simple container interface to house the 
IndexReader and pass it around:

public interface IndexReaderContainer {
        IndexReader getIndexReader();
        void close() throws IOException;
}

I have 3 implementations of this interface: 
  single-use - close() simply closes IndexReader, 
  shared - does reference counting and closes IndexReader only when ref count = 
0, and 
  multi-reader - keeps track of sub-IndexReaderContainer's and closes each one 
on close(). MultiReader is created with closeSubReaders=false.

The application code always calls IndexReaderContainer.close(), never 
IndexReader.close() directly.

Not very pretty but this way I have my combination of single-use and shared 
indexes, and I don't have to change Lucene code.

Personally, I have no sympathy for people who double-close their IndexReaders - 
let them get an exception, but I understand that it is not easy to change 
something that's been working in a particular way for years.

Thanks,

Alexey

-----Original Message-----
From: Michael McCandless [mailto:luc...@mikemccandless.com] 
Sent: Tuesday, April 26, 2011 11:20 PM
To: java-user@lucene.apache.org
Subject: Re: IndexReader.close() behavior

The code is tricky, but it's intentional.

We always set closed to true to guard against double close, ie, it's
fine to double-close an IndexReader, ie doing so will not "steal"
references from other places that have incRef'd the reader.

Can you pass closeSubReaders=false when you create your MultiReader?
This way, the MultiReader incRefs on init and decRefs on close.

But: what Lucene version are you using...?

Mike

http://blog.mikemccandless.com

On Tue, Apr 26, 2011 at 8:46 AM, Alexey Lef <ale...@sciquest.com> wrote:
> This is the code in IndexReader.close():
>
>  public final synchronized void close() throws IOException {
>    if (!closed) {
>      decRef();
>      closed = true;
>    }
>  }
>
> What strikes me as odd is that “closed” variable is set to true regardless of 
> whether the index was actually closed using doClose(). It is causing a bit of 
> a problem in the following setup.
>
> We have many indexes. Some of these indexes are kept open and IndexReader is 
> reused for performance reasons, some are open just for a single search. Some 
> of them are combined together at the search time depending on the user 
> performing the search. For every search, if there are multiple relevant 
> IndexReader, we create a new MultiReader instance and close it once the 
> search is done. Since reference counting is already built into the 
> IndexReader, I figured I can just call incRef() when using a cached 
> IndexReader and then call close() or let MultiReader call close() on it once 
> it is done. The problem is only the first close() calls decRef(),subsequent 
> calls do nothing, and if there were multiple incRef() calls, the IndexReader 
> ends up being left open.
>
> I thought of using decRef() instead of close() but MultiReader doesn’t have 
> an option of calling decRef() on subreaders, only close().
>
> The following change to IndexReader.close() fixes the problem:
>
>  public final synchronized void close() throws IOException {
>    if (refCount.get() > 0) {
>      decRef();
>    }
>  }
>
> It seems more logical. Also it is now consistent with ensureOpen() method.
>
> Am I missing something here? Is there a good reason to prevent close() from 
> doing anything after the first call even if the IndexReader was not actually 
> closed? Does it even need to be synchronized if decRef() does all the work?
>
> Thanks,
>
> Alexey
>
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: java-user-unsubscr...@lucene.apache.org
For additional commands, e-mail: java-user-h...@lucene.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: java-user-unsubscr...@lucene.apache.org
For additional commands, e-mail: java-user-h...@lucene.apache.org

Reply via email to