You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ho...@apache.org on 2014/03/03 22:37:51 UTC
svn commit: r1573763 - in /lucene/dev/trunk/solr: CHANGES.txt
core/src/java/org/apache/solr/core/SolrCore.java
core/src/test/org/apache/solr/search/TestIndexSearcher.java
core/src/test/org/apache/solr/search/TestSearcherReuse.java
Author: hossman
Date: Mon Mar 3 21:37:50 2014
New Revision: 1573763
URL: http://svn.apache.org/r1573763
Log:
SOLR-5783: Requests to open a new searcher will now reuse the current registered searcher (w/o additional warming) if possible in situations where the underlying index has not changed
Added:
lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestSearcherReuse.java (with props)
Modified:
lucene/dev/trunk/solr/CHANGES.txt
lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/SolrCore.java
lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestIndexSearcher.java
Modified: lucene/dev/trunk/solr/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/CHANGES.txt?rev=1573763&r1=1573762&r2=1573763&view=diff
==============================================================================
--- lucene/dev/trunk/solr/CHANGES.txt (original)
+++ lucene/dev/trunk/solr/CHANGES.txt Mon Mar 3 21:37:50 2014
@@ -119,6 +119,11 @@ Optimizations
stage gets all fields. Requests with fl=id or fl=id,score are now single-pass.
(Shawn Smith, Vitaliy Zhovtyuk, shalin)
+* SOLR-5783: Requests to open a new searcher will now reuse the current registered
+ searcher (w/o additional warming) if possible in situations where the underlying
+ index has not changed. This reduces overhead in situations such as deletes that
+ do not modify the index, and/or redundant commits. (hossman)
+
Other Changes
---------------------
Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/SolrCore.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/SolrCore.java?rev=1573763&r1=1573762&r2=1573763&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/SolrCore.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/SolrCore.java Mon Mar 3 21:37:50 2014
@@ -1459,20 +1459,33 @@ public final class SolrCore implements S
}
}
- 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
@@ -1668,69 +1681,69 @@ public final class SolrCore implements S
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);
+ }
+ } catch (Throwable e) {
+ SolrException.log(log, null, e);
+ if (e instanceof Error) {
+ throw (Error) e;
}
- return null;
}
+ 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);
+ }
+ } catch (Throwable e) {
+ SolrException.log(log, null, e);
+ if (e instanceof Error) {
+ throw (Error) e;
}
- return null;
}
+ return null;
}
- );
+ });
+ }
+
}
+
// WARNING: this code assumes a single threaded executor (that all tasks
// queued will finish first).
final RefCounted<SolrIndexSearcher> currSearcherHolderF = currSearcherHolder;
Modified: lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestIndexSearcher.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestIndexSearcher.java?rev=1573763&r1=1573762&r2=1573763&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestIndexSearcher.java (original)
+++ lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestIndexSearcher.java Mon Mar 3 21:37:50 2014
@@ -105,8 +105,10 @@ public class TestIndexSearcher extends S
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 @@ public class TestIndexSearcher extends S
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();
Added: lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestSearcherReuse.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestSearcherReuse.java?rev=1573763&view=auto
==============================================================================
--- lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestSearcherReuse.java (added)
+++ lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestSearcherReuse.java Mon Mar 3 21:37:50 2014
@@ -0,0 +1,237 @@
+/*
+ * 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.solr.schema.SchemaField;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.schema.ManagedIndexSchema;
+
+import org.apache.lucene.util.TestUtil;
+
+import org.apache.commons.io.FileUtils;
+
+import java.io.File;
+import java.util.Collections;
+
+import org.junit.BeforeClass;
+import org.junit.AfterClass;
+
+/**
+ * Requests to open a new searcher w/o any underlying change to the index exposed
+ * by the current searcher should result in the same searcher 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 {
+
+ private static File solrHome;
+
+ private static final String collection = "collection1";
+ private static final String confPath = collection + "/conf";
+
+ /**
+ * We're using a Managed schema so we can confirm that opening a new searcher
+ * after a schema modification results in getting a new searcher with the new
+ * schema linked to it.
+ */
+ @BeforeClass
+ private static void setupTempDirAndCoreWithManagedSchema() throws Exception {
+ createTempDir();
+ solrHome = new File(TEMP_DIR, TestSearcherReuse.class.getSimpleName());
+ solrHome = solrHome.getAbsoluteFile();
+
+ File confDir = new File(solrHome, confPath);
+ File testHomeConfDir = new File(TEST_HOME(), confPath);
+ FileUtils.copyFileToDirectory(new File(testHomeConfDir, "solrconfig-managed-schema.xml"), confDir);
+ FileUtils.copyFileToDirectory(new File(testHomeConfDir, "solrconfig.snippet.randomindexconfig.xml"), confDir);
+ FileUtils.copyFileToDirectory(new File(testHomeConfDir, "schema-id-and-version-fields-only.xml"), confDir);
+
+ // initCore will trigger an upgrade to managed schema, since the solrconfig has
+ // <schemaFactory class="ManagedIndexSchemaFactory" ... />
+ System.setProperty("managed.schema.mutable", "true");
+ initCore("solrconfig-managed-schema.xml", "schema-id-and-version-fields-only.xml",
+ solrHome.getPath());
+ }
+
+ @AfterClass
+ private static void deleteCoreAndTempSolrHomeDirectory() throws Exception {
+ FileUtils.deleteDirectory(solrHome);
+ solrHome = null;
+ }
+
+ @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));
+ 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("id:match_no_documents"));
+ 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"));
+ 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();
+ }
+
+ beforeReq = req("q","foo");
+ try {
+ // NOTE: we *have* to call getSearcher() in advance: delayed binding
+ SolrIndexSearcher before = beforeReq.getSearcher();
+
+ // create a new field & add it.
+ assertTrue("schema not mutable", beforeReq.getSchema().isMutable());
+ ManagedIndexSchema oldSchema = (ManagedIndexSchema) beforeReq.getSchema();
+ SchemaField newField = oldSchema.newField
+ ("hoss", "string", Collections.<String,Object>emptyMap());
+ IndexSchema newSchema = oldSchema.addField(newField);
+ h.getCore().setLatestSchema(newSchema);
+
+ // sanity check, later asserts assume this
+ assertNotSame(oldSchema, newSchema);
+
+ // the schema has changed - but nothing has requested a new Searcher yet
+ assertSearcherHasNotChanged(before);
+
+ // only now should we get a new searcher...
+ assertU(commit("softCommit","true","openSearcher","true"));
+ assertSearcherHasChanged(before);
+
+ // sanity that opening the new searcher was useful to get new schema...
+ SolrQueryRequest afterReq = req("q","foo");
+ try {
+ assertSame(newSchema, afterReq.getSchema());
+ assertSame(newSchema, afterReq.getSearcher().getSchema());
+ } finally {
+ afterReq.close();
+ }
+
+ } finally {
+ beforeReq.close();
+ }
+
+ }
+
+ /**
+ * 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();
+ }
+ }
+
+}