From ae430140414c6c461049658a3dbdcbdaaace4056 Mon Sep 17 00:00:00 2001 From: Michael McCandless Date: Mon, 3 Oct 2011 22:07:00 +0000 Subject: [PATCH] LUCENE-3464: rename IR.reopen -> IR.openIfChanged, and return null (not old reader) if there are no changes git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1178612 13f79535-47bb-0310-9956-ffa450edef68 --- lucene/CHANGES.txt | 5 + lucene/MIGRATE.txt | 7 +- .../org/apache/lucene/index/NRTManager.java | 9 +- .../apache/lucene/search/SearcherManager.java | 6 +- .../index/TestMultiPassIndexSplitter.java | 3 - .../lucene/store/TestNRTCachingDirectory.java | 4 +- .../apache/lucene/index/DirectoryReader.java | 72 +++---- .../org/apache/lucene/index/IndexReader.java | 178 +++++++++--------- .../org/apache/lucene/index/MultiReader.java | 42 +++-- .../apache/lucene/index/ParallelReader.java | 21 ++- .../apache/lucene/index/SegmentReader.java | 6 +- .../apache/lucene/search/IndexSearcher.java | 2 +- .../apache/lucene/index/TestIndexReader.java | 33 ++-- .../lucene/index/TestIndexReaderClone.java | 11 +- .../lucene/index/TestIndexReaderReopen.java | 71 ++++--- .../apache/lucene/index/TestIndexWriter.java | 3 +- .../lucene/index/TestIndexWriterCommit.java | 15 +- .../lucene/index/TestIndexWriterReader.java | 18 +- .../apache/lucene/index/TestNRTThreads.java | 4 +- .../apache/lucene/index/TestNeverDelete.java | 4 +- .../lucene/index/TestRollingUpdates.java | 4 +- .../apache/lucene/index/TestStressNRT.java | 9 +- .../lucene/search/TestCachingSpanFilter.java | 8 +- .../search/TestCachingWrapperFilter.java | 8 +- .../byTask/tasks/NearRealtimeReaderTask.java | 4 +- .../byTask/tasks/ReopenReaderTask.java | 4 +- .../taxonomy/lucene/LuceneTaxonomyReader.java | 4 +- .../taxonomy/lucene/LuceneTaxonomyWriter.java | 4 +- .../search/TestTotalFacetCountsCache.java | 3 +- .../facet/taxonomy/lucene/TestIndexClose.java | 8 +- .../java/org/apache/solr/core/SolrCore.java | 5 +- .../solr/update/DirectUpdateHandler2.java | 5 +- 32 files changed, 328 insertions(+), 252 deletions(-) diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index 917392808fc..1feade26c92 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -608,6 +608,11 @@ Changes in backwards compatibility policy As this is expert API, most code will not be affected. (Uwe Schindler, Doron Cohen, Mike McCandless) +* LUCENE-3464: IndexReader.reopen has been renamed to + IndexReader.openIfChanged (a static method), and now returns null + (instead of the old reader) if there are no changes in the index, to + prevent the common pitfall of accidentally closing the old reader. + Bug fixes * LUCENE-3412: SloppyPhraseScorer was returning non-deterministic results diff --git a/lucene/MIGRATE.txt b/lucene/MIGRATE.txt index 20b8827ad83..bc6df20e590 100644 --- a/lucene/MIGRATE.txt +++ b/lucene/MIGRATE.txt @@ -521,4 +521,9 @@ you can now do this: * LUCENE-3396: Analyzer.tokenStream() and .reusableTokenStream() have been made final. It is now necessary to use Analyzer.TokenStreamComponents to define an analysis process. Analyzer also has its own way of managing the reuse of TokenStreamComponents (either - globally, or per-field). To define another Strategy, implement Analyzer.ReuseStrategy. \ No newline at end of file + globally, or per-field). To define another Strategy, implement Analyzer.ReuseStrategy. + +* LUCENE-3464: IndexReader.reopen has been renamed to + IndexReader.openIfChanged (a static method), and now returns null + (instead of the old reader) if there are no changes to the index, to + prevent the common pitfall of accidentally closing the old reader. diff --git a/lucene/contrib/misc/src/java/org/apache/lucene/index/NRTManager.java b/lucene/contrib/misc/src/java/org/apache/lucene/index/NRTManager.java index 59b04e377e3..b664cdee4b9 100644 --- a/lucene/contrib/misc/src/java/org/apache/lucene/index/NRTManager.java +++ b/lucene/contrib/misc/src/java/org/apache/lucene/index/NRTManager.java @@ -307,7 +307,14 @@ public class NRTManager implements Closeable { // Start from whichever searcher is most current: final IndexSearcher startSearcher = noDeletesSearchingGen.get() > searchingGen.get() ? noDeletesCurrentSearcher : currentSearcher; - final IndexReader nextReader = startSearcher.getIndexReader().reopen(writer, applyDeletes); + IndexReader nextReader = IndexReader.openIfChanged(startSearcher.getIndexReader(), writer, applyDeletes); + if (nextReader == null) { + // NOTE: doesn't happen currently in Lucene (reopen on + // NRT reader always returns new reader), but could in + // the future: + nextReader = startSearcher.getIndexReader(); + nextReader.incRef(); + } if (nextReader != startSearcher.getIndexReader()) { final IndexSearcher nextSearcher = new IndexSearcher(nextReader, es); diff --git a/lucene/contrib/misc/src/java/org/apache/lucene/search/SearcherManager.java b/lucene/contrib/misc/src/java/org/apache/lucene/search/SearcherManager.java index c87494016cc..f7e585a66e2 100644 --- a/lucene/contrib/misc/src/java/org/apache/lucene/search/SearcherManager.java +++ b/lucene/contrib/misc/src/java/org/apache/lucene/search/SearcherManager.java @@ -111,7 +111,7 @@ public class SearcherManager implements Closeable { } /** You must call this, periodically, to perform a - * reopen. This calls {@link IndexReader#reopen} on the + * reopen. This calls {@link IndexReader#openIfChanged} on the * underlying reader, and if that returns a new reader, * it's warmed (if you provided a {@link SearcherWarmer} * and then swapped into production. @@ -139,8 +139,8 @@ public class SearcherManager implements Closeable { // threads just return immediately: if (reopening.tryAcquire()) { try { - IndexReader newReader = currentSearcher.getIndexReader().reopen(); - if (newReader != currentSearcher.getIndexReader()) { + IndexReader newReader = IndexReader.openIfChanged(currentSearcher.getIndexReader()); + if (newReader != null) { IndexSearcher newSearcher = new IndexSearcher(newReader, es); boolean success = false; try { diff --git a/lucene/contrib/misc/src/test/org/apache/lucene/index/TestMultiPassIndexSplitter.java b/lucene/contrib/misc/src/test/org/apache/lucene/index/TestMultiPassIndexSplitter.java index 3f4d609d53a..b9b20d8dfb4 100644 --- a/lucene/contrib/misc/src/test/org/apache/lucene/index/TestMultiPassIndexSplitter.java +++ b/lucene/contrib/misc/src/test/org/apache/lucene/index/TestMultiPassIndexSplitter.java @@ -45,9 +45,6 @@ public class TestMultiPassIndexSplitter extends LuceneTestCase { input = IndexReader.open(dir, false); // delete the last doc input.deleteDocument(input.maxDoc() - 1); - IndexReader inputOld = input; - input = input.reopen(true); - inputOld.close(); } @Override diff --git a/lucene/contrib/misc/src/test/org/apache/lucene/store/TestNRTCachingDirectory.java b/lucene/contrib/misc/src/test/org/apache/lucene/store/TestNRTCachingDirectory.java index 3efb3fdfe69..3a3a94a1c00 100644 --- a/lucene/contrib/misc/src/test/org/apache/lucene/store/TestNRTCachingDirectory.java +++ b/lucene/contrib/misc/src/test/org/apache/lucene/store/TestNRTCachingDirectory.java @@ -65,8 +65,8 @@ public class TestNRTCachingDirectory extends LuceneTestCase { if (r == null) { r = IndexReader.open(w.w, false); } else { - final IndexReader r2 = r.reopen(); - if (r2 != r) { + final IndexReader r2 = IndexReader.openIfChanged(r); + if (r2 != null) { r.close(); r = r2; } diff --git a/lucene/src/java/org/apache/lucene/index/DirectoryReader.java b/lucene/src/java/org/apache/lucene/index/DirectoryReader.java index ad46bdb5e86..6c9313491be 100644 --- a/lucene/src/java/org/apache/lucene/index/DirectoryReader.java +++ b/lucene/src/java/org/apache/lucene/index/DirectoryReader.java @@ -198,8 +198,8 @@ class DirectoryReader extends IndexReader implements Cloneable { initialize(readers.toArray(new SegmentReader[readers.size()])); } - /** This constructor is only used for {@link #reopen()} */ - DirectoryReader(Directory directory, SegmentInfos infos, SegmentReader[] oldReaders, int[] oldStarts, + /** This constructor is only used for {@link #doOpenIfChanged()} */ + DirectoryReader(Directory directory, SegmentInfos infos, SegmentReader[] oldReaders, boolean readOnly, boolean doClone, int termInfosIndexDivisor, CodecProvider codecs, Collection readerFinishedListeners) throws IOException { this.directory = directory; @@ -255,18 +255,21 @@ class DirectoryReader extends IndexReader implements Cloneable { // this is a new reader; in case we hit an exception we can close it safely newReader = SegmentReader.get(readOnly, infos.info(i), termInfosIndexDivisor, IOContext.READ); newReader.readerFinishedListeners = readerFinishedListeners; - } else { - newReader = newReaders[i].reopenSegment(infos.info(i), doClone, readOnly); - assert newReader.readerFinishedListeners == readerFinishedListeners; - } - if (newReader == newReaders[i]) { - // this reader will be shared between the old and the new one, - // so we must incRef it - readerShared[i] = true; - newReader.incRef(); - } else { readerShared[i] = false; newReaders[i] = newReader; + } else { + newReader = newReaders[i].reopenSegment(infos.info(i), doClone, readOnly); + if (newReader == null) { + // this reader will be shared between the old and the new one, + // so we must incRef it + readerShared[i] = true; + newReaders[i].incRef(); + } else { + assert newReader.readerFinishedListeners == readerFinishedListeners; + readerShared[i] = false; + // Steal ref returned to us by reopenSegment: + newReaders[i] = newReader; + } } success = true; } finally { @@ -364,8 +367,8 @@ class DirectoryReader extends IndexReader implements Cloneable { @Override public final synchronized IndexReader clone(boolean openReadOnly) throws CorruptIndexException, IOException { - // doReopen calls ensureOpen - DirectoryReader newReader = doReopen((SegmentInfos) segmentInfos.clone(), true, openReadOnly); + // doOpenIfChanged calls ensureOpen + DirectoryReader newReader = doOpenIfChanged((SegmentInfos) segmentInfos.clone(), true, openReadOnly); if (this != newReader) { newReader.deletionPolicy = deletionPolicy; @@ -388,22 +391,24 @@ class DirectoryReader extends IndexReader implements Cloneable { } @Override - public final IndexReader reopen() throws CorruptIndexException, IOException { + protected final IndexReader doOpenIfChanged() throws CorruptIndexException, IOException { // Preserve current readOnly - return doReopen(readOnly, null); + return doOpenIfChanged(readOnly, null); } @Override - public final IndexReader reopen(boolean openReadOnly) throws CorruptIndexException, IOException { - return doReopen(openReadOnly, null); + protected final IndexReader doOpenIfChanged(boolean openReadOnly) throws CorruptIndexException, IOException { + return doOpenIfChanged(openReadOnly, null); } @Override - public final IndexReader reopen(final IndexCommit commit) throws CorruptIndexException, IOException { - return doReopen(true, commit); + protected final IndexReader doOpenIfChanged(final IndexCommit commit) throws CorruptIndexException, IOException { + return doOpenIfChanged(true, commit); } - private final IndexReader doReopenFromWriter(boolean openReadOnly, IndexCommit commit) throws CorruptIndexException, IOException { + // NOTE: always returns a non-null result (ie new reader) + // but that could change someday + private final IndexReader doOpenFromWriter(boolean openReadOnly, IndexCommit commit) throws CorruptIndexException, IOException { assert readOnly; if (!openReadOnly) { @@ -422,7 +427,7 @@ class DirectoryReader extends IndexReader implements Cloneable { return reader; } - private IndexReader doReopen(final boolean openReadOnly, IndexCommit commit) throws CorruptIndexException, IOException { + private IndexReader doOpenIfChanged(final boolean openReadOnly, IndexCommit commit) throws CorruptIndexException, IOException { ensureOpen(); assert commit == null || openReadOnly; @@ -430,13 +435,13 @@ class DirectoryReader extends IndexReader implements Cloneable { // If we were obtained by writer.getReader(), re-ask the // writer to get a new reader. if (writer != null) { - return doReopenFromWriter(openReadOnly, commit); + return doOpenFromWriter(openReadOnly, commit); } else { - return doReopenNoWriter(openReadOnly, commit); + return doOpenNoWriter(openReadOnly, commit); } } - private synchronized IndexReader doReopenNoWriter(final boolean openReadOnly, IndexCommit commit) throws CorruptIndexException, IOException { + private synchronized IndexReader doOpenNoWriter(final boolean openReadOnly, IndexCommit commit) throws CorruptIndexException, IOException { if (commit == null) { if (hasChanges) { @@ -451,25 +456,26 @@ class DirectoryReader extends IndexReader implements Cloneable { if (openReadOnly) { return clone(openReadOnly); } else { - return this; + return null; } } else if (isCurrent()) { if (openReadOnly != readOnly) { // Just fallback to clone return clone(openReadOnly); } else { - return this; + return null; } } } else { - if (directory != commit.getDirectory()) + if (directory != commit.getDirectory()) { throw new IOException("the specified commit does not match the specified Directory"); + } if (segmentInfos != null && commit.getSegmentsFileName().equals(segmentInfos.getCurrentSegmentFileName())) { if (readOnly != openReadOnly) { // Just fallback to clone return clone(openReadOnly); } else { - return this; + return null; } } } @@ -479,15 +485,13 @@ class DirectoryReader extends IndexReader implements Cloneable { protected Object doBody(String segmentFileName) throws CorruptIndexException, IOException { final SegmentInfos infos = new SegmentInfos(codecs); infos.read(directory, segmentFileName, codecs); - return doReopen(infos, false, openReadOnly); + return doOpenIfChanged(infos, false, openReadOnly); } }.run(commit); } - private synchronized DirectoryReader doReopen(SegmentInfos infos, boolean doClone, boolean openReadOnly) throws CorruptIndexException, IOException { - DirectoryReader reader; - reader = new DirectoryReader(directory, infos, subReaders, starts, openReadOnly, doClone, termInfosIndexDivisor, codecs, readerFinishedListeners); - return reader; + private synchronized DirectoryReader doOpenIfChanged(SegmentInfos infos, boolean doClone, boolean openReadOnly) throws CorruptIndexException, IOException { + return new DirectoryReader(directory, infos, subReaders, openReadOnly, doClone, termInfosIndexDivisor, codecs, readerFinishedListeners); } /** Version number when this IndexReader was opened. */ diff --git a/lucene/src/java/org/apache/lucene/index/IndexReader.java b/lucene/src/java/org/apache/lucene/index/IndexReader.java index 266317b8869..3ceb2135093 100644 --- a/lucene/src/java/org/apache/lucene/index/IndexReader.java +++ b/lucene/src/java/org/apache/lucene/index/IndexReader.java @@ -344,7 +344,7 @@ public abstract class IndexReader implements Cloneable,Closeable { * @throws CorruptIndexException * @throws IOException if there is a low-level IO error * - * @see #reopen(IndexWriter,boolean) + * @see #openIfChanged(IndexReader,IndexWriter,boolean) * * @lucene.experimental */ @@ -537,90 +537,81 @@ public abstract class IndexReader implements Cloneable,Closeable { } /** - * Refreshes an IndexReader if the index has changed since this instance - * was (re)opened. - *

- * Opening an IndexReader is an expensive operation. This method can be used - * to refresh an existing IndexReader to reduce these costs. This method - * tries to only load segments that have changed or were created after the - * IndexReader was (re)opened. - *

- * If the index has not changed since this instance was (re)opened, then this - * call is a NOOP and returns this instance. Otherwise, a new instance is - * returned. The old instance is not closed and remains usable.
- *

- * If the reader is reopened, even though they share - * resources internally, it's safe to make changes - * (deletions, norms) with the new reader. All shared - * mutable state obeys "copy on write" semantics to ensure - * the changes are not seen by other readers. - *

- * You can determine whether a reader was actually reopened by comparing the - * old instance with the instance returned by this method: - * 

-   * IndexReader reader = ... 
-   * ...
-   * IndexReader newReader = r.reopen();
-   * if (newReader != reader) {
-   * ...     // reader was reopened
-   *   reader.close(); 
-   * }
-   * reader = newReader;
-   * ...
-   *
+ * If the index has changed since the provided reader was + * opened, open and return a new reader; else, return + * null. The new reader, if not null, will be the same + * type of reader as the previous one, ie an NRT reader + * will open a new NRT reader, a MultiReader will open a + * new MultiReader, etc. * - * Be sure to synchronize that code so that other threads, - * if present, can never use reader after it has been - * closed and before it's switched to newReader. + *

This method is typically far less costly than opening a + * fully new IndexReader as it shares + * resources (for example sub-readers) with the provided + * IndexReader, when possible. * - *

NOTE: If this reader is a near real-time - * reader (obtained from {@link IndexWriter#getReader()}, - * reopen() will simply call writer.getReader() again for - * you, though this may change in the future. + *

The provided reader is not closed (you are responsible + * for doing so); if a new reader is returned you also + * must eventually close it. Be sure to never close a + * reader while other threads are still using it; see + * SearcherManager in + * contrib/misc to simplify managing this. + * + *

If a new reader is returned, it's safe to make changes + * (deletions, norms) with it. All shared mutable state + * 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 + * IndexReader instance which you must eventually close */ - public synchronized IndexReader reopen() throws CorruptIndexException, IOException { - throw new UnsupportedOperationException("This reader does not support reopen()."); - } - - - /** Just like {@link #reopen()}, except you can change the - * readOnly of the original reader. If the index is - * unchanged but readOnly is different then a new reader - * will be returned. */ - public synchronized IndexReader reopen(boolean openReadOnly) throws CorruptIndexException, IOException { - throw new UnsupportedOperationException("This reader does not support reopen()."); - } - - /** Expert: reopen this reader on a specific commit point. - * This always returns a readOnly reader. If the - * specified commit point matches what this reader is - * already on, and this reader is already readOnly, then - * this same instance is returned; if it is not already - * readOnly, a readOnly clone is returned. */ - public synchronized IndexReader reopen(final IndexCommit commit) throws CorruptIndexException, IOException { - throw new UnsupportedOperationException("This reader does not support reopen(IndexCommit)."); + public static IndexReader openIfChanged(IndexReader oldReader) throws IOException { + return oldReader.doOpenIfChanged(); } /** - * Expert: returns a readonly reader, covering all - * committed as well as un-committed changes to the index. - * This provides "near real-time" searching, in that - * changes made during an IndexWriter session can be + * If the index has changed since the provided reader was + * opened, open and return a new reader, with the + * specified readOnly; else, return + * null. + * + * @see #openIfChanged(IndexReader) + */ + public static IndexReader openIfChanged(IndexReader oldReader, boolean readOnly) throws IOException { + return oldReader.doOpenIfChanged(readOnly); + } + + /** + * If the IndexCommit differs from what the + * provided reader is searching, or the provided reader is + * not already read-only, open and return a new + * readOnly=true reader; else, return null. + * + * @see #openIfChanged(IndexReader) + */ + // TODO: should you be able to specify readOnly? + public static IndexReader openIfChanged(IndexReader oldReader, IndexCommit commit) throws IOException { + return oldReader.doOpenIfChanged(commit); + } + + /** + * Expert: If there changes (committed or not) in the + * {@link IndexWriter} versus what the provided reader is + * searching, then open and return a new read-only + * IndexReader searching both committed and uncommitted + * changes from the writer; else, return null (though, the + * current implementation never returns null). + * + *

This provides "near real-time" searching, in that + * changes made during an {@link IndexWriter} session can be * quickly made available for searching without closing * the writer nor calling {@link #commit}. * - *

Note that this is functionally equivalent to calling - * {#flush} (an internal IndexWriter operation) and then using {@link IndexReader#open} to - * open a new reader. But the turnaround time of this - * method should be faster since it avoids the potentially - * costly {@link #commit}.

- * - *

You must close the {@link IndexReader} returned by - * this method once you are done using it.

- * *

It's near real-time because there is no hard * guarantee on how quickly you can get a new reader after * making changes with IndexWriter. You'll have to @@ -629,11 +620,6 @@ public abstract class IndexReader implements Cloneable,Closeable { * feature, please report back on your findings so we can * learn, improve and iterate.

* - *

The resulting reader supports {@link - * IndexReader#reopen}, but that call will simply forward - * back to this method (though this may change in the - * future).

- * *

The very first time this method is called, this * writer instance will make every effort to pool the * readers that it opens for doing merges, applying @@ -649,7 +635,7 @@ public abstract class IndexReader implements Cloneable,Closeable { *

If an addIndexes* call is running in another thread, * then this reader will only search those segments from * the foreign index that have been successfully copied - * over, so far

. + * over, so far.

* *

NOTE: Once the writer is closed, any * outstanding readers may continue to be used. However, @@ -657,9 +643,11 @@ public abstract class IndexReader implements Cloneable,Closeable { * hit an {@link AlreadyClosedException}.

* * @return IndexReader that covers entire index plus all - * changes made so far by this IndexWriter instance + * changes made so far by this IndexWriter instance, or + * null if there are no new changes * * @param writer The IndexWriter to open from + * * @param applyAllDeletes If true, all buffered deletes will * be applied (made visible) in the returned reader. If * false, the deletes are not applied but remain buffered @@ -672,7 +660,23 @@ public abstract class IndexReader implements Cloneable,Closeable { * * @lucene.experimental */ - public IndexReader reopen(IndexWriter writer, boolean applyAllDeletes) throws CorruptIndexException, IOException { + public static IndexReader openIfChanged(IndexReader oldReader, IndexWriter writer, boolean applyAllDeletes) throws IOException { + return oldReader.doOpenIfChanged(writer, applyAllDeletes); + } + + protected IndexReader doOpenIfChanged() throws CorruptIndexException, IOException { + throw new UnsupportedOperationException("This reader does not support reopen()."); + } + + protected IndexReader doOpenIfChanged(boolean openReadOnly) throws CorruptIndexException, IOException { + throw new UnsupportedOperationException("This reader does not support reopen()."); + } + + protected IndexReader doOpenIfChanged(final IndexCommit commit) throws CorruptIndexException, IOException { + throw new UnsupportedOperationException("This reader does not support reopen(IndexCommit)."); + } + + protected IndexReader doOpenIfChanged(IndexWriter writer, boolean applyAllDeletes) throws CorruptIndexException, IOException { return writer.getReader(applyAllDeletes); } @@ -687,7 +691,7 @@ public abstract class IndexReader implements Cloneable,Closeable { * changes to the index on close, but the old reader still * reflects all changes made up until it was cloned. *

- * Like {@link #reopen()}, it's safe to make changes to + * Like {@link #openIfChanged(IndexReader)}, it's safe to make changes to * either the original or the cloned reader: all shared * mutable state obeys "copy on write" semantics to ensure * the changes are not seen by other readers. @@ -808,7 +812,7 @@ public abstract class IndexReader implements Cloneable,Closeable { * implemented in the IndexReader base class. * *

If this reader is based on a Directory (ie, was - * created by calling {@link #open}, or {@link #reopen} on + * created by calling {@link #open}, or {@link #openIfChanged} on * a reader based on a Directory), then this method * returns the version recorded in the commit that the * reader opened. This version is advanced every time @@ -816,7 +820,7 @@ public abstract class IndexReader implements Cloneable,Closeable { * *

If instead this reader is a near real-time reader * (ie, obtained by a call to {@link - * IndexWriter#getReader}, or by calling {@link #reopen} + * IndexWriter#getReader}, or by calling {@link #openIfChanged} * on a near real-time reader), then this method returns * the version of the last commit done by the writer. * Note that even as further changes are made with the @@ -849,14 +853,14 @@ public abstract class IndexReader implements Cloneable,Closeable { * index since this reader was opened. * *

If this reader is based on a Directory (ie, was - * created by calling {@link #open}, or {@link #reopen} on + * created by calling {@link #open}, or {@link #openIfChanged} on * a reader based on a Directory), then this method checks * if any further commits (see {@link IndexWriter#commit} * have occurred in that directory).

* *

If instead this reader is a near real-time reader * (ie, obtained by a call to {@link - * IndexWriter#getReader}, or by calling {@link #reopen} + * IndexWriter#getReader}, or by calling {@link #openIfChanged} * on a near real-time reader), then this method checks if * either a new commmit has occurred, or any new * uncommitted changes have taken place via the writer. @@ -864,7 +868,7 @@ public abstract class IndexReader implements Cloneable,Closeable { * merging, this method will still return false.

* *

In any event, if this returns false, you should call - * {@link #reopen} to get a new reader that sees the + * {@link #openIfChanged} to get a new reader that sees the * changes.

* * @throws CorruptIndexException if the index is corrupt diff --git a/lucene/src/java/org/apache/lucene/index/MultiReader.java b/lucene/src/java/org/apache/lucene/index/MultiReader.java index dfdc0dbb052..1cef8208051 100644 --- a/lucene/src/java/org/apache/lucene/index/MultiReader.java +++ b/lucene/src/java/org/apache/lucene/index/MultiReader.java @@ -99,14 +99,14 @@ public class MultiReader extends IndexReader implements Cloneable { /** * Tries to reopen the subreaders. *
- * If one or more subreaders could be re-opened (i. e. subReader.reopen() - * returned a new instance != subReader), then a new MultiReader instance + * If one or more subreaders could be re-opened (i. e. IndexReader.openIfChanged(subReader) + * returned a new instance), then a new MultiReader instance * is returned, otherwise this instance is returned. *

* A re-opened instance might share one or more subreaders with the old * instance. Index modification operations result in undefined behavior * when performed before the old instance is closed. - * (see {@link IndexReader#reopen()}). + * (see {@link IndexReader#openIfChanged}). *

* If subreaders are shared, then the reference count of those * readers is increased to ensure that the subreaders remain open @@ -116,8 +116,8 @@ public class MultiReader extends IndexReader implements Cloneable { * @throws IOException if there is a low-level IO error */ @Override - public synchronized IndexReader reopen() throws CorruptIndexException, IOException { - return doReopen(false); + protected synchronized IndexReader doOpenIfChanged() throws CorruptIndexException, IOException { + return doOpenIfChanged(false); } /** @@ -132,7 +132,7 @@ public class MultiReader extends IndexReader implements Cloneable { @Override public synchronized Object clone() { try { - return doReopen(true); + return doOpenIfChanged(true); } catch (Exception ex) { throw new RuntimeException(ex); } @@ -146,33 +146,35 @@ public class MultiReader extends IndexReader implements Cloneable { /** * If clone is true then we clone each of the subreaders * @param doClone - * @return New IndexReader, or same one (this) if - * reopen/clone is not necessary + * @return New IndexReader, or null if open/clone is not necessary * @throws CorruptIndexException * @throws IOException */ - protected IndexReader doReopen(boolean doClone) throws CorruptIndexException, IOException { + protected IndexReader doOpenIfChanged(boolean doClone) throws CorruptIndexException, IOException { ensureOpen(); - boolean reopened = false; + boolean changed = false; IndexReader[] newSubReaders = new IndexReader[subReaders.length]; boolean success = false; try { for (int i = 0; i < subReaders.length; i++) { - if (doClone) + if (doClone) { newSubReaders[i] = (IndexReader) subReaders[i].clone(); - else - newSubReaders[i] = subReaders[i].reopen(); - // if at least one of the subreaders was updated we remember that - // and return a new MultiReader - if (newSubReaders[i] != subReaders[i]) { - reopened = true; + changed = true; + } else { + final IndexReader newSubReader = IndexReader.openIfChanged(subReaders[i]); + if (newSubReader != null) { + newSubReaders[i] = newSubReader; + changed = true; + } else { + newSubReaders[i] = subReaders[i]; + } } } success = true; } finally { - if (!success && reopened) { + if (!success && changed) { for (int i = 0; i < newSubReaders.length; i++) { if (newSubReaders[i] != subReaders[i]) { try { @@ -185,7 +187,7 @@ public class MultiReader extends IndexReader implements Cloneable { } } - if (reopened) { + if (changed) { boolean[] newDecrefOnClose = new boolean[subReaders.length]; for (int i = 0; i < subReaders.length; i++) { if (newSubReaders[i] == subReaders[i]) { @@ -197,7 +199,7 @@ public class MultiReader extends IndexReader implements Cloneable { mr.decrefOnClose = newDecrefOnClose; return mr; } else { - return this; + return null; } } diff --git a/lucene/src/java/org/apache/lucene/index/ParallelReader.java b/lucene/src/java/org/apache/lucene/index/ParallelReader.java index df4b731cd3f..04ce475dfd1 100644 --- a/lucene/src/java/org/apache/lucene/index/ParallelReader.java +++ b/lucene/src/java/org/apache/lucene/index/ParallelReader.java @@ -229,12 +229,12 @@ public class ParallelReader extends IndexReader { *
* If one or more subreaders could be re-opened (i. e. subReader.reopen() * returned a new instance != subReader), then a new ParallelReader instance - * is returned, otherwise this instance is returned. + * is returned, otherwise null is returned. *

* A re-opened instance might share one or more subreaders with the old * instance. Index modification operations result in undefined behavior * when performed before the old instance is closed. - * (see {@link IndexReader#reopen()}). + * (see {@link IndexReader#openIfChanged}). *

* If subreaders are shared, then the reference count of those * readers is increased to ensure that the subreaders remain open @@ -244,7 +244,7 @@ public class ParallelReader extends IndexReader { * @throws IOException if there is a low-level IO error */ @Override - public synchronized IndexReader reopen() throws CorruptIndexException, IOException { + protected synchronized IndexReader doOpenIfChanged() throws CorruptIndexException, IOException { // doReopen calls ensureOpen return doReopen(false); } @@ -262,15 +262,16 @@ public class ParallelReader extends IndexReader { IndexReader newReader = null; if (doClone) { newReader = (IndexReader) oldReader.clone(); + reopened = true; } else { - newReader = oldReader.reopen(); + newReader = IndexReader.openIfChanged(oldReader); + if (newReader != null) { + reopened = true; + } else { + newReader = oldReader; + } } newReaders.add(newReader); - // if at least one of the subreaders was updated we remember that - // and return a new ParallelReader - if (newReader != oldReader) { - reopened = true; - } } success = true; } finally { @@ -310,7 +311,7 @@ public class ParallelReader extends IndexReader { return pr; } else { // No subreader was refreshed - return this; + return null; } } diff --git a/lucene/src/java/org/apache/lucene/index/SegmentReader.java b/lucene/src/java/org/apache/lucene/index/SegmentReader.java index 10f1cf91445..92462bb6ed6 100644 --- a/lucene/src/java/org/apache/lucene/index/SegmentReader.java +++ b/lucene/src/java/org/apache/lucene/index/SegmentReader.java @@ -204,13 +204,13 @@ public class SegmentReader extends IndexReader implements Cloneable { } @Override - public synchronized IndexReader reopen() + protected synchronized IndexReader doOpenIfChanged() throws CorruptIndexException, IOException { return reopenSegment(si, false, readOnly); } @Override - public synchronized IndexReader reopen(boolean openReadOnly) + protected synchronized IndexReader doOpenIfChanged(boolean openReadOnly) throws CorruptIndexException, IOException { return reopenSegment(si, false, openReadOnly); } @@ -233,7 +233,7 @@ public class SegmentReader extends IndexReader implements Cloneable { // if we're cloning we need to run through the reopenSegment logic // also if both old and new readers aren't readonly, we clone to avoid sharing modifications if (normsUpToDate && deletionsUpToDate && !doClone && openReadOnly && readOnly) { - return this; + return null; } // When cloning, the incoming SegmentInfos should not diff --git a/lucene/src/java/org/apache/lucene/search/IndexSearcher.java b/lucene/src/java/org/apache/lucene/search/IndexSearcher.java index 6e1d49440e9..b70f3021440 100644 --- a/lucene/src/java/org/apache/lucene/search/IndexSearcher.java +++ b/lucene/src/java/org/apache/lucene/search/IndexSearcher.java @@ -54,7 +54,7 @@ import org.apache.lucene.util.ThreadInterruptedException; * multiple searches instead of creating a new one * per-search. If your index has changed and you wish to * see the changes reflected in searching, you should - * use {@link IndexReader#reopen} to obtain a new reader and + * use {@link IndexReader#openIfChanged} to obtain a new reader and * then create a new IndexSearcher from that. Also, for * low-latency turnaround it's best to use a near-real-time * reader ({@link IndexReader#open(IndexWriter,boolean)}). diff --git a/lucene/src/test/org/apache/lucene/index/TestIndexReader.java b/lucene/src/test/org/apache/lucene/index/TestIndexReader.java index a55bfe33fa2..ec6c8c6ad51 100644 --- a/lucene/src/test/org/apache/lucene/index/TestIndexReader.java +++ b/lucene/src/test/org/apache/lucene/index/TestIndexReader.java @@ -92,7 +92,8 @@ public class TestIndexReader extends LuceneTestCase addDocumentWithFields(writer); writer.close(); - IndexReader r3 = r2.reopen(); + IndexReader r3 = IndexReader.openIfChanged(r2); + assertNotNull(r3); assertFalse(c.equals(r3.getIndexCommit())); assertFalse(r2.getIndexCommit().isOptimized()); r3.close(); @@ -103,7 +104,8 @@ public class TestIndexReader extends LuceneTestCase writer.optimize(); writer.close(); - r3 = r2.reopen(); + r3 = IndexReader.openIfChanged(r2); + assertNotNull(r3); assertTrue(r3.getIndexCommit().isOptimized()); r2.close(); r3.close(); @@ -965,7 +967,8 @@ public class TestIndexReader extends LuceneTestCase addDocumentWithFields(writer); writer.close(); - IndexReader r2 = r.reopen(); + IndexReader r2 = IndexReader.openIfChanged(r); + assertNotNull(r2); assertFalse(c.equals(r2.getIndexCommit())); assertFalse(r2.getIndexCommit().isOptimized()); r2.close(); @@ -976,7 +979,9 @@ public class TestIndexReader extends LuceneTestCase writer.optimize(); writer.close(); - r2 = r.reopen(); + r2 = IndexReader.openIfChanged(r); + assertNotNull(r2); + assertNull(IndexReader.openIfChanged(r2)); assertTrue(r2.getIndexCommit().isOptimized()); r.close(); @@ -1011,7 +1016,8 @@ public class TestIndexReader extends LuceneTestCase writer.close(); // Make sure reopen is still readonly: - IndexReader r2 = r.reopen(); + IndexReader r2 = IndexReader.openIfChanged(r); + assertNotNull(r2); r.close(); assertFalse(r == r2); @@ -1030,7 +1036,8 @@ public class TestIndexReader extends LuceneTestCase writer.close(); // Make sure reopen to a single segment is still readonly: - IndexReader r3 = r2.reopen(); + IndexReader r3 = IndexReader.openIfChanged(r2); + assertNotNull(r3); assertFalse(r3 == r2); r2.close(); @@ -1179,7 +1186,8 @@ public class TestIndexReader extends LuceneTestCase writer.commit(); // Reopen reader1 --> reader2 - IndexReader r2 = r.reopen(); + IndexReader r2 = IndexReader.openIfChanged(r); + assertNotNull(r2); r.close(); IndexReader sub0 = r2.getSequentialSubReaders()[0]; final int[] ints2 = FieldCache.DEFAULT.getInts(sub0, "number"); @@ -1206,7 +1214,8 @@ public class TestIndexReader extends LuceneTestCase assertEquals(36, r1.getUniqueTermCount()); writer.addDocument(doc); writer.commit(); - IndexReader r2 = r.reopen(); + IndexReader r2 = IndexReader.openIfChanged(r); + assertNotNull(r2); r.close(); try { r2.getUniqueTermCount(); @@ -1253,7 +1262,9 @@ public class TestIndexReader extends LuceneTestCase writer.close(); // LUCENE-1718: ensure re-open carries over no terms index: - IndexReader r2 = r.reopen(); + IndexReader r2 = IndexReader.openIfChanged(r); + assertNotNull(r2); + assertNull(IndexReader.openIfChanged(r2)); r.close(); IndexReader[] subReaders = r2.getSequentialSubReaders(); assertEquals(2, subReaders.length); @@ -1282,8 +1293,8 @@ public class TestIndexReader extends LuceneTestCase writer.addDocument(doc); writer.prepareCommit(); assertTrue(r.isCurrent()); - IndexReader r2 = r.reopen(); - assertTrue(r == r2); + IndexReader r2 = IndexReader.openIfChanged(r); + assertNull(r2); writer.commit(); assertFalse(r.isCurrent()); writer.close(); diff --git a/lucene/src/test/org/apache/lucene/index/TestIndexReaderClone.java b/lucene/src/test/org/apache/lucene/index/TestIndexReaderClone.java index 570ee843fbb..4bcfd26500a 100644 --- a/lucene/src/test/org/apache/lucene/index/TestIndexReaderClone.java +++ b/lucene/src/test/org/apache/lucene/index/TestIndexReaderClone.java @@ -115,7 +115,8 @@ public class TestIndexReaderClone extends LuceneTestCase { TestIndexReaderReopen.modifyIndex(5, dir1); - IndexReader reader2 = reader1.reopen(); + IndexReader reader2 = IndexReader.openIfChanged(reader1); + assertNotNull(reader2); assertTrue(reader1 != reader2); assertTrue(deleteWorked(1, reader2)); @@ -156,7 +157,8 @@ public class TestIndexReaderClone extends LuceneTestCase { assertTrue(deleteWorked(1, reader)); assertEquals(docCount-1, reader.numDocs()); - IndexReader readOnlyReader = reader.reopen(true); + IndexReader readOnlyReader = IndexReader.openIfChanged(reader, true); + assertNotNull(readOnlyReader); if (!isReadOnly(readOnlyReader)) { fail("reader isn't read only"); } @@ -394,7 +396,10 @@ public class TestIndexReaderClone extends LuceneTestCase { assertDelDocsRefCountEquals(1, clonedSegmentReader); // test a reopened reader - IndexReader reopenedReader = clonedReader.reopen(); + IndexReader reopenedReader = IndexReader.openIfChanged(clonedReader); + if (reopenedReader == null) { + reopenedReader = clonedReader; + } IndexReader cloneReader2 = (IndexReader) reopenedReader.clone(); SegmentReader cloneSegmentReader2 = getOnlySegmentReader(cloneReader2); assertDelDocsRefCountEquals(2, cloneSegmentReader2); diff --git a/lucene/src/test/org/apache/lucene/index/TestIndexReaderReopen.java b/lucene/src/test/org/apache/lucene/index/TestIndexReaderReopen.java index 011f9b7162c..30605580f3c 100644 --- a/lucene/src/test/org/apache/lucene/index/TestIndexReaderReopen.java +++ b/lucene/src/test/org/apache/lucene/index/TestIndexReaderReopen.java @@ -192,8 +192,8 @@ public class TestIndexReaderReopen extends LuceneTestCase { iwriter.commit(); if (withReopen) { // reopen - IndexReader r2 = reader.reopen(); - if (reader != r2) { + IndexReader r2 = IndexReader.openIfChanged(reader); + if (r2 != null) { reader.close(); reader = r2; } @@ -468,12 +468,15 @@ public class TestIndexReaderReopen extends LuceneTestCase { modifyIndex(0, dir2); assertRefCountEquals(1 + mode, reader1); - IndexReader multiReader2 = multiReader1.reopen(); + IndexReader multiReader2 = IndexReader.openIfChanged(multiReader1); + assertNotNull(multiReader2); // index1 hasn't changed, so multiReader2 should share reader1 now with multiReader1 assertRefCountEquals(2 + mode, reader1); modifyIndex(0, dir1); - IndexReader reader2 = reader1.reopen(); + IndexReader reader2 = IndexReader.openIfChanged(reader1); + assertNotNull(reader2); + assertNull(IndexReader.openIfChanged(reader2)); assertRefCountEquals(2 + mode, reader1); if (mode == 1) { @@ -481,7 +484,8 @@ public class TestIndexReaderReopen extends LuceneTestCase { } modifyIndex(1, dir1); - IndexReader reader3 = reader2.reopen(); + IndexReader reader3 = IndexReader.openIfChanged(reader2); + assertNotNull(reader3); assertRefCountEquals(2 + mode, reader1); assertRefCountEquals(1, reader2); @@ -541,13 +545,16 @@ public class TestIndexReaderReopen extends LuceneTestCase { modifyIndex(1, dir2); assertRefCountEquals(1 + mode, reader1); - IndexReader parallelReader2 = parallelReader1.reopen(); + IndexReader parallelReader2 = IndexReader.openIfChanged(parallelReader1); + assertNotNull(parallelReader2); + assertNull(IndexReader.openIfChanged(parallelReader2)); // index1 hasn't changed, so parallelReader2 should share reader1 now with multiReader1 assertRefCountEquals(2 + mode, reader1); modifyIndex(0, dir1); modifyIndex(0, dir2); - IndexReader reader2 = reader1.reopen(); + IndexReader reader2 = IndexReader.openIfChanged(reader1); + assertNotNull(reader2); assertRefCountEquals(2 + mode, reader1); if (mode == 1) { @@ -555,7 +562,8 @@ public class TestIndexReaderReopen extends LuceneTestCase { } modifyIndex(4, dir1); - IndexReader reader3 = reader2.reopen(); + IndexReader reader3 = IndexReader.openIfChanged(reader2); + assertNotNull(reader3); assertRefCountEquals(2 + mode, reader1); assertRefCountEquals(1, reader2); @@ -609,25 +617,29 @@ public class TestIndexReaderReopen extends LuceneTestCase { modifier.deleteDocument(0); modifier.close(); - IndexReader reader2 = reader1.reopen(); + IndexReader reader2 = IndexReader.openIfChanged(reader1); + assertNotNull(reader2); modifier = IndexReader.open(dir1, false); DefaultSimilarity sim = new DefaultSimilarity(); modifier.setNorm(1, "field1", sim.encodeNormValue(50f)); modifier.setNorm(1, "field2", sim.encodeNormValue(50f)); modifier.close(); - IndexReader reader3 = reader2.reopen(); + IndexReader reader3 = IndexReader.openIfChanged(reader2); + assertNotNull(reader3); SegmentReader segmentReader3 = getOnlySegmentReader(reader3); modifier = IndexReader.open(dir1, false); modifier.deleteDocument(2); modifier.close(); - IndexReader reader4 = reader3.reopen(); + IndexReader reader4 = IndexReader.openIfChanged(reader3); + assertNotNull(reader4); modifier = IndexReader.open(dir1, false); modifier.deleteDocument(3); modifier.close(); - IndexReader reader5 = reader3.reopen(); + IndexReader reader5 = IndexReader.openIfChanged(reader3); + assertNotNull(reader5); // Now reader2-reader5 references reader1. reader1 and reader2 // share the same norms. reader3, reader4, reader5 also share norms. @@ -738,11 +750,11 @@ public class TestIndexReaderReopen extends LuceneTestCase { for (int i = 0; i < n; i++) { if (i % 2 == 0) { - IndexReader refreshed = reader.reopen(); - if (refreshed != reader) { + IndexReader refreshed = IndexReader.openIfChanged(reader); + if (refreshed != null) { readersToClose.add(reader); + reader = refreshed; } - reader = refreshed; } final IndexReader r = reader; @@ -766,7 +778,10 @@ public class TestIndexReaderReopen extends LuceneTestCase { break; } else { // not synchronized - IndexReader refreshed = r.reopen(); + IndexReader refreshed = IndexReader.openIfChanged(r); + if (refreshed == null) { + refreshed = r; + } IndexSearcher searcher = newSearcher(refreshed); ScoreDoc[] hits = searcher.search( @@ -907,7 +922,10 @@ public class TestIndexReaderReopen extends LuceneTestCase { IndexReader refreshed = null; try { - refreshed = reader.reopen(); + refreshed = IndexReader.openIfChanged(reader); + if (refreshed == null) { + refreshed = reader; + } } finally { if (refreshed == null && r != null) { // Hit exception -- close opened reader @@ -1094,7 +1112,8 @@ public class TestIndexReaderReopen extends LuceneTestCase { r2.deleteDocument(0); r2.close(); - IndexReader r3 = r1.reopen(); + IndexReader r3 = IndexReader.openIfChanged(r1); + assertNotNull(r3); assertTrue(r1 != r3); r1.close(); try { @@ -1118,7 +1137,9 @@ public class TestIndexReaderReopen extends LuceneTestCase { modifyIndex(5, dir); // Add another doc (3 segments) - IndexReader r2 = r1.reopen(); // MSR + IndexReader r2 = IndexReader.openIfChanged(r1); // MSR + assertNotNull(r2); + assertNull(IndexReader.openIfChanged(r2)); assertTrue(r1 != r2); SegmentReader sr1 = (SegmentReader) r1.getSequentialSubReaders()[0]; // Get SRs for the first segment from original @@ -1151,7 +1172,8 @@ public class TestIndexReaderReopen extends LuceneTestCase { // Add doc: modifyIndex(5, dir); - IndexReader r2 = r1.reopen(); + IndexReader r2 = IndexReader.openIfChanged(r1); + assertNotNull(r2); assertTrue(r1 != r2); IndexReader[] rs2 = r2.getSequentialSubReaders(); @@ -1207,7 +1229,8 @@ public class TestIndexReaderReopen extends LuceneTestCase { Collection commits = IndexReader.listCommits(dir); for (final IndexCommit commit : commits) { - IndexReader r2 = r.reopen(commit); + IndexReader r2 = IndexReader.openIfChanged(r, commit); + assertNotNull(r2); assertTrue(r2 != r); // Reader should be readOnly @@ -1262,7 +1285,8 @@ public class TestIndexReaderReopen extends LuceneTestCase { assertEquals(17, ints[0]); // Reopen to readonly w/ no chnages - IndexReader r3 = r.reopen(true); + IndexReader r3 = IndexReader.openIfChanged(r, true); + assertNotNull(r3); assertTrue(((DirectoryReader) r3).readOnly); r3.close(); @@ -1271,7 +1295,8 @@ public class TestIndexReaderReopen extends LuceneTestCase { writer.commit(); // Reopen reader1 --> reader2 - IndexReader r2 = r.reopen(true); + IndexReader r2 = IndexReader.openIfChanged(r, true); + assertNotNull(r2); r.close(); assertTrue(((DirectoryReader) r2).readOnly); IndexReader[] subs = r2.getSequentialSubReaders(); diff --git a/lucene/src/test/org/apache/lucene/index/TestIndexWriter.java b/lucene/src/test/org/apache/lucene/index/TestIndexWriter.java index 970b76226f6..d7e5e8e1a70 100644 --- a/lucene/src/test/org/apache/lucene/index/TestIndexWriter.java +++ b/lucene/src/test/org/apache/lucene/index/TestIndexWriter.java @@ -1402,7 +1402,8 @@ public class TestIndexWriter extends LuceneTestCase { if (iter == 1) { w.commit(); } - IndexReader r2 = r.reopen(); + IndexReader r2 = IndexReader.openIfChanged(r); + assertNotNull(r2); assertTrue(r != r2); files = Arrays.asList(dir.listAll()); diff --git a/lucene/src/test/org/apache/lucene/index/TestIndexWriterCommit.java b/lucene/src/test/org/apache/lucene/index/TestIndexWriterCommit.java index 9e6c9281cc3..02d7c6d4071 100644 --- a/lucene/src/test/org/apache/lucene/index/TestIndexWriterCommit.java +++ b/lucene/src/test/org/apache/lucene/index/TestIndexWriterCommit.java @@ -348,7 +348,8 @@ public class TestIndexWriterCommit extends LuceneTestCase { f.setValue(s); w.addDocument(doc); w.commit(); - IndexReader r2 = r.reopen(); + IndexReader r2 = IndexReader.openIfChanged(r); + assertNotNull(r2); assertTrue(r2 != r); r.close(); r = r2; @@ -390,7 +391,8 @@ public class TestIndexWriterCommit extends LuceneTestCase { IndexReader reader = IndexReader.open(dir, true); assertEquals(0, reader.numDocs()); writer.commit(); - IndexReader reader2 = reader.reopen(); + IndexReader reader2 = IndexReader.openIfChanged(reader); + assertNotNull(reader2); assertEquals(0, reader.numDocs()); assertEquals(23, reader2.numDocs()); reader.close(); @@ -530,7 +532,8 @@ public class TestIndexWriterCommit extends LuceneTestCase { writer.commit(); - IndexReader reader3 = reader.reopen(); + IndexReader reader3 = IndexReader.openIfChanged(reader); + assertNotNull(reader3); assertEquals(0, reader.numDocs()); assertEquals(0, reader2.numDocs()); assertEquals(23, reader3.numDocs()); @@ -586,10 +589,10 @@ public class TestIndexWriterCommit extends LuceneTestCase { writer.rollback(); - IndexReader reader3 = reader.reopen(); + IndexReader reader3 = IndexReader.openIfChanged(reader); + assertNull(reader3); assertEquals(0, reader.numDocs()); assertEquals(0, reader2.numDocs()); - assertEquals(0, reader3.numDocs()); reader.close(); reader2.close(); @@ -597,8 +600,6 @@ public class TestIndexWriterCommit extends LuceneTestCase { for (int i = 0; i < 17; i++) TestIndexWriter.addDoc(writer); - assertEquals(0, reader3.numDocs()); - reader3.close(); reader = IndexReader.open(dir, true); assertEquals(0, reader.numDocs()); reader.close(); diff --git a/lucene/src/test/org/apache/lucene/index/TestIndexWriterReader.java b/lucene/src/test/org/apache/lucene/index/TestIndexWriterReader.java index d44e542729c..dffc9662afc 100644 --- a/lucene/src/test/org/apache/lucene/index/TestIndexWriterReader.java +++ b/lucene/src/test/org/apache/lucene/index/TestIndexWriterReader.java @@ -677,8 +677,8 @@ public class TestIndexWriterReader extends LuceneTestCase { } ((ConcurrentMergeScheduler) writer.getConfig().getMergeScheduler()).sync(); - IndexReader r2 = r1.reopen(); - if (r2 != r1) { + IndexReader r2 = IndexReader.openIfChanged(r1); + if (r2 != null) { r1.close(); r1 = r2; } @@ -709,7 +709,7 @@ public class TestIndexWriterReader extends LuceneTestCase { assertEquals(100, searcher.search(q, 10).totalHits); searcher.close(); try { - r.reopen(); + IndexReader.openIfChanged(r); fail("failed to hit AlreadyClosedException"); } catch (AlreadyClosedException ace) { // expected @@ -766,8 +766,8 @@ public class TestIndexWriterReader extends LuceneTestCase { int lastCount = 0; while(System.currentTimeMillis() < endTime) { - IndexReader r2 = r.reopen(); - if (r2 != r) { + IndexReader r2 = IndexReader.openIfChanged(r); + if (r2 != null) { r.close(); r = r2; } @@ -783,7 +783,7 @@ public class TestIndexWriterReader extends LuceneTestCase { threads[i].join(); } // final check - IndexReader r2 = r.reopen(); + IndexReader r2 = IndexReader.openIfChanged(r); if (r2 != r) { r.close(); r = r2; @@ -856,7 +856,7 @@ public class TestIndexWriterReader extends LuceneTestCase { int sum = 0; while(System.currentTimeMillis() < endTime) { - IndexReader r2 = r.reopen(); + IndexReader r2 = IndexReader.openIfChanged(r); if (r2 != r) { r.close(); r = r2; @@ -871,8 +871,8 @@ public class TestIndexWriterReader extends LuceneTestCase { threads[i].join(); } // at least search once - IndexReader r2 = r.reopen(); - if (r2 != r) { + IndexReader r2 = IndexReader.openIfChanged(r); + if (r2 != null) { r.close(); r = r2; } diff --git a/lucene/src/test/org/apache/lucene/index/TestNRTThreads.java b/lucene/src/test/org/apache/lucene/index/TestNRTThreads.java index 975156f90c2..f40d653d5eb 100644 --- a/lucene/src/test/org/apache/lucene/index/TestNRTThreads.java +++ b/lucene/src/test/org/apache/lucene/index/TestNRTThreads.java @@ -41,8 +41,8 @@ public class TestNRTThreads extends ThreadedIndexingAndSearchingTestCase { if (VERBOSE) { System.out.println("TEST: now reopen r=" + r); } - final IndexReader r2 = r.reopen(); - if (r != r2) { + final IndexReader r2 = IndexReader.openIfChanged(r); + if (r2 != null) { r.close(); r = r2; } diff --git a/lucene/src/test/org/apache/lucene/index/TestNeverDelete.java b/lucene/src/test/org/apache/lucene/index/TestNeverDelete.java index 46183cf4d64..36ad4f1ea74 100644 --- a/lucene/src/test/org/apache/lucene/index/TestNeverDelete.java +++ b/lucene/src/test/org/apache/lucene/index/TestNeverDelete.java @@ -92,8 +92,8 @@ public class TestNeverDelete extends LuceneTestCase { for(String fileName : allFiles) { assertTrue("file " + fileName + " does not exist", d.fileExists(fileName)); } - IndexReader r2 = r.reopen(); - if (r2 != r) { + IndexReader r2 = IndexReader.openIfChanged(r); + if (r2 != null) { r.close(); r = r2; } diff --git a/lucene/src/test/org/apache/lucene/index/TestRollingUpdates.java b/lucene/src/test/org/apache/lucene/index/TestRollingUpdates.java index 62ca6ed3214..aaaca02d246 100644 --- a/lucene/src/test/org/apache/lucene/index/TestRollingUpdates.java +++ b/lucene/src/test/org/apache/lucene/index/TestRollingUpdates.java @@ -135,8 +135,8 @@ public class TestRollingUpdates extends LuceneTestCase { if (open == null) { open = IndexReader.open(writer, true); } - IndexReader reader = open.reopen(); - if (reader != open) { + IndexReader reader = IndexReader.openIfChanged(open); + if (reader != null) { open.close(); open = reader; } diff --git a/lucene/src/test/org/apache/lucene/index/TestStressNRT.java b/lucene/src/test/org/apache/lucene/index/TestStressNRT.java index 8e3fc5b9df3..117ce004224 100644 --- a/lucene/src/test/org/apache/lucene/index/TestStressNRT.java +++ b/lucene/src/test/org/apache/lucene/index/TestStressNRT.java @@ -147,7 +147,7 @@ public class TestStressNRT extends LuceneTestCase { if (VERBOSE) { System.out.println("TEST: " + Thread.currentThread().getName() + ": reopen reader=" + oldReader + " version=" + version); } - newReader = oldReader.reopen(writer.w, true); + newReader = IndexReader.openIfChanged(oldReader, writer.w, true); } } else { // assertU(commit()); @@ -158,13 +158,14 @@ public class TestStressNRT extends LuceneTestCase { if (VERBOSE) { System.out.println("TEST: " + Thread.currentThread().getName() + ": now reopen after commit"); } - newReader = oldReader.reopen(); + newReader = IndexReader.openIfChanged(oldReader); } // Code below assumes newReader comes w/ // extra ref: - if (newReader == oldReader) { - newReader.incRef(); + if (newReader == null) { + oldReader.incRef(); + newReader = oldReader; } oldReader.decRef(); diff --git a/lucene/src/test/org/apache/lucene/search/TestCachingSpanFilter.java b/lucene/src/test/org/apache/lucene/search/TestCachingSpanFilter.java index bb001a1db32..702c2fa350c 100644 --- a/lucene/src/test/org/apache/lucene/search/TestCachingSpanFilter.java +++ b/lucene/src/test/org/apache/lucene/search/TestCachingSpanFilter.java @@ -152,10 +152,12 @@ public class TestCachingSpanFilter extends LuceneTestCase { private static IndexReader refreshReader(IndexReader reader) throws IOException { IndexReader oldReader = reader; - reader = reader.reopen(); - if (reader != oldReader) { + reader = IndexReader.openIfChanged(reader); + if (reader != null) { oldReader.close(); + return reader; + } else { + return oldReader; } - return reader; } } diff --git a/lucene/src/test/org/apache/lucene/search/TestCachingWrapperFilter.java b/lucene/src/test/org/apache/lucene/search/TestCachingWrapperFilter.java index 3f2c99e0c89..48bf9915ef1 100644 --- a/lucene/src/test/org/apache/lucene/search/TestCachingWrapperFilter.java +++ b/lucene/src/test/org/apache/lucene/search/TestCachingWrapperFilter.java @@ -304,10 +304,12 @@ public class TestCachingWrapperFilter extends LuceneTestCase { private static IndexReader refreshReader(IndexReader reader) throws IOException { IndexReader oldReader = reader; - reader = reader.reopen(); - if (reader != oldReader) { + reader = IndexReader.openIfChanged(reader); + if (reader != null) { oldReader.close(); + return reader; + } else { + return oldReader; } - return reader; } } diff --git a/modules/benchmark/src/java/org/apache/lucene/benchmark/byTask/tasks/NearRealtimeReaderTask.java b/modules/benchmark/src/java/org/apache/lucene/benchmark/byTask/tasks/NearRealtimeReaderTask.java index 47ea3f428d9..b77525cc685 100644 --- a/modules/benchmark/src/java/org/apache/lucene/benchmark/byTask/tasks/NearRealtimeReaderTask.java +++ b/modules/benchmark/src/java/org/apache/lucene/benchmark/byTask/tasks/NearRealtimeReaderTask.java @@ -77,8 +77,8 @@ public class NearRealtimeReaderTask extends PerfTask { } t = System.currentTimeMillis(); - final IndexReader newReader = r.reopen(); - if (r != newReader) { + final IndexReader newReader = IndexReader.openIfChanged(r); + if (newReader != null) { final int delay = (int) (System.currentTimeMillis()-t); if (reopenTimes.length == reopenCount) { reopenTimes = ArrayUtil.grow(reopenTimes, 1+reopenCount); diff --git a/modules/benchmark/src/java/org/apache/lucene/benchmark/byTask/tasks/ReopenReaderTask.java b/modules/benchmark/src/java/org/apache/lucene/benchmark/byTask/tasks/ReopenReaderTask.java index 10198c51fd6..4eb2a9d9765 100644 --- a/modules/benchmark/src/java/org/apache/lucene/benchmark/byTask/tasks/ReopenReaderTask.java +++ b/modules/benchmark/src/java/org/apache/lucene/benchmark/byTask/tasks/ReopenReaderTask.java @@ -34,8 +34,8 @@ public class ReopenReaderTask extends PerfTask { @Override public int doLogic() throws IOException { IndexReader r = getRunData().getIndexReader(); - IndexReader nr = r.reopen(); - if (nr != r) { + IndexReader nr = IndexReader.openIfChanged(r); + if (nr != null) { getRunData().setIndexReader(nr); nr.decRef(); } diff --git a/modules/facet/src/java/org/apache/lucene/facet/taxonomy/lucene/LuceneTaxonomyReader.java b/modules/facet/src/java/org/apache/lucene/facet/taxonomy/lucene/LuceneTaxonomyReader.java index eb8a1556ef7..d4bfc221f64 100644 --- a/modules/facet/src/java/org/apache/lucene/facet/taxonomy/lucene/LuceneTaxonomyReader.java +++ b/modules/facet/src/java/org/apache/lucene/facet/taxonomy/lucene/LuceneTaxonomyReader.java @@ -368,8 +368,8 @@ public class LuceneTaxonomyReader implements TaxonomyReader { // safely read indexReader without holding the write lock, because // no other thread can be writing at this time (this method is the // only possible writer, and it is "synchronized" to avoid this case). - IndexReader r2 = indexReader.reopen(); - if (indexReader != r2) { + IndexReader r2 = IndexReader.openIfChanged(indexReader); + if (r2 != null) { IndexReader oldreader = indexReader; // we can close the old searcher, but need to synchronize this // so that we don't close it in the middle that another routine diff --git a/modules/facet/src/java/org/apache/lucene/facet/taxonomy/lucene/LuceneTaxonomyWriter.java b/modules/facet/src/java/org/apache/lucene/facet/taxonomy/lucene/LuceneTaxonomyWriter.java index 2b87dde629a..8da8920d9c2 100644 --- a/modules/facet/src/java/org/apache/lucene/facet/taxonomy/lucene/LuceneTaxonomyWriter.java +++ b/modules/facet/src/java/org/apache/lucene/facet/taxonomy/lucene/LuceneTaxonomyWriter.java @@ -564,8 +564,8 @@ public class LuceneTaxonomyWriter implements TaxonomyWriter { private synchronized void refreshReader() throws IOException { if (reader != null) { - IndexReader r2 = reader.reopen(); - if (reader != r2) { + IndexReader r2 = IndexReader.openIfChanged(reader); + if (r2 != null) { reader.close(); reader = r2; } diff --git a/modules/facet/src/test/org/apache/lucene/facet/search/TestTotalFacetCountsCache.java b/modules/facet/src/test/org/apache/lucene/facet/search/TestTotalFacetCountsCache.java index b1e28c4a876..8b54182d9b0 100644 --- a/modules/facet/src/test/org/apache/lucene/facet/search/TestTotalFacetCountsCache.java +++ b/modules/facet/src/test/org/apache/lucene/facet/search/TestTotalFacetCountsCache.java @@ -299,7 +299,8 @@ public class TestTotalFacetCountsCache extends LuceneTestCase { writers[0].taxWriter.close(); readers[0].taxReader.refresh(); - IndexReader r2 = readers[0].indexReader.reopen(); + IndexReader r2 = IndexReader.openIfChanged(readers[0].indexReader); + assertNotNull(r2); // Hold on to the 'original' reader so we can do some checks with it IndexReader origReader = null; diff --git a/modules/facet/src/test/org/apache/lucene/facet/taxonomy/lucene/TestIndexClose.java b/modules/facet/src/test/org/apache/lucene/facet/taxonomy/lucene/TestIndexClose.java index 352c12dab79..ef1276bae50 100644 --- a/modules/facet/src/test/org/apache/lucene/facet/taxonomy/lucene/TestIndexClose.java +++ b/modules/facet/src/test/org/apache/lucene/facet/taxonomy/lucene/TestIndexClose.java @@ -160,10 +160,10 @@ public class TestIndexClose extends LuceneTestCase { // System.err.println("opened "+mynum); } @Override - public synchronized IndexReader reopen() throws CorruptIndexException, IOException { - IndexReader n = in.reopen(); - if (n==in) { - return this; + protected synchronized IndexReader doOpenIfChanged() throws CorruptIndexException, IOException { + IndexReader n = IndexReader.openIfChanged(in); + if (n == null) { + return null; } return new InstrumentedIndexReader(n); } diff --git a/solr/core/src/java/org/apache/solr/core/SolrCore.java b/solr/core/src/java/org/apache/solr/core/SolrCore.java index b65a21437ef..c932d343807 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrCore.java +++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java @@ -1115,10 +1115,11 @@ public final class SolrCore implements SolrInfoMBean { IndexReader currentReader = newestSearcher.get().getIndexReader(); IndexReader newReader; - newReader = currentReader.reopen(); + newReader = IndexReader.openIfChanged(currentReader); - if (newReader == currentReader) { + if (newReader == null) { currentReader.incRef(); + newReader = currentReader; } tmp = new SolrIndexSearcher(this, schema, "main", newReader, true, true, true, directoryFactory); diff --git a/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java b/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java index 2c4553e382a..a4902c58a02 100644 --- a/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java +++ b/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java @@ -385,11 +385,12 @@ public class DirectUpdateHandler2 extends UpdateHandler { IndexReader currentReader = previousSearcher.getIndexReader(); IndexReader newReader; - newReader = currentReader.reopen(indexWriterProvider.getIndexWriter(core), true); + newReader = IndexReader.openIfChanged(currentReader, indexWriterProvider.getIndexWriter(core), true); - if (newReader == currentReader) { + if (newReader == null) { currentReader.incRef(); + newReader = currentReader; } return new SolrIndexSearcher(core, schema, "main", newReader, true, true, true, core.getDirectoryFactory());