Index: solr/core/src/test/org/apache/solr/search/TestIndexSearcher.java
===================================================================
--- solr/core/src/test/org/apache/solr/search/TestIndexSearcher.java	(revision 1573023)
+++ solr/core/src/test/org/apache/solr/search/TestIndexSearcher.java	(working copy)
@@ -105,8 +105,10 @@
     int baseRefCount = rCtx3.reader().getRefCount();
     assertEquals(1, baseRefCount);
 
-    assertU(commit());
+    assertU(commit()); // nothing has changed
     SolrQueryRequest sr4 = req("q","foo");
+    assertSame("nothing changed, searcher should be the same",
+               sr3.getSearcher(), sr4.getSearcher());
     IndexReaderContext rCtx4 = sr4.getSearcher().getTopReaderContext();
 
     // force an index change so the registered searcher won't be the one we are testing (and
@@ -114,9 +116,9 @@
     assertU(adoc("id","7", "v_f","7574"));
     assertU(commit()); 
 
-    // test that reader didn't change (according to equals at least... which uses the wrapped reader)
-    assertEquals(rCtx3.reader(), rCtx4.reader());
-    assertEquals(baseRefCount+1, rCtx4.reader().getRefCount());
+    // test that reader didn't change
+    assertSame(rCtx3.reader(), rCtx4.reader());
+    assertEquals(baseRefCount, rCtx4.reader().getRefCount());
     sr3.close();
     assertEquals(baseRefCount, rCtx4.reader().getRefCount());
     sr4.close();
