true, all buffered deletes will
+ * be applied (made visible) in the {@link IndexSearcher} / {@link IndexReader}.
+ * If false, 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 false.
+ * See {@link IndexReader#openIfChanged(IndexReader, IndexWriter, boolean)}.
+ * @param warmer An optional {@link SearcherWarmer}. Pass
+ * null if you don't require the searcher to warmed
+ * before going live. If this is non-null 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 null
+ * 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;
+ 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
+ * null if you don't require the searcher to warmed
+ * before going live. If this is non-null 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 null
+ * 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);
}
/**
* 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 +156,9 @@ public abstract class SearcherManager {
// 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 +245,10 @@ public abstract class SearcherManager {
}
}
- 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 true, all buffered deletes will
- * be applied (made visible) in the {@link IndexSearcher} / {@link IndexReader}.
- * If false, 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 false.
- * @param warmer An optional {@link SearcherWarmer}. Pass
- * null if you don't require the searcher to warmed
- * before going live. If this is non-null 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 null
- * 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
- * null if you don't require the searcher to warmed
- * before going live. If this is non-null 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 null
- * 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);
- }
- }
}
diff --git a/lucene/contrib/misc/src/test/org/apache/lucene/search/TestSearcherManager.java b/lucene/contrib/misc/src/test/org/apache/lucene/search/TestSearcherManager.java
index 28327643215..c06d8f188fd 100644
--- a/lucene/contrib/misc/src/test/org/apache/lucene/search/TestSearcherManager.java
+++ b/lucene/contrib/misc/src/test/org/apache/lucene/search/TestSearcherManager.java
@@ -72,12 +72,12 @@ public class TestSearcherManager extends ThreadedIndexingAndSearchingTestCase {
}
};
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 @@ public class TestSearcherManager extends ThreadedIndexingAndSearchingTestCase {
}
}
};
- 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());
diff --git a/lucene/src/java/org/apache/lucene/index/DirectoryReader.java b/lucene/src/java/org/apache/lucene/index/DirectoryReader.java
index 6c9313491be..fe64a210aee 100644
--- a/lucene/src/java/org/apache/lucene/index/DirectoryReader.java
+++ b/lucene/src/java/org/apache/lucene/index/DirectoryReader.java
@@ -406,8 +406,15 @@ class DirectoryReader extends IndexReader implements Cloneable {
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 @@ class DirectoryReader extends IndexReader implements Cloneable {
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;
}
diff --git a/lucene/src/java/org/apache/lucene/index/IndexReader.java b/lucene/src/java/org/apache/lucene/index/IndexReader.java
index 6d20ec7e06f..07a8b9e59c5 100644
--- a/lucene/src/java/org/apache/lucene/index/IndexReader.java
+++ b/lucene/src/java/org/apache/lucene/index/IndexReader.java
@@ -561,10 +561,6 @@ public abstract class IndexReader implements Cloneable,Closeable {
* with the old reader uses "copy on write" semantics to
* ensure the changes are not seen by other readers.
*
- * NOTE: 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 diff --git a/lucene/src/java/org/apache/lucene/index/IndexWriter.java b/lucene/src/java/org/apache/lucene/index/IndexWriter.java index 75688919666..0f5b2849465 100644 --- a/lucene/src/java/org/apache/lucene/index/IndexWriter.java +++ b/lucene/src/java/org/apache/lucene/index/IndexWriter.java @@ -4073,6 +4073,7 @@ public class IndexWriter implements Closeable, TwoPhaseCommit { 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(); } diff --git a/lucene/src/test-framework/org/apache/lucene/index/ThreadedIndexingAndSearchingTestCase.java b/lucene/src/test-framework/org/apache/lucene/index/ThreadedIndexingAndSearchingTestCase.java index 4c5d3a87766..425fe7b6568 100644 --- a/lucene/src/test-framework/org/apache/lucene/index/ThreadedIndexingAndSearchingTestCase.java +++ b/lucene/src/test-framework/org/apache/lucene/index/ThreadedIndexingAndSearchingTestCase.java @@ -55,7 +55,7 @@ import org.apache.lucene.util._TestUtil; // 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. */ diff --git a/lucene/src/test/org/apache/lucene/index/TestIndexWriterReader.java b/lucene/src/test/org/apache/lucene/index/TestIndexWriterReader.java index dffc9662afc..c012cdac5ff 100644 --- a/lucene/src/test/org/apache/lucene/index/TestIndexWriterReader.java +++ b/lucene/src/test/org/apache/lucene/index/TestIndexWriterReader.java @@ -857,7 +857,7 @@ public class TestIndexWriterReader extends LuceneTestCase { 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 class TestIndexWriterReader extends LuceneTestCase { } } + 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(); + } } diff --git a/lucene/src/test/org/apache/lucene/search/TestCachingSpanFilter.java b/lucene/src/test/org/apache/lucene/search/TestCachingSpanFilter.java index 702c2fa350c..eefeb301cf2 100644 --- a/lucene/src/test/org/apache/lucene/search/TestCachingSpanFilter.java +++ b/lucene/src/test/org/apache/lucene/search/TestCachingSpanFilter.java @@ -116,10 +116,10 @@ public class TestCachingSpanFilter extends LuceneTestCase { // 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); diff --git a/lucene/src/test/org/apache/lucene/search/TestCachingWrapperFilter.java b/lucene/src/test/org/apache/lucene/search/TestCachingWrapperFilter.java index 48bf9915ef1..5bf9b7c08b3 100644 --- a/lucene/src/test/org/apache/lucene/search/TestCachingWrapperFilter.java +++ b/lucene/src/test/org/apache/lucene/search/TestCachingWrapperFilter.java @@ -235,10 +235,9 @@ public class TestCachingWrapperFilter extends LuceneTestCase { // 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);