On 5/12/06, jason rutherglen <[EMAIL PROTECTED]> wrote:
The locking and referencing is a little bit complex and looks like it can
easily be screwed up. Any hints from Solr dev team on how to pass the existing
into the SolrIndexSearcher?
Yeah, IMO, getSearcher() is the most complex piece of code in Solr...
I reviewed it like 10 times (and found a race condition on the 5th ;-)
_searcher contains the reference to the current searcher, but you need
to grab the searcherLock to make sure that it doesn't go away.
I *think* that the initialization of currSearchHolder could be moved
to earlier in the function.
You just need to make sure that you .decref() it for any possible
error path. Do something like:
Move this to the very top of the function:
RefCounted<SolrIndexSearcher> currSearcherHolder=null;
Then at the end of the first sync block (but still in it) get the
refcounted searcher:
if (_searcher!=null) {
currSearcherHolder=_searcher;
currSearcherHolder.incref();
}
Then get the searcher from the holder where you create the new
searcher, and decref() the currSearchHolder if there are any errors.
SolrIndexSearcher tmp;
try {
if (onDeckSearchers < 1) {
// should never happen... just a sanity check
log.severe("ERROR!!! onDeckSearchers is " + onDeckSearchers);
// reset to 1 (don't bother synchronizing)
onDeckSearchers=1;
} else if (onDeckSearchers > 1) {
log.info("PERFORMANCE WARNING: Overlapping onDeckSearchers=" +
onDeckSearchers);
}
SolrIndexSearcher currSearcher = currSearcherHolder==null ? null
: currSearcherHolder.get(); // NEW CODE
tmp = new SolrIndexSearcher(schema, "main", index_path, true,
currSearcher);
// The new SolrIndexSearcher should *not* hold onto a reference
to currSearcher past it's constructor.
} catch (Throwable th) {
if (currSearcherHolder!=null) currSearchHolder.decref(); // NEW CODE
synchronized(searcherLock) {
onDeckSearchers--;
// notify another waiter to continue... it may succeed
// and wake any others.
searcherLock.notify();
}
throw new RuntimeException(th);
}
-Yonik