Index: solr/core/src/test/org/apache/solr/search/TestSearcherReuse.java
===================================================================
--- solr/core/src/test/org/apache/solr/search/TestSearcherReuse.java	(revision 0)
+++ solr/core/src/test/org/apache/solr/search/TestSearcherReuse.java	(revision 0)
@@ -0,0 +1,164 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.search;
+
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.request.SolrQueryRequest;
+
+import org.apache.lucene.util.TestUtil;
+
+import org.junit.BeforeClass;
+
+/**
+ * Requests to open a new searcher w/o any underlying change to the index exposed 
+ * by the current searcher should result in the same seracher being re-used.
+ *
+ * Likewise, if there <em>is</em> in fact an underlying index change, we want to 
+ * assert that a new searcher will in fact be opened.
+ */
+public class TestSearcherReuse extends SolrTestCaseJ4 {
+
+  @BeforeClass
+  public static void beforeClass() throws Exception {
+    initCore("solrconfig.xml","schema.xml");
+  }
+
+  @Override
+  public void tearDown() throws Exception {
+    super.tearDown();
+    assertU(delQ("*:*"));
+    optimize();
+    assertU(commit());
+  }
+
+  public void test() throws Exception {
+
+    // seed some docs & segments
+    int numDocs = atLeast(1);
+    for (int i = 1; i <= numDocs; i++) {
+      // NOTE: starting at "1", we'll use id=0 later
+      assertU(adoc("id", ""+i, "v_s", TestUtil.randomSimpleString(random(),3,10)));
+      if (random().nextBoolean()) {
+        assertU(commit());
+      }
+    }
+    assertU(commit());
+
+    // seed a single query into the cache
+    assertQ(req("*:*"), "//*[@numFound='"+numDocs+"']");
+
+    final SolrQueryRequest baseReq = req("q","foo");
+    try {
+      // we make no index changes in this block, so the searcher should always be the same
+      // NOTE: we *have* to call getSearcher() in advance, it's a delayed binding
+      final SolrIndexSearcher expectedSearcher = baseReq.getSearcher();
+
+      assertU(commit());
+      assertSearcherHasNotChanged(expectedSearcher);
+
+      assertU(commit("openSearcher","true"));
+      assertSearcherHasNotChanged(expectedSearcher);
+
+      assertU(commit("softCommit","true"));
+      assertSearcherHasNotChanged(expectedSearcher);
+
+      assertU(commit("softCommit","true","openSearcher","true"));
+      assertSearcherHasNotChanged(expectedSearcher);
+
+      assertU(delQ("match_no_documents_s:foo"));
+      assertU(commit());
+      assertSearcherHasNotChanged(expectedSearcher);
+
+      assertU(delI("0")); // no doc has this id, yet
+      assertU(commit());
+      assertSearcherHasNotChanged(expectedSearcher);
+
+    } finally {
+      baseReq.close();
+    }
+
+    // now do a variety of things that *should* always garuntee a new searcher
+    SolrQueryRequest beforeReq;
+
+    beforeReq = req("q","foo");
+    try {
+      // NOTE: we *have* to call getSearcher() in advance: delayed binding
+      SolrIndexSearcher before = beforeReq.getSearcher();
+      assertU(delI("1"));
+      assertU(commit());
+      assertSearcherHasChanged(before);
+    } finally {
+      beforeReq.close();
+    }
+    
+    beforeReq = req("q","foo");
+    try {
+      // NOTE: we *have* to call getSearcher() in advance: delayed binding
+      SolrIndexSearcher before = beforeReq.getSearcher();
+      assertU(adoc("id", "0", "v_s", "foo"));
+      assertU(commit());
+      assertSearcherHasChanged(before);
+    } finally {
+      beforeReq.close();
+    }
+
+    beforeReq = req("q","foo");
+    try {
+      // NOTE: we *have* to call getSearcher() in advance: delayed binding
+      SolrIndexSearcher before = beforeReq.getSearcher();
+      assertU(delQ("id:[0 TO 5]"));
+      assertU(commit());
+      assertSearcherHasChanged(before);
+    } finally {
+      beforeReq.close();
+    }
+
+    // :nocommit: add a test where there is a managed schema modification
+
+  }
+  
+  /**
+   * Given an existing searcher, creates a new SolrRequest, and verifies that the 
+   * searcher in that request is <b>not</b> the same as the previous searcher -- 
+   * cleaningly closing the new SolrRequest either way.
+   */
+  public static void assertSearcherHasChanged(SolrIndexSearcher previous) {
+    SolrQueryRequest req = req("*:*");
+    try {
+      SolrIndexSearcher newSearcher = req.getSearcher();
+      assertNotSame(previous, newSearcher);
+    } finally {
+      req.close();
+    }
+  }
+
+  /**
+   * Given an existing searcher, creates a new SolrRequest, and verifies that the 
+   * searcher in that request is the same as the expected searcher -- cleaningly 
+   * closing the new SolrRequest either way.
+   */
+  public static void assertSearcherHasNotChanged(SolrIndexSearcher expected) {
+    SolrQueryRequest req = req("*:*");
+    try {
+      assertSame(expected, req.getSearcher());
+    } finally {
+      req.close();
+    }
+  }
+
+}

Property changes on: solr/core/src/test/org/apache/solr/search/TestSearcherReuse.java
___________________________________________________________________
Added: svn:eol-style
   + native

Index: solr/core/src/java/org/apache/solr/core/SolrCore.java
===================================================================
--- solr/core/src/java/org/apache/solr/core/SolrCore.java	(revision 1573023)
+++ solr/core/src/java/org/apache/solr/core/SolrCore.java	(working copy)
@@ -1456,20 +1456,33 @@
           }
         }
 
-        if (newReader == null) {
-          // if this is a request for a realtime searcher, just return the same searcher if there haven't been any changes.
+        if (newReader == null) { // the underlying index has not changed at all
+
           if (realtime) {
+            // if this is a request for a realtime searcher, just return the same searcher
             newestSearcher.incref();
             return newestSearcher;
-          }
 
+          } else if (newestSearcher.get().getSchema() == getLatestSchema()) {
+            // absolutely nothing has changed, can use the same searcher
+            // but log a message about it to minimize confusion
+
+            newestSearcher.incref();
+            log.info("SolrIndexSearcher has not changed - not re-opening: " + newestSearcher.get().getName());
+            return newestSearcher;
+
+          } // ELSE: open a new searcher against the old reader...
           currentReader.incRef();
           newReader = currentReader;
         }
 
-       // for now, turn off caches if this is for a realtime reader (caches take a little while to instantiate)
-        tmp = new SolrIndexSearcher(this, newIndexDir, getLatestSchema(), getSolrConfig().indexConfig, 
-            (realtime ? "realtime":"main"), newReader, true, !realtime, true, directoryFactory);
+        // for now, turn off caches if this is for a realtime reader 
+        // (caches take a little while to instantiate)
+        final boolean useCaches = !realtime;
+        final String newName = realtime ? "realtime" : "main";
+        tmp = new SolrIndexSearcher(this, newIndexDir, getLatestSchema(), 
+                                    getSolrConfig().indexConfig, newName,
+                                    newReader, true, useCaches, true, directoryFactory);
 
       } else {
         // newestSearcher == null at this point
@@ -1665,69 +1678,69 @@
 
       Future future=null;
 
-      // warm the new searcher based on the current searcher.
-      // should this go before the other event handlers or after?
-      if (currSearcher != null) {
-        future = searcherExecutor.submit(
-            new Callable() {
-              @Override
-              public Object call() throws Exception {
-                try {
-                  newSearcher.warm(currSearcher);
-                } catch (Throwable e) {
-                  SolrException.log(log,e);
-                  if (e instanceof Error) {
-                    throw (Error) e;
-                  }
+      // if the underlying seracher has not changed, no warming is needed
+      if (newSearcher != currSearcher) {
+        
+        // warm the new searcher based on the current searcher.
+        // should this go before the other event handlers or after?
+        if (currSearcher != null) {
+          future = searcherExecutor.submit(new Callable() {
+            @Override
+            public Object call() throws Exception {
+              try {
+                newSearcher.warm(currSearcher);
+              } catch (Throwable e) {
+                SolrException.log(log, e);
+                if (e instanceof Error) {
+                  throw (Error) e;
                 }
-                return null;
               }
+              return null;
             }
-        );
-      }
-
-      if (currSearcher==null && firstSearcherListeners.size() > 0) {
-        future = searcherExecutor.submit(
-            new Callable() {
-              @Override
-              public Object call() throws Exception {
-                try {
-                  for (SolrEventListener listener : firstSearcherListeners) {
-                    listener.newSearcher(newSearcher,null);
-                  }
-                } catch (Throwable e) {
-                  SolrException.log(log,null,e);
-                  if (e instanceof Error) {
-                    throw (Error) e;
-                  }
+          });
+        }
+        
+        if (currSearcher == null && firstSearcherListeners.size() > 0) {
+          future = searcherExecutor.submit(new Callable() {
+            @Override
+            public Object call() throws Exception {
+              try {
+                for (SolrEventListener listener : firstSearcherListeners) {
+                  listener.newSearcher(newSearcher, null);
                 }
-                return null;
+              } catch (Throwable e) {
+                SolrException.log(log, null, e);
+                if (e instanceof Error) {
+                  throw (Error) e;
+                }
               }
+              return null;
             }
-        );
-      }
-
-      if (currSearcher!=null && newSearcherListeners.size() > 0) {
-        future = searcherExecutor.submit(
-            new Callable() {
-              @Override
-              public Object call() throws Exception {
-                try {
-                  for (SolrEventListener listener : newSearcherListeners) {
-                    listener.newSearcher(newSearcher, currSearcher);
-                  }
-                } catch (Throwable e) {
-                  SolrException.log(log,null,e);
-                  if (e instanceof Error) {
-                    throw (Error) e;
-                  }
+          });
+        }
+        
+        if (currSearcher != null && newSearcherListeners.size() > 0) {
+          future = searcherExecutor.submit(new Callable() {
+            @Override
+            public Object call() throws Exception {
+              try {
+                for (SolrEventListener listener : newSearcherListeners) {
+                  listener.newSearcher(newSearcher, currSearcher);
                 }
-                return null;
+              } catch (Throwable e) {
+                SolrException.log(log, null, e);
+                if (e instanceof Error) {
+                  throw (Error) e;
+                }
               }
+              return null;
             }
-        );
+          });
+        }
+        
       }
 
+
       // WARNING: this code assumes a single threaded executor (that all tasks
       // queued will finish first).
       final RefCounted<SolrIndexSearcher> currSearcherHolderF = currSearcherHolder;
