lucene-solr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Yonik Seeley" <ysee...@gmail.com>
Subject Re: SolrCore question for updateable indexsearcher
Date Sat, 13 May 2006 01:09:31 GMT
On 5/12/06, jason rutherglen <jasonhusong@yahoo.com> 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

Mime
View raw message