Google Git
Sign in
apache / lucene-jira-archive / refs/heads/gh-pages / . / docs / attachments / LUCENE-3520 / LUCENE-3520.patch
blob: 5e2a546cd8e37508eeb1e29093c939990aa00ea1 [file] [log] [blame]
Index: lucene/contrib/misc/src/test/org/apache/lucene/search/TestSearcherManager.java
===================================================================
--- lucene/contrib/misc/src/test/org/apache/lucene/search/TestSearcherManager.java (revision 1183753)
+++ lucene/contrib/misc/src/test/org/apache/lucene/search/TestSearcherManager.java (working copy)
@@ -72,12 +72,12 @@
}
};
if (random.nextBoolean()) {
- mgr = SearcherManager.open(writer, true, warmer, es);
+ mgr = new SearcherManager(writer, true, warmer, es);
isNRT = true;
} else {
// SearcherManager needs to see empty commit:
writer.commit();
- mgr = SearcherManager.open(dir, warmer, es);
+ mgr = new SearcherManager(dir, warmer, es);
isNRT = false;
}
@@ -198,8 +198,8 @@
}
}
};
- final SearcherManager searcherManager = random.nextBoolean() ? SearcherManager.open(dir,
- warmer, es) : SearcherManager.open(writer, random.nextBoolean(), warmer, es);
+ final SearcherManager searcherManager = random.nextBoolean() ? new SearcherManager(dir,
+ warmer, es) : new SearcherManager(writer, random.nextBoolean(), warmer, es);
IndexSearcher searcher = searcherManager.acquire();
try {
assertEquals(1, searcher.getIndexReader().numDocs());
Index: lucene/contrib/misc/src/java/org/apache/lucene/search/SearcherLifetimeManager.java
===================================================================
--- lucene/contrib/misc/src/java/org/apache/lucene/search/SearcherLifetimeManager.java (revision 1183650)
+++ lucene/contrib/misc/src/java/org/apache/lucene/search/SearcherLifetimeManager.java (working copy)
@@ -173,8 +173,8 @@
// incRef done by SearcherTracker ctor:
tracker.close();
}
- } else {
- assert tracker.searcher == searcher;
+ } else if (tracker.searcher != searcher) {
+ throw new IllegalArgumentException("the provided searcher has the same underlying reader version yet the searcher instance differs from before (new=" + searcher + " vs old=" + tracker.searcher);
}
return version;
Index: lucene/contrib/misc/src/java/org/apache/lucene/search/SearcherManager.java
===================================================================
--- lucene/contrib/misc/src/java/org/apache/lucene/search/SearcherManager.java (revision 1183650)
+++ lucene/contrib/misc/src/java/org/apache/lucene/search/SearcherManager.java (working copy)
@@ -64,23 +64,79 @@
* @lucene.experimental
*/
-public abstract class SearcherManager {
+public final class SearcherManager {
- protected volatile IndexSearcher currentSearcher;
- protected final ExecutorService es;
- protected final SearcherWarmer warmer;
- protected final Semaphore reopenLock = new Semaphore(1);
+ private volatile IndexSearcher currentSearcher;
+ private final ExecutorService es;
+ private final SearcherWarmer warmer;
+ private final Semaphore reopenLock = new Semaphore(1);
+ private final IndexWriter writer;
- protected SearcherManager(IndexReader openedReader, SearcherWarmer warmer,
+ /**
+ * Creates and returns a new SearcherManager from the given {@link IndexWriter}.
+ * @param writer the IndexWriter to open the IndexReader from.
+ * @param applyAllDeletes If <code>true</code>, all buffered deletes will
+ * be applied (made visible) in the {@link IndexSearcher} / {@link IndexReader}.
+ * If <code>false</code>, the deletes may or may not be applied, but remain buffered
+ * (in IndexWriter) so that they will be applied in the future.
+ * Applying deletes can be costly, so if your app can tolerate deleted documents
+ * being returned you might gain some performance by passing <code>false</code>.
+ * See {@link IndexReader#openIfChanged(IndexReader, IndexWriter, boolean)}.
+ * @param warmer An optional {@link SearcherWarmer}. Pass
+ * <code>null</code> if you don't require the searcher to warmed
+ * before going live. If this is <code>non-null</code> then a
+ * merged segment warmer is installed on the
+ * provided IndexWriter's config.
+ * @param es An optional {@link ExecutorService} so different segments can
+ * be searched concurrently (see {@link
+ * IndexSearcher#IndexSearcher(IndexReader,ExecutorService)}. Pass <code>null</code>
+ * to search segments sequentially.
+ *
+ * @throws IOException
+ */
+ public SearcherManager(IndexWriter writer, boolean applyAllDeletes,
+ final SearcherWarmer warmer, final ExecutorService es) throws IOException {
+ this.es = es;
+ this.warmer = warmer;
+ this.writer = writer;
+ currentSearcher = new IndexSearcher(IndexReader.open(writer, applyAllDeletes));
+ if (warmer != null) {
+ writer.getConfig().setMergedSegmentWarmer(
+ new IndexWriter.IndexReaderWarmer() {
+ @Override
+ public void warm(IndexReader reader) throws IOException {
+ warmer.warm(new IndexSearcher(reader, es));
+ }
+ });
+ }
+ }
+
+ /**
+ * Creates and returns a new SearcherManager from the given {@link Directory}.
+ * @param dir the directory to open the IndexReader on.
+ * @param warmer An optional {@link SearcherWarmer}. Pass
+ * <code>null</code> if you don't require the searcher to warmed
+ * before going live. If this is <code>non-null</code> then a
+ * merged segment warmer is installed on the
+ * provided IndexWriter's config.
+ * @param es And optional {@link ExecutorService} so different segments can
+ * be searched concurrently (see {@link
+ * IndexSearcher#IndexSearcher(IndexReader,ExecutorService)}. Pass <code>null</code>
+ * to search segments sequentially.
+ *
+ * @throws IOException
+ */
+ public SearcherManager(Directory dir, SearcherWarmer warmer,
ExecutorService es) throws IOException {
this.es = es;
this.warmer = warmer;
- currentSearcher = new IndexSearcher(openedReader, es);
+ currentSearcher = new IndexSearcher(IndexReader.open(dir, true), es);
+ writer = null;
}
/**
* You must call this, periodically, to perform a reopen. This calls
- * {@link #openIfChanged(IndexReader)} with the underlying reader, and if that returns a
+ * {@link IndexReader#openIfChanged(IndexReader)} with the underlying reader, and if that returns a
* new reader, it's warmed (if you provided a {@link SearcherWarmer} and then
* swapped into production.
*
@@ -103,7 +159,9 @@
// threads just return immediately:
if (reopenLock.tryAcquire()) {
try {
- final IndexReader newReader = openIfChanged(currentSearcher.getIndexReader());
+ // IR.openIfChanged preserves NRT and applyDeletes
+ // in the newly returned reader:
+ final IndexReader newReader = IndexReader.openIfChanged(currentSearcher.getIndexReader());
if (newReader != null) {
final IndexSearcher newSearcher = new IndexSearcher(newReader, es);
boolean success = false;
@@ -190,122 +248,10 @@
}
}
- protected synchronized void swapSearcher(IndexSearcher newSearcher) throws IOException {
+ private synchronized void swapSearcher(IndexSearcher newSearcher) throws IOException {
ensureOpen();
final IndexSearcher oldSearcher = currentSearcher;
currentSearcher = newSearcher;
release(oldSearcher);
}
-
- protected abstract IndexReader openIfChanged(IndexReader oldReader)
- throws IOException;
-
- /**
- * Creates and returns a new SearcherManager from the given {@link IndexWriter}.
- * @param writer the IndexWriter to open the IndexReader from.
- * @param applyAllDeletes If <code>true</code>, all buffered deletes will
- * be applied (made visible) in the {@link IndexSearcher} / {@link IndexReader}.
- * If <code>false</code>, the deletes are not applied but remain buffered
- * (in IndexWriter) so that they will be applied in the future.
- * Applying deletes can be costly, so if your app can tolerate deleted documents
- * being returned you might gain some performance by passing <code>false</code>.
- * @param warmer An optional {@link SearcherWarmer}. Pass
- * <code>null</code> if you don't require the searcher to warmed
- * before going live. If this is <code>non-null</code> then a
- * merged segment warmer is installed on the
- * provided IndexWriter's config.
- * @param es An optional {@link ExecutorService} so different segments can
- * be searched concurrently (see {@link
- * IndexSearcher#IndexSearcher(IndexReader,ExecutorService)}. Pass <code>null</code>
- * to search segments sequentially.
- *
- * @see IndexReader#openIfChanged(IndexReader, IndexWriter, boolean)
- * @throws IOException
- */
- public static SearcherManager open(IndexWriter writer, boolean applyAllDeletes,
- SearcherWarmer warmer, ExecutorService es) throws IOException {
- final IndexReader open = IndexReader.open(writer, true);
- boolean success = false;
- try {
- SearcherManager manager = new NRTSearcherManager(writer, applyAllDeletes,
- open, warmer, es);
- success = true;
- return manager;
- } finally {
- if (!success) {
- open.close();
- }
- }
- }
-
- /**
- * Creates and returns a new SearcherManager from the given {@link Directory}.
- * @param dir the directory to open the IndexReader on.
- * @param warmer An optional {@link SearcherWarmer}. Pass
- * <code>null</code> if you don't require the searcher to warmed
- * before going live. If this is <code>non-null</code> then a
- * merged segment warmer is installed on the
- * provided IndexWriter's config.
- * @param es And optional {@link ExecutorService} so different segments can
- * be searched concurrently (see {@link
- * IndexSearcher#IndexSearcher(IndexReader,ExecutorService)}. Pass <code>null</code>
- * to search segments sequentially.
- *
- * @throws IOException
- */
- public static SearcherManager open(Directory dir, SearcherWarmer warmer,
- ExecutorService es) throws IOException {
- final IndexReader open = IndexReader.open(dir, true);
- boolean success = false;
- try {
- SearcherManager manager = new DirectorySearchManager(open, warmer, es);
- success = true;
- return manager;
- } finally {
- if (!success) {
- open.close();
- }
- }
- }
-
- static final class NRTSearcherManager extends SearcherManager {
- private final IndexWriter writer;
- private final boolean applyDeletes;
-
- NRTSearcherManager(final IndexWriter writer, final boolean applyDeletes,
- final IndexReader openedReader, final SearcherWarmer warmer, final ExecutorService es)
- throws IOException {
- super(openedReader, warmer, es);
- this.writer = writer;
- this.applyDeletes = applyDeletes;
- if (warmer != null) {
- writer.getConfig().setMergedSegmentWarmer(
- new IndexWriter.IndexReaderWarmer() {
- @Override
- public void warm(IndexReader reader) throws IOException {
- warmer.warm(new IndexSearcher(reader, es));
- }
- });
- }
- }
-
- @Override
- protected IndexReader openIfChanged(IndexReader oldReader)
- throws IOException {
- return IndexReader.openIfChanged(oldReader, writer, applyDeletes);
- }
- }
-
- static final class DirectorySearchManager extends SearcherManager {
- DirectorySearchManager(IndexReader openedReader,
- SearcherWarmer warmer, ExecutorService es) throws IOException {
- super(openedReader, warmer, es);
- }
-
- @Override
- protected IndexReader openIfChanged(IndexReader oldReader)
- throws IOException {
- return IndexReader.openIfChanged(oldReader, true);
- }
- }
}
Index: lucene/contrib/misc/src/java/org/apache/lucene/index/NRTManager.java
===================================================================
--- lucene/contrib/misc/src/java/org/apache/lucene/index/NRTManager.java (revision 1183650)
+++ lucene/contrib/misc/src/java/org/apache/lucene/index/NRTManager.java (working copy)
@@ -93,10 +93,10 @@
SearcherWarmer warmer, boolean alwaysApplyDeletes) throws IOException {
this.writer = writer;
if (alwaysApplyDeletes) {
- withoutDeletes = withDeletes = new SearcherManagerRef(true, 0, SearcherManager.open(writer, true, warmer, es));
+ withoutDeletes = withDeletes = new SearcherManagerRef(true, 0, new SearcherManager(writer, true, warmer, es));
} else {
- withDeletes = new SearcherManagerRef(true, 0, SearcherManager.open(writer, true, warmer, es));
- withoutDeletes = new SearcherManagerRef(false, 0, SearcherManager.open(writer, false, warmer, es));
+ withDeletes = new SearcherManagerRef(true, 0, new SearcherManager(writer, true, warmer, es));
+ withoutDeletes = new SearcherManagerRef(false, 0, new SearcherManager(writer, false, warmer, es));
}
indexingGen = new AtomicLong(1);
}
Index: lucene/src/test/org/apache/lucene/search/TestCachingWrapperFilter.java
===================================================================
--- lucene/src/test/org/apache/lucene/search/TestCachingWrapperFilter.java (revision 1183650)
+++ lucene/src/test/org/apache/lucene/search/TestCachingWrapperFilter.java (working copy)
@@ -235,10 +235,9 @@
// make sure we get a cache hit when we reopen reader
// that had no change to deletions
+ writer.deleteDocuments(new Term("foo", "bar"));
reader = refreshReader(reader);
- assertTrue(reader != oldReader);
- searcher.close();
- searcher = newSearcher(reader, false);
+ assertTrue(reader == oldReader);
int missCount = filter.missCount;
docs = searcher.search(constantScore, 1);
assertEquals("[just filter] Should find a hit...", 1, docs.totalHits);
Index: lucene/src/test/org/apache/lucene/search/TestCachingSpanFilter.java
===================================================================
--- lucene/src/test/org/apache/lucene/search/TestCachingSpanFilter.java (revision 1183650)
+++ lucene/src/test/org/apache/lucene/search/TestCachingSpanFilter.java (working copy)
@@ -116,10 +116,10 @@
// make sure we get a cache hit when we reopen readers
// that had no new deletions
+ // Deletes nothing:
+ writer.deleteDocuments(new Term("foo", "bar"));
reader = refreshReader(reader);
- assertTrue(reader != oldReader);
- searcher.close();
- searcher = newSearcher(reader, false);
+ assertTrue(reader == oldReader);
int missCount = filter.missCount;
docs = searcher.search(constantScore, 1);
assertEquals("[just filter] Should find a hit...", 1, docs.totalHits);
Index: lucene/src/test/org/apache/lucene/index/TestIndexWriterReader.java
===================================================================
--- lucene/src/test/org/apache/lucene/index/TestIndexWriterReader.java (revision 1183650)
+++ lucene/src/test/org/apache/lucene/index/TestIndexWriterReader.java (working copy)
@@ -857,7 +857,7 @@
int sum = 0;
while(System.currentTimeMillis() < endTime) {
IndexReader r2 = IndexReader.openIfChanged(r);
- if (r2 != r) {
+ if (r2 != null) {
r.close();
r = r2;
}
@@ -1016,4 +1016,40 @@
}
}
+ public void testReopenAfterNoRealChange() throws Exception {
+ Directory d = newDirectory();
+ IndexWriter w = new IndexWriter(
+ d,
+ newIndexWriterConfig(TEST_VERSION_CURRENT, new MockAnalyzer(random)));
+ w.setInfoStream(VERBOSE ? System.out : null);
+
+ IndexReader r = w.getReader(); // start pooling readers
+
+ IndexReader r2 = IndexReader.openIfChanged(r);
+ assertNull(r2);
+
+ w.addDocument(new Document());
+ IndexReader r3 = IndexReader.openIfChanged(r);
+ assertNotNull(r3);
+ assertTrue(r3.getVersion() != r.getVersion());
+ assertTrue(r3.isCurrent());
+
+ // Deletes nothing in reality...:
+ w.deleteDocuments(new Term("foo", "bar"));
+
+ // ... but IW marks this as not current:
+ assertFalse(r3.isCurrent());
+ IndexReader r4 = IndexReader.openIfChanged(r3);
+ assertNull(r4);
+
+ // Deletes nothing in reality...:
+ w.deleteDocuments(new Term("foo", "bar"));
+ IndexReader r5 = IndexReader.openIfChanged(r3, w, true);
+ assertNull(r5);
+
+ r3.close();
+
+ w.close();
+ d.close();
+ }
}
Index: lucene/src/java/org/apache/lucene/index/DirectoryReader.java
===================================================================
--- lucene/src/java/org/apache/lucene/index/DirectoryReader.java (revision 1183650)
+++ lucene/src/java/org/apache/lucene/index/DirectoryReader.java (working copy)
@@ -406,8 +406,15 @@
return doOpenIfChanged(true, commit);
}
- // NOTE: always returns a non-null result (ie new reader)
- // but that could change someday
+ @Override
+ protected final IndexReader doOpenIfChanged(IndexWriter writer, boolean applyAllDeletes) throws CorruptIndexException, IOException {
+ if (writer == this.writer && applyAllDeletes == this.applyAllDeletes) {
+ return doOpenIfChanged();
+ } else {
+ return super.doOpenIfChanged(writer, applyAllDeletes);
+ }
+ }
+
private final IndexReader doOpenFromWriter(boolean openReadOnly, IndexCommit commit) throws CorruptIndexException, IOException {
assert readOnly;
@@ -419,10 +426,18 @@
throw new IllegalArgumentException("a reader obtained from IndexWriter.getReader() cannot currently accept a commit");
}
- // TODO: right now we *always* make a new reader; in
- // the future we could have write make some effort to
- // detect that no changes have occurred
+ if (writer.nrtIsCurrent(segmentInfos)) {
+ return null;
+ }
+
IndexReader reader = writer.getReader(applyAllDeletes);
+
+ // If in fact no changes took place, return null:
+ if (reader.getVersion() == getVersion()) {
+ reader.decRef();
+ return null;
+ }
+
reader.readerFinishedListeners = readerFinishedListeners;
return reader;
}
Index: lucene/src/java/org/apache/lucene/index/IndexReader.java
===================================================================
--- lucene/src/java/org/apache/lucene/index/IndexReader.java (revision 1183650)
+++ lucene/src/java/org/apache/lucene/index/IndexReader.java (working copy)
@@ -561,10 +561,6 @@
* with the old reader uses "copy on write" semantics to
* ensure the changes are not seen by other readers.
*
- * <p><b>NOTE</b>: If the provided reader is a near real-time
- * reader, this method will return another near-real-time
- * reader.
- *
* @throws CorruptIndexException if the index is corrupt
* @throws IOException if there is a low-level IO error
* @return null if there are no changes; else, a new
Index: lucene/src/java/org/apache/lucene/index/IndexWriter.java
===================================================================
--- lucene/src/java/org/apache/lucene/index/IndexWriter.java (revision 1183650)
+++ lucene/src/java/org/apache/lucene/index/IndexWriter.java (working copy)
@@ -4073,6 +4073,7 @@
synchronized boolean nrtIsCurrent(SegmentInfos infos) {
//System.out.println("IW.nrtIsCurrent " + (infos.version == segmentInfos.version && !docWriter.anyChanges() && !bufferedDeletesStream.any()));
+ ensureOpen();
return infos.version == segmentInfos.version && !docWriter.anyChanges() && !bufferedDeletesStream.any();
}
Index: lucene/src/test-framework/org/apache/lucene/index/ThreadedIndexingAndSearchingTestCase.java
===================================================================
--- lucene/src/test-framework/org/apache/lucene/index/ThreadedIndexingAndSearchingTestCase.java (revision 1183650)
+++ lucene/src/test-framework/org/apache/lucene/index/ThreadedIndexingAndSearchingTestCase.java (working copy)
@@ -55,7 +55,7 @@
// TODO
// - mix in optimize, addIndexes
-// - randomoly mix in non-congruent docs
+// - randomly mix in non-congruent docs
/** Utility class that spawns multiple indexing and
* searching threads. */