Index: solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java =================================================================== --- solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java (revision 1176096) +++ solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java (working copy) @@ -385,11 +385,12 @@ IndexReader currentReader = previousSearcher.getIndexReader(); IndexReader newReader; - newReader = currentReader.reopen(indexWriterProvider.getIndexWriter(core), true); + newReader = currentReader.reopenIfChanged(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()); Index: solr/core/src/java/org/apache/solr/core/SolrCore.java =================================================================== --- solr/core/src/java/org/apache/solr/core/SolrCore.java (revision 1176096) +++ solr/core/src/java/org/apache/solr/core/SolrCore.java (working copy) @@ -1115,10 +1115,11 @@ IndexReader currentReader = newestSearcher.get().getIndexReader(); IndexReader newReader; - newReader = currentReader.reopen(); + newReader = currentReader.reopenIfChanged(); - if (newReader == currentReader) { + if (newReader == null) { currentReader.incRef(); + newReader = currentReader; } tmp = new SolrIndexSearcher(this, schema, "main", newReader, true, true, true, directoryFactory); Index: modules/facet/src/test/org/apache/lucene/facet/search/TestTotalFacetCountsCache.java =================================================================== --- modules/facet/src/test/org/apache/lucene/facet/search/TestTotalFacetCountsCache.java (revision 1176096) +++ modules/facet/src/test/org/apache/lucene/facet/search/TestTotalFacetCountsCache.java (working copy) @@ -299,7 +299,8 @@ writers[0].taxWriter.close(); readers[0].taxReader.refresh(); - IndexReader r2 = readers[0].indexReader.reopen(); + IndexReader r2 = readers[0].indexReader.reopenIfChanged(); + assertNotNull(r2); // Hold on to the 'original' reader so we can do some checks with it IndexReader origReader = null; Index: modules/facet/src/test/org/apache/lucene/facet/taxonomy/lucene/TestIndexClose.java =================================================================== --- modules/facet/src/test/org/apache/lucene/facet/taxonomy/lucene/TestIndexClose.java (revision 1176096) +++ modules/facet/src/test/org/apache/lucene/facet/taxonomy/lucene/TestIndexClose.java (working copy) @@ -160,10 +160,10 @@ // System.err.println("opened "+mynum); } @Override - public synchronized IndexReader reopen() throws CorruptIndexException, IOException { - IndexReader n = in.reopen(); - if (n==in) { - return this; + public synchronized IndexReader reopenIfChanged() throws CorruptIndexException, IOException { + IndexReader n = in.reopenIfChanged(); + if (n == null) { + return null; } return new InstrumentedIndexReader(n); } Index: modules/facet/src/java/org/apache/lucene/facet/taxonomy/lucene/LuceneTaxonomyWriter.java =================================================================== --- modules/facet/src/java/org/apache/lucene/facet/taxonomy/lucene/LuceneTaxonomyWriter.java (revision 1176096) +++ modules/facet/src/java/org/apache/lucene/facet/taxonomy/lucene/LuceneTaxonomyWriter.java (working copy) @@ -564,8 +564,8 @@ private synchronized void refreshReader() throws IOException { if (reader != null) { - IndexReader r2 = reader.reopen(); - if (reader != r2) { + IndexReader r2 = reader.reopenIfChanged(); + if (r2 != null) { reader.close(); reader = r2; } Index: modules/facet/src/java/org/apache/lucene/facet/taxonomy/lucene/LuceneTaxonomyReader.java =================================================================== --- modules/facet/src/java/org/apache/lucene/facet/taxonomy/lucene/LuceneTaxonomyReader.java (revision 1176096) +++ modules/facet/src/java/org/apache/lucene/facet/taxonomy/lucene/LuceneTaxonomyReader.java (working copy) @@ -368,8 +368,8 @@ // 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.reopenIfChanged(); + 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 Index: modules/benchmark/src/java/org/apache/lucene/benchmark/byTask/tasks/NearRealtimeReaderTask.java =================================================================== --- modules/benchmark/src/java/org/apache/lucene/benchmark/byTask/tasks/NearRealtimeReaderTask.java (revision 1176096) +++ modules/benchmark/src/java/org/apache/lucene/benchmark/byTask/tasks/NearRealtimeReaderTask.java (working copy) @@ -77,8 +77,8 @@ } t = System.currentTimeMillis(); - final IndexReader newReader = r.reopen(); - if (r != newReader) { + final IndexReader newReader = r.reopenIfChanged(); + if (newReader != null) { final int delay = (int) (System.currentTimeMillis()-t); if (reopenTimes.length == reopenCount) { reopenTimes = ArrayUtil.grow(reopenTimes, 1+reopenCount); Index: modules/benchmark/src/java/org/apache/lucene/benchmark/byTask/tasks/ReopenReaderTask.java =================================================================== --- modules/benchmark/src/java/org/apache/lucene/benchmark/byTask/tasks/ReopenReaderTask.java (revision 1176096) +++ modules/benchmark/src/java/org/apache/lucene/benchmark/byTask/tasks/ReopenReaderTask.java (working copy) @@ -34,8 +34,8 @@ @Override public int doLogic() throws IOException { IndexReader r = getRunData().getIndexReader(); - IndexReader nr = r.reopen(); - if (nr != r) { + IndexReader nr = r.reopenIfChanged(); + if (nr != null) { getRunData().setIndexReader(nr); nr.decRef(); } Index: lucene/contrib/misc/src/test/org/apache/lucene/index/TestMultiPassIndexSplitter.java =================================================================== --- lucene/contrib/misc/src/test/org/apache/lucene/index/TestMultiPassIndexSplitter.java (revision 1176096) +++ lucene/contrib/misc/src/test/org/apache/lucene/index/TestMultiPassIndexSplitter.java (working copy) @@ -45,9 +45,6 @@ input = IndexReader.open(dir, false); // delete the last doc input.deleteDocument(input.maxDoc() - 1); - IndexReader inputOld = input; - input = input.reopen(true); - inputOld.close(); } @Override Index: lucene/contrib/misc/src/test/org/apache/lucene/store/TestNRTCachingDirectory.java =================================================================== --- lucene/contrib/misc/src/test/org/apache/lucene/store/TestNRTCachingDirectory.java (revision 1176096) +++ lucene/contrib/misc/src/test/org/apache/lucene/store/TestNRTCachingDirectory.java (working copy) @@ -65,8 +65,8 @@ if (r == null) { r = IndexReader.open(w.w, false); } else { - final IndexReader r2 = r.reopen(); - if (r2 != r) { + final IndexReader r2 = r.reopenIfChanged(); + if (r2 != null) { r.close(); r = r2; } Index: lucene/contrib/misc/src/java/org/apache/lucene/search/SearcherManager.java =================================================================== --- lucene/contrib/misc/src/java/org/apache/lucene/search/SearcherManager.java (revision 1176096) +++ lucene/contrib/misc/src/java/org/apache/lucene/search/SearcherManager.java (working copy) @@ -110,7 +110,7 @@ } /** You must call this, periodically, to perform a - * reopen. This calls {@link IndexReader#reopen} on the + * reopen. This calls {@link IndexReader#reopenIfChanged} 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. @@ -138,8 +138,8 @@ // threads just return immediately: if (!reopening.getAndSet(true)) { try { - IndexReader newReader = currentSearcher.getIndexReader().reopen(); - if (newReader != currentSearcher.getIndexReader()) { + IndexReader newReader = currentSearcher.getIndexReader().reopenIfChanged(); + if (newReader != null) { IndexSearcher newSearcher = new IndexSearcher(newReader, es); if (warmer != null) { warmer.warm(newSearcher); Index: lucene/contrib/misc/src/java/org/apache/lucene/index/NRTManager.java =================================================================== --- lucene/contrib/misc/src/java/org/apache/lucene/index/NRTManager.java (revision 1176096) +++ lucene/contrib/misc/src/java/org/apache/lucene/index/NRTManager.java (working copy) @@ -307,7 +307,14 @@ // 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 = startSearcher.getIndexReader().reopenIfChanged(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(); + } final IndexSearcher nextSearcher = new IndexSearcher(nextReader, es); if (warmer != null) { Index: lucene/CHANGES.txt =================================================================== --- lucene/CHANGES.txt (revision 1176096) +++ lucene/CHANGES.txt (working copy) @@ -604,6 +604,10 @@ 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 reopenIfChanged, + and now returns null (instead of the old reader) if there are no + changes in the index. + Bug fixes * LUCENE-3412: SloppyPhraseScorer was returning non-deterministic results Index: lucene/MIGRATE.txt =================================================================== --- lucene/MIGRATE.txt (revision 1176096) +++ lucene/MIGRATE.txt (working copy) @@ -521,4 +521,9 @@ * 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 changed to + IndexReader.reopenIfChanged. Also, the new method now returns null + if there were no changes to the index, to prevent the common pitfall + of accidentally closing the old reader. Index: lucene/src/test/org/apache/lucene/search/TestCachingWrapperFilter.java =================================================================== --- lucene/src/test/org/apache/lucene/search/TestCachingWrapperFilter.java (revision 1176096) +++ lucene/src/test/org/apache/lucene/search/TestCachingWrapperFilter.java (working copy) @@ -304,10 +304,12 @@ private static IndexReader refreshReader(IndexReader reader) throws IOException { IndexReader oldReader = reader; - reader = reader.reopen(); - if (reader != oldReader) { + reader = reader.reopenIfChanged(); + if (reader != null) { oldReader.close(); + return reader; + } else { + return oldReader; } - return reader; } } Index: lucene/src/test/org/apache/lucene/search/TestCachingSpanFilter.java =================================================================== --- lucene/src/test/org/apache/lucene/search/TestCachingSpanFilter.java (revision 1176096) +++ lucene/src/test/org/apache/lucene/search/TestCachingSpanFilter.java (working copy) @@ -152,10 +152,12 @@ private static IndexReader refreshReader(IndexReader reader) throws IOException { IndexReader oldReader = reader; - reader = reader.reopen(); - if (reader != oldReader) { + reader = reader.reopenIfChanged(); + if (reader != null) { oldReader.close(); + return reader; + } else { + return oldReader; } - return reader; } } Index: lucene/src/test/org/apache/lucene/index/TestIndexWriterCommit.java =================================================================== --- lucene/src/test/org/apache/lucene/index/TestIndexWriterCommit.java (revision 1176096) +++ lucene/src/test/org/apache/lucene/index/TestIndexWriterCommit.java (working copy) @@ -348,7 +348,8 @@ f.setValue(s); w.addDocument(doc); w.commit(); - IndexReader r2 = r.reopen(); + IndexReader r2 = r.reopenIfChanged(); + assertNotNull(r2); assertTrue(r2 != r); r.close(); r = r2; @@ -390,7 +391,8 @@ IndexReader reader = IndexReader.open(dir, true); assertEquals(0, reader.numDocs()); writer.commit(); - IndexReader reader2 = reader.reopen(); + IndexReader reader2 = reader.reopenIfChanged(); + assertNotNull(reader2); assertEquals(0, reader.numDocs()); assertEquals(23, reader2.numDocs()); reader.close(); @@ -530,7 +532,8 @@ writer.commit(); - IndexReader reader3 = reader.reopen(); + IndexReader reader3 = reader.reopenIfChanged(); + assertNotNull(reader3); assertEquals(0, reader.numDocs()); assertEquals(0, reader2.numDocs()); assertEquals(23, reader3.numDocs()); @@ -586,10 +589,10 @@ writer.rollback(); - IndexReader reader3 = reader.reopen(); + IndexReader reader3 = reader.reopenIfChanged(); + assertNull(reader3); assertEquals(0, reader.numDocs()); assertEquals(0, reader2.numDocs()); - assertEquals(0, reader3.numDocs()); reader.close(); reader2.close(); @@ -597,8 +600,6 @@ 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(); Index: lucene/src/test/org/apache/lucene/index/TestStressNRT.java =================================================================== --- lucene/src/test/org/apache/lucene/index/TestStressNRT.java (revision 1176096) +++ lucene/src/test/org/apache/lucene/index/TestStressNRT.java (working copy) @@ -147,7 +147,7 @@ if (VERBOSE) { System.out.println("TEST: " + Thread.currentThread().getName() + ": reopen reader=" + oldReader + " version=" + version); } - newReader = oldReader.reopen(writer.w, true); + newReader = oldReader.reopenIfChanged(writer.w, true); } } else { // assertU(commit()); @@ -158,13 +158,14 @@ if (VERBOSE) { System.out.println("TEST: " + Thread.currentThread().getName() + ": now reopen after commit"); } - newReader = oldReader.reopen(); + newReader = oldReader.reopenIfChanged(); } // Code below assumes newReader comes w/ // extra ref: - if (newReader == oldReader) { - newReader.incRef(); + if (newReader == null) { + oldReader.incRef(); + newReader = oldReader; } oldReader.decRef(); Index: lucene/src/test/org/apache/lucene/index/TestNRTThreads.java =================================================================== --- lucene/src/test/org/apache/lucene/index/TestNRTThreads.java (revision 1176096) +++ lucene/src/test/org/apache/lucene/index/TestNRTThreads.java (working copy) @@ -41,8 +41,8 @@ if (VERBOSE) { System.out.println("TEST: now reopen r=" + r); } - final IndexReader r2 = r.reopen(); - if (r != r2) { + final IndexReader r2 = r.reopenIfChanged(); + if (r2 != null) { r.close(); r = r2; } Index: lucene/src/test/org/apache/lucene/index/TestIndexWriterReader.java =================================================================== --- lucene/src/test/org/apache/lucene/index/TestIndexWriterReader.java (revision 1176096) +++ lucene/src/test/org/apache/lucene/index/TestIndexWriterReader.java (working copy) @@ -677,8 +677,8 @@ } ((ConcurrentMergeScheduler) writer.getConfig().getMergeScheduler()).sync(); - IndexReader r2 = r1.reopen(); - if (r2 != r1) { + IndexReader r2 = r1.reopenIfChanged(); + if (r2 != null) { r1.close(); r1 = r2; } @@ -709,7 +709,7 @@ assertEquals(100, searcher.search(q, 10).totalHits); searcher.close(); try { - r.reopen(); + r.reopenIfChanged(); fail("failed to hit AlreadyClosedException"); } catch (AlreadyClosedException ace) { // expected @@ -766,8 +766,8 @@ int lastCount = 0; while(System.currentTimeMillis() < endTime) { - IndexReader r2 = r.reopen(); - if (r2 != r) { + IndexReader r2 = r.reopenIfChanged(); + if (r2 != null) { r.close(); r = r2; } @@ -783,7 +783,7 @@ threads[i].join(); } // final check - IndexReader r2 = r.reopen(); + IndexReader r2 = r.reopenIfChanged(); if (r2 != r) { r.close(); r = r2; @@ -856,7 +856,7 @@ int sum = 0; while(System.currentTimeMillis() < endTime) { - IndexReader r2 = r.reopen(); + IndexReader r2 = r.reopenIfChanged(); if (r2 != r) { r.close(); r = r2; @@ -871,8 +871,8 @@ threads[i].join(); } // at least search once - IndexReader r2 = r.reopen(); - if (r2 != r) { + IndexReader r2 = r.reopenIfChanged(); + if (r2 != null) { r.close(); r = r2; } Index: lucene/src/test/org/apache/lucene/index/TestRollingUpdates.java =================================================================== --- lucene/src/test/org/apache/lucene/index/TestRollingUpdates.java (revision 1176096) +++ lucene/src/test/org/apache/lucene/index/TestRollingUpdates.java (working copy) @@ -135,8 +135,8 @@ if (open == null) { open = IndexReader.open(writer, true); } - IndexReader reader = open.reopen(); - if (reader != open) { + IndexReader reader = open.reopenIfChanged(); + if (reader != null) { open.close(); open = reader; } Index: lucene/src/test/org/apache/lucene/index/TestIndexReaderClone.java =================================================================== --- lucene/src/test/org/apache/lucene/index/TestIndexReaderClone.java (revision 1176096) +++ lucene/src/test/org/apache/lucene/index/TestIndexReaderClone.java (working copy) @@ -115,7 +115,8 @@ TestIndexReaderReopen.modifyIndex(5, dir1); - IndexReader reader2 = reader1.reopen(); + IndexReader reader2 = reader1.reopenIfChanged(); + assertNotNull(reader2); assertTrue(reader1 != reader2); assertTrue(deleteWorked(1, reader2)); @@ -156,7 +157,8 @@ assertTrue(deleteWorked(1, reader)); assertEquals(docCount-1, reader.numDocs()); - IndexReader readOnlyReader = reader.reopen(true); + IndexReader readOnlyReader = reader.reopenIfChanged(true); + assertNotNull(readOnlyReader); if (!isReadOnly(readOnlyReader)) { fail("reader isn't read only"); } @@ -394,7 +396,10 @@ assertDelDocsRefCountEquals(1, clonedSegmentReader); // test a reopened reader - IndexReader reopenedReader = clonedReader.reopen(); + IndexReader reopenedReader = clonedReader.reopenIfChanged(); + if (reopenedReader == null) { + reopenedReader = clonedReader; + } IndexReader cloneReader2 = (IndexReader) reopenedReader.clone(); SegmentReader cloneSegmentReader2 = getOnlySegmentReader(cloneReader2); assertDelDocsRefCountEquals(2, cloneSegmentReader2); Index: lucene/src/test/org/apache/lucene/index/TestIndexReader.java =================================================================== --- lucene/src/test/org/apache/lucene/index/TestIndexReader.java (revision 1176096) +++ lucene/src/test/org/apache/lucene/index/TestIndexReader.java (working copy) @@ -91,7 +91,8 @@ addDocumentWithFields(writer); writer.close(); - IndexReader r3 = r2.reopen(); + IndexReader r3 = r2.reopenIfChanged(); + assertNotNull(r3); assertFalse(c.equals(r3.getIndexCommit())); assertFalse(r2.getIndexCommit().isOptimized()); r3.close(); @@ -102,7 +103,8 @@ writer.optimize(); writer.close(); - r3 = r2.reopen(); + r3 = r2.reopenIfChanged(); + assertNotNull(r3); assertTrue(r3.getIndexCommit().isOptimized()); r2.close(); r3.close(); @@ -964,7 +966,8 @@ addDocumentWithFields(writer); writer.close(); - IndexReader r2 = r.reopen(); + IndexReader r2 = r.reopenIfChanged(); + assertNotNull(r2); assertFalse(c.equals(r2.getIndexCommit())); assertFalse(r2.getIndexCommit().isOptimized()); r2.close(); @@ -975,7 +978,9 @@ writer.optimize(); writer.close(); - r2 = r.reopen(); + r2 = r.reopenIfChanged(); + assertNotNull(r2); + assertNull(r2.reopenIfChanged()); assertTrue(r2.getIndexCommit().isOptimized()); r.close(); @@ -1010,7 +1015,8 @@ writer.close(); // Make sure reopen is still readonly: - IndexReader r2 = r.reopen(); + IndexReader r2 = r.reopenIfChanged(); + assertNotNull(r2); r.close(); assertFalse(r == r2); @@ -1029,7 +1035,8 @@ writer.close(); // Make sure reopen to a single segment is still readonly: - IndexReader r3 = r2.reopen(); + IndexReader r3 = r2.reopenIfChanged(); + assertNotNull(r3); assertFalse(r3 == r2); r2.close(); @@ -1178,7 +1185,8 @@ writer.commit(); // Reopen reader1 --> reader2 - IndexReader r2 = r.reopen(); + IndexReader r2 = r.reopenIfChanged(); + assertNotNull(r2); r.close(); IndexReader sub0 = r2.getSequentialSubReaders()[0]; final int[] ints2 = FieldCache.DEFAULT.getInts(sub0, "number"); @@ -1205,7 +1213,8 @@ assertEquals(36, r1.getUniqueTermCount()); writer.addDocument(doc); writer.commit(); - IndexReader r2 = r.reopen(); + IndexReader r2 = r.reopenIfChanged(); + assertNotNull(r2); r.close(); try { r2.getUniqueTermCount(); @@ -1252,7 +1261,9 @@ writer.close(); // LUCENE-1718: ensure re-open carries over no terms index: - IndexReader r2 = r.reopen(); + IndexReader r2 = r.reopenIfChanged(); + assertNotNull(r2); + assertNull(r2.reopenIfChanged()); r.close(); IndexReader[] subReaders = r2.getSequentialSubReaders(); assertEquals(2, subReaders.length); @@ -1281,8 +1292,8 @@ writer.addDocument(doc); writer.prepareCommit(); assertTrue(r.isCurrent()); - IndexReader r2 = r.reopen(); - assertTrue(r == r2); + IndexReader r2 = r.reopenIfChanged(); + assertNull(r2); writer.commit(); assertFalse(r.isCurrent()); writer.close(); Index: lucene/src/test/org/apache/lucene/index/TestIndexWriter.java =================================================================== --- lucene/src/test/org/apache/lucene/index/TestIndexWriter.java (revision 1176096) +++ lucene/src/test/org/apache/lucene/index/TestIndexWriter.java (working copy) @@ -1402,7 +1402,8 @@ if (iter == 1) { w.commit(); } - IndexReader r2 = r.reopen(); + IndexReader r2 = r.reopenIfChanged(); + assertNotNull(r2); assertTrue(r != r2); files = Arrays.asList(dir.listAll()); Index: lucene/src/test/org/apache/lucene/index/TestNeverDelete.java =================================================================== --- lucene/src/test/org/apache/lucene/index/TestNeverDelete.java (revision 1176096) +++ lucene/src/test/org/apache/lucene/index/TestNeverDelete.java (working copy) @@ -92,8 +92,8 @@ for(String fileName : allFiles) { assertTrue("file " + fileName + " does not exist", d.fileExists(fileName)); } - IndexReader r2 = r.reopen(); - if (r2 != r) { + IndexReader r2 = r.reopenIfChanged(); + if (r2 != null) { r.close(); r = r2; } Index: lucene/src/test/org/apache/lucene/index/TestIndexReaderReopen.java =================================================================== --- lucene/src/test/org/apache/lucene/index/TestIndexReaderReopen.java (revision 1176096) +++ lucene/src/test/org/apache/lucene/index/TestIndexReaderReopen.java (working copy) @@ -194,8 +194,8 @@ iwriter.commit(); if (withReopen) { // reopen - IndexReader r2 = reader.reopen(); - if (reader != r2) { + IndexReader r2 = reader.reopenIfChanged(); + if (r2 != null) { reader.close(); reader = r2; } @@ -470,12 +470,15 @@ modifyIndex(0, dir2); assertRefCountEquals(1 + mode, reader1); - IndexReader multiReader2 = multiReader1.reopen(); + IndexReader multiReader2 = multiReader1.reopenIfChanged(); + 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 = reader1.reopenIfChanged(); + assertNotNull(reader2); + assertNull(reader2.reopenIfChanged()); assertRefCountEquals(2 + mode, reader1); if (mode == 1) { @@ -483,7 +486,8 @@ } modifyIndex(1, dir1); - IndexReader reader3 = reader2.reopen(); + IndexReader reader3 = reader2.reopenIfChanged(); + assertNotNull(reader3); assertRefCountEquals(2 + mode, reader1); assertRefCountEquals(1, reader2); @@ -543,13 +547,16 @@ modifyIndex(1, dir2); assertRefCountEquals(1 + mode, reader1); - IndexReader parallelReader2 = parallelReader1.reopen(); + IndexReader parallelReader2 = parallelReader1.reopenIfChanged(); + assertNotNull(parallelReader2); + assertNull(parallelReader2.reopenIfChanged()); // 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 = reader1.reopenIfChanged(); + assertNotNull(reader2); assertRefCountEquals(2 + mode, reader1); if (mode == 1) { @@ -557,7 +564,8 @@ } modifyIndex(4, dir1); - IndexReader reader3 = reader2.reopen(); + IndexReader reader3 = reader2.reopenIfChanged(); + assertNotNull(reader3); assertRefCountEquals(2 + mode, reader1); assertRefCountEquals(1, reader2); @@ -611,25 +619,29 @@ modifier.deleteDocument(0); modifier.close(); - IndexReader reader2 = reader1.reopen(); + IndexReader reader2 = reader1.reopenIfChanged(); + 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 = reader2.reopenIfChanged(); + assertNotNull(reader3); SegmentReader segmentReader3 = getOnlySegmentReader(reader3); modifier = IndexReader.open(dir1, false); modifier.deleteDocument(2); modifier.close(); - IndexReader reader4 = reader3.reopen(); + IndexReader reader4 = reader3.reopenIfChanged(); + assertNotNull(reader4); modifier = IndexReader.open(dir1, false); modifier.deleteDocument(3); modifier.close(); - IndexReader reader5 = reader3.reopen(); + IndexReader reader5 = reader3.reopenIfChanged(); + assertNotNull(reader5); // Now reader2-reader5 references reader1. reader1 and reader2 // share the same norms. reader3, reader4, reader5 also share norms. @@ -740,11 +752,11 @@ for (int i = 0; i < n; i++) { if (i % 2 == 0) { - IndexReader refreshed = reader.reopen(); - if (refreshed != reader) { + IndexReader refreshed = reader.reopenIfChanged(); + if (refreshed != null) { readersToClose.add(reader); + reader = refreshed; } - reader = refreshed; } final IndexReader r = reader; @@ -768,7 +780,10 @@ break; } else { // not synchronized - IndexReader refreshed = r.reopen(); + IndexReader refreshed = r.reopenIfChanged(); + if (refreshed == null) { + refreshed = r; + } IndexSearcher searcher = newSearcher(refreshed); ScoreDoc[] hits = searcher.search( @@ -909,7 +924,10 @@ IndexReader refreshed = null; try { - refreshed = reader.reopen(); + refreshed = reader.reopenIfChanged(); + if (refreshed == null) { + refreshed = reader; + } } finally { if (refreshed == null && r != null) { // Hit exception -- close opened reader @@ -1096,7 +1114,8 @@ r2.deleteDocument(0); r2.close(); - IndexReader r3 = r1.reopen(); + IndexReader r3 = r1.reopenIfChanged(); + assertNotNull(r3); assertTrue(r1 != r3); r1.close(); try { @@ -1120,7 +1139,9 @@ modifyIndex(5, dir); // Add another doc (3 segments) - IndexReader r2 = r1.reopen(); // MSR + IndexReader r2 = r1.reopenIfChanged(); // MSR + assertNotNull(r2); + assertNull(r2.reopenIfChanged()); assertTrue(r1 != r2); SegmentReader sr1 = (SegmentReader) r1.getSequentialSubReaders()[0]; // Get SRs for the first segment from original @@ -1153,7 +1174,8 @@ // Add doc: modifyIndex(5, dir); - IndexReader r2 = r1.reopen(); + IndexReader r2 = r1.reopenIfChanged(); + assertNotNull(r2); assertTrue(r1 != r2); IndexReader[] rs2 = r2.getSequentialSubReaders(); @@ -1209,7 +1231,8 @@ Collection commits = IndexReader.listCommits(dir); for (final IndexCommit commit : commits) { - IndexReader r2 = r.reopen(commit); + IndexReader r2 = r.reopenIfChanged(commit); + assertNotNull(r2); assertTrue(r2 != r); // Reader should be readOnly @@ -1264,7 +1287,8 @@ assertEquals(17, ints[0]); // Reopen to readonly w/ no chnages - IndexReader r3 = r.reopen(true); + IndexReader r3 = r.reopenIfChanged(true); + assertNotNull(r3); assertTrue(((DirectoryReader) r3).readOnly); r3.close(); @@ -1273,7 +1297,8 @@ writer.commit(); // Reopen reader1 --> reader2 - IndexReader r2 = r.reopen(true); + IndexReader r2 = r.reopenIfChanged(true); + assertNotNull(r2); r.close(); assertTrue(((DirectoryReader) r2).readOnly); IndexReader[] subs = r2.getSequentialSubReaders(); Index: lucene/src/java/org/apache/lucene/search/IndexSearcher.java =================================================================== --- lucene/src/java/org/apache/lucene/search/IndexSearcher.java (revision 1176096) +++ lucene/src/java/org/apache/lucene/search/IndexSearcher.java (working copy) @@ -55,7 +55,7 @@ * 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#reopenIfChanged} 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)}). Index: lucene/src/java/org/apache/lucene/index/ParallelReader.java =================================================================== --- lucene/src/java/org/apache/lucene/index/ParallelReader.java (revision 1176096) +++ lucene/src/java/org/apache/lucene/index/ParallelReader.java (working copy) @@ -229,12 +229,12 @@ *
* 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#reopenIfChanged()}). *

* If subreaders are shared, then the reference count of those * readers is increased to ensure that the subreaders remain open @@ -244,7 +244,7 @@ * @throws IOException if there is a low-level IO error */ @Override - public synchronized IndexReader reopen() throws CorruptIndexException, IOException { + public synchronized IndexReader reopenIfChanged() throws CorruptIndexException, IOException { // doReopen calls ensureOpen return doReopen(false); } @@ -262,15 +262,16 @@ IndexReader newReader = null; if (doClone) { newReader = (IndexReader) oldReader.clone(); + reopened = true; } else { - newReader = oldReader.reopen(); + newReader = oldReader.reopenIfChanged(); + 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 @@ return pr; } else { // No subreader was refreshed - return this; + return null; } } Index: lucene/src/java/org/apache/lucene/index/SegmentReader.java =================================================================== --- lucene/src/java/org/apache/lucene/index/SegmentReader.java (revision 1176096) +++ lucene/src/java/org/apache/lucene/index/SegmentReader.java (working copy) @@ -204,13 +204,13 @@ } @Override - public synchronized IndexReader reopen() + public synchronized IndexReader reopenIfChanged() throws CorruptIndexException, IOException { return reopenSegment(si, false, readOnly); } @Override - public synchronized IndexReader reopen(boolean openReadOnly) + public synchronized IndexReader reopenIfChanged(boolean openReadOnly) throws CorruptIndexException, IOException { return reopenSegment(si, false, openReadOnly); } @@ -233,7 +233,7 @@ // 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 Index: lucene/src/java/org/apache/lucene/index/MultiReader.java =================================================================== --- lucene/src/java/org/apache/lucene/index/MultiReader.java (revision 1176096) +++ lucene/src/java/org/apache/lucene/index/MultiReader.java (working copy) @@ -99,14 +99,14 @@ /** * Tries to reopen the subreaders. *
- * If one or more subreaders could be re-opened (i. e. subReader.reopen() + * If one or more subreaders could be re-opened (i. e. subReader.reopenIfChanged() * returned a new instance != subReader), 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#reopenIfChanged()}). *

* If subreaders are shared, then the reference count of those * readers is increased to ensure that the subreaders remain open @@ -116,7 +116,7 @@ * @throws IOException if there is a low-level IO error */ @Override - public synchronized IndexReader reopen() throws CorruptIndexException, IOException { + public synchronized IndexReader reopenIfChanged() throws CorruptIndexException, IOException { return doReopen(false); } @@ -160,14 +160,17 @@ 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; + } else { + final IndexReader newSubReader = subReaders[i].reopenIfChanged(); + if (newSubReader != null) { + newSubReaders[i] = newSubReader; + reopened = true; + } else { + newSubReaders[i] = subReaders[i]; + } } } success = true; @@ -197,7 +200,7 @@ mr.decrefOnClose = newDecrefOnClose; return mr; } else { - return this; + return null; } } Index: lucene/src/java/org/apache/lucene/index/DirectoryReader.java =================================================================== --- lucene/src/java/org/apache/lucene/index/DirectoryReader.java (revision 1176096) +++ lucene/src/java/org/apache/lucene/index/DirectoryReader.java (working copy) @@ -198,8 +198,8 @@ 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 #reopenIfChanged()} */ + DirectoryReader(Directory directory, SegmentInfos infos, SegmentReader[] oldReaders, boolean readOnly, boolean doClone, int termInfosIndexDivisor, CodecProvider codecs, Collection readerFinishedListeners) throws IOException { this.directory = directory; @@ -255,19 +255,22 @@ // 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; + readerShared[i] = false; + newReaders[i] = newReader; } else { newReader = newReaders[i].reopenSegment(infos.info(i), doClone, readOnly); - assert newReader.readerFinishedListeners == readerFinishedListeners; + 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; + } } - 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; - } success = true; } finally { if (!success) { @@ -388,21 +391,23 @@ } @Override - public final IndexReader reopen() throws CorruptIndexException, IOException { + public final IndexReader reopenIfChanged() throws CorruptIndexException, IOException { // Preserve current readOnly return doReopen(readOnly, null); } @Override - public final IndexReader reopen(boolean openReadOnly) throws CorruptIndexException, IOException { + public final IndexReader reopenIfChanged(boolean openReadOnly) throws CorruptIndexException, IOException { return doReopen(openReadOnly, null); } @Override - public final IndexReader reopen(final IndexCommit commit) throws CorruptIndexException, IOException { + public final IndexReader reopenIfChanged(final IndexCommit commit) throws CorruptIndexException, IOException { return doReopen(true, commit); } + // NOTE: always returns a non-null result (ie new reader) + // but that could change someday private final IndexReader doReopenFromWriter(boolean openReadOnly, IndexCommit commit) throws CorruptIndexException, IOException { assert readOnly; @@ -451,25 +456,26 @@ 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; } } } @@ -486,7 +492,7 @@ 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); + reader = new DirectoryReader(directory, infos, subReaders, openReadOnly, doClone, termInfosIndexDivisor, codecs, readerFinishedListeners); return reader; } Index: lucene/src/java/org/apache/lucene/index/IndexReader.java =================================================================== --- lucene/src/java/org/apache/lucene/index/IndexReader.java (revision 1176096) +++ lucene/src/java/org/apache/lucene/index/IndexReader.java (working copy) @@ -309,7 +309,7 @@ * @throws CorruptIndexException * @throws IOException if there is a low-level IO error * - * @see #reopen(IndexWriter,boolean) + * @see #reopenIfChanged(IndexWriter,boolean) * * @lucene.experimental */ @@ -502,60 +502,48 @@ } /** - * Refreshes an IndexReader if the index has changed since this instance - * was (re)opened. + * Refreshes an IndexReader if the index has + * changed since this instance was opened, returning + * null if there are no changes or a new IndexReader + * instance if there were. The current + * IndexReader (this) is never closed; you + * must eventually close it once all threads are done + * using it (see SearcherManager in + * contrib/misc to help manage this). + * *

- * 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.
+ * This method is typically far less costly than opening a + * fully new IndexReader as it shares + * resources (for example sub-readers) with the previous + * IndexReader, when possible. + * *

* 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 + * mutable state uses "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;
-   * ...
-   * 
* - * 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. - * *

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. + * 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 { + public synchronized IndexReader reopenIfChanged() 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 + /** Just like {@link #reopenIfChanged()}, 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 { + public synchronized IndexReader reopenIfChanged(boolean openReadOnly) throws CorruptIndexException, IOException { throw new UnsupportedOperationException("This reader does not support reopen()."); } @@ -565,7 +553,7 @@ * 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 { + public synchronized IndexReader reopenIfChanged(final IndexCommit commit) throws CorruptIndexException, IOException { throw new UnsupportedOperationException("This reader does not support reopen(IndexCommit)."); } @@ -595,7 +583,7 @@ * learn, improve and iterate.

* *

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

* @@ -637,7 +625,7 @@ * * @lucene.experimental */ - public IndexReader reopen(IndexWriter writer, boolean applyAllDeletes) throws CorruptIndexException, IOException { + public IndexReader reopenIfChanged(IndexWriter writer, boolean applyAllDeletes) throws CorruptIndexException, IOException { return writer.getReader(applyAllDeletes); } @@ -652,7 +640,7 @@ * 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 #reopenIfChanged()}, 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. @@ -773,7 +761,7 @@ * 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 #reopenIfChanged} 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 @@ -781,7 +769,7 @@ * *

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 #reopenIfChanged} * 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 @@ -814,14 +802,14 @@ * 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 #reopenIfChanged} 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 #reopenIfChanged} * 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. @@ -829,7 +817,7 @@ * 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 #reopenIfChanged} to get a new reader that sees the * changes.

* * @throws CorruptIndexException if the index is corrupt