svn commit: r678418 - in /lucene/solr/trunk/src/java/org/apache/solr: core/SolrCore.java handler/component/QueryElevationComponent.java

[yo...@apache.org]

Author: yonik
Date: Mon Jul 21 07:01:34 2008
New Revision: 678418

URL: http://svn.apache.org/viewvc?rev=678418&view=rev
Log:
SOLR-593: getNewestSearcher() to fix inform deadlocks from getSearcher(), fix elevation component not decrementing it's reference

Modified:
    lucene/solr/trunk/src/java/org/apache/solr/core/SolrCore.java
    lucene/solr/trunk/src/java/org/apache/solr/handler/component/QueryElevationComponent.java

Modified: lucene/solr/trunk/src/java/org/apache/solr/core/SolrCore.java
URL: http://svn.apache.org/viewvc/lucene/solr/trunk/src/java/org/apache/solr/core/SolrCore.java?rev=678418&r1=678417&r2=678418&view=diff
==============================================================================
--- lucene/solr/trunk/src/java/org/apache/solr/core/SolrCore.java (original)
+++ lucene/solr/trunk/src/java/org/apache/solr/core/SolrCore.java Mon Jul 21 07:01:34 2008
@@ -70,7 +70,7 @@
   private String name;
   private String logid; // used to show what name is set
   private final CoreDescriptor coreDescriptor;
-  
+
   private final SolrConfig solrConfig;
   private final IndexSchema schema;
   private final String dataDir;
@@ -686,12 +686,21 @@
   // get it (and it will increment the ref count at the same time)
   private RefCounted<SolrIndexSearcher> _searcher;
 
+  // All of the open searchers.  Don't access this directly.
+  // protected by synchronizing on searcherLock.
+  private final LinkedList<RefCounted<SolrIndexSearcher>> _searchers = new LinkedList<RefCounted<SolrIndexSearcher>>();
+
   final ExecutorService searcherExecutor = Executors.newSingleThreadExecutor();
   private int onDeckSearchers;  // number of searchers preparing
   private Object searcherLock = new Object();  // the sync object for the searcher
   private final int maxWarmingSearchers;  // max number of on-deck searchers allowed
 
-
+  /**
+  * Return a registered {@link RefCounted}&lt;{@link SolrIndexSearcher}&gt; with
+  * the reference count incremented.  It <b>must</b> be decremented when no longer needed.
+  * This method should not be called from SolrCoreAware.inform() since it can result
+  * in a deadlock if useColdSearcher==false. 
+  */
   public RefCounted<SolrIndexSearcher> getSearcher() {
     try {
       return getSearcher(false,true,null);
@@ -702,6 +711,28 @@
   }
 
   /**
+  * Return the newest {@link RefCounted}&lt;{@link SolrIndexSearcher}&gt; with
+  * the reference count incremented.  It <b>must</b> be decremented when no longer needed.
+  * If no searcher is currently open, then if openNew==true a new searcher will be opened,
+  * or null is returned if openNew==false.
+  */
+  public RefCounted<SolrIndexSearcher> getNewestSearcher(boolean openNew) {
+    synchronized (searcherLock) {
+      if (_searchers.isEmpty()) {
+        if (!openNew) return null;
+        // Not currently implemented since simply calling getSearcher during inform()
+        // can result in a deadlock.  Right now, solr always opens a searcher first
+        // before calling inform() anyway, so this should never happen.
+        throw new UnsupportedOperationException();
+      }
+      RefCounted<SolrIndexSearcher> newest = _searchers.getLast();
+      newest.incref();
+      return newest;
+    }
+  }
+
+
+  /**
    * Get a {@link SolrIndexSearcher} or start the process of creating a new one.
    * <p>
    * The registered searcher is the default searcher used to service queries.
@@ -810,6 +841,7 @@
 
     RefCounted<SolrIndexSearcher> currSearcherHolder=null;
     final RefCounted<SolrIndexSearcher> newSearchHolder=newHolder(newSearcher);
+
     if (returnSearcher) newSearchHolder.incref();
 
     // a signal to decrement onDeckSearchers if something goes wrong.
@@ -820,6 +852,8 @@
 
       boolean alreadyRegistered = false;
       synchronized (searcherLock) {
+        _searchers.add(newSearchHolder);
+
         if (_searcher == null) {
           // if there isn't a current searcher then we may
           // want to register this one before warming is complete instead of waiting.
@@ -965,6 +999,15 @@
     RefCounted<SolrIndexSearcher> holder = new RefCounted<SolrIndexSearcher>(newSearcher) {
       public void close() {
         try {
+          synchronized(searcherLock) {
+            // it's possible for someone to get a reference via the _searchers queue
+            // and increment the refcount while RefCounted.close() is being called.
+            // we check the refcount again to see if this has happened and abort the close.
+            // This relies on the RefCounted class allowing close() to be called every
+            // time the counter hits zero.
+            if (refcount.get() > 0) return;
+            _searchers.remove(this);
+          }
           resource.close();
         } catch (IOException e) {
           log.severe("Error closing searcher:" + SolrException.toStr(e));

Modified: lucene/solr/trunk/src/java/org/apache/solr/handler/component/QueryElevationComponent.java
URL: http://svn.apache.org/viewvc/lucene/solr/trunk/src/java/org/apache/solr/handler/component/QueryElevationComponent.java?rev=678418&r1=678417&r2=678418&view=diff
==============================================================================
--- lucene/solr/trunk/src/java/org/apache/solr/handler/component/QueryElevationComponent.java (original)
+++ lucene/solr/trunk/src/java/org/apache/solr/handler/component/QueryElevationComponent.java Mon Jul 21 07:01:34 2008
@@ -61,7 +61,9 @@
 import org.apache.solr.schema.FieldType;
 import org.apache.solr.schema.SchemaField;
 import org.apache.solr.search.SortSpec;
+import org.apache.solr.search.SolrIndexSearcher;
 import org.apache.solr.util.VersionedFile;
+import org.apache.solr.util.RefCounted;
 import org.apache.solr.util.plugin.SolrCoreAware;
 import org.apache.solr.request.SolrQueryRequest;
 import org.w3c.dom.Node;
@@ -182,8 +184,14 @@
         }
         else {
           // preload the first data
-          IndexReader reader = core.getSearcher().get().getReader(); 
-          getElevationMap( reader, core );
+          RefCounted<SolrIndexSearcher> searchHolder = null;
+          try {
+            searchHolder = core.getNewestSearcher(true);
+            IndexReader reader = searchHolder.get().getReader();
+            getElevationMap( reader, core );
+          } finally {
+            if (searchHolder != null) searchHolder.decref();
+          }
         }
       }
     }