Index: lucene/core/src/test/org/apache/lucene/search/TestNRTManager.java =================================================================== --- lucene/core/src/test/org/apache/lucene/search/TestNRTManager.java (revision 1244909) +++ lucene/core/src/test/org/apache/lucene/search/TestNRTManager.java (working copy) @@ -57,7 +57,7 @@ System.out.println("TEST: finalSearcher maxGen=" + maxGen); } nrtDeletes.waitForGeneration(maxGen); - return nrtDeletes.getSearcherManager().acquire(); + return nrtDeletes.acquire(); } @Override @@ -84,14 +84,14 @@ System.out.println(Thread.currentThread().getName() + ": nrt: verify " + id); } nrtDeletes.waitForGeneration(gen); - final IndexSearcher s = nrtDeletes.getSearcherManager().acquire(); + final IndexSearcher s = nrtDeletes.acquire(); if (VERBOSE) { System.out.println(Thread.currentThread().getName() + ": nrt: got searcher=" + s); } try { assertEquals(docs.size(), s.search(new TermQuery(id), 10).totalHits); } finally { - nrtDeletes.getSearcherManager().release(s); + nrtDeletes.release(s); } } @@ -107,14 +107,14 @@ System.out.println(Thread.currentThread().getName() + ": nrt: verify " + id); } nrtNoDeletes.waitForGeneration(gen); - final IndexSearcher s = nrtNoDeletes.getSearcherManager().acquire(); + final IndexSearcher s = nrtNoDeletes.acquire(); if (VERBOSE) { System.out.println(Thread.currentThread().getName() + ": nrt: got searcher=" + s); } try { assertEquals(docs.size(), s.search(new TermQuery(id), 10).totalHits); } finally { - nrtNoDeletes.getSearcherManager().release(s); + nrtNoDeletes.release(s); } } lastGens.set(gen); @@ -130,14 +130,14 @@ System.out.println(Thread.currentThread().getName() + ": nrt: verify " + id); } nrtNoDeletes.waitForGeneration(gen); - final IndexSearcher s = nrtNoDeletes.getSearcherManager().acquire(); + final IndexSearcher s = nrtNoDeletes.acquire(); if (VERBOSE) { System.out.println(Thread.currentThread().getName() + ": nrt: got searcher=" + s); } try { assertEquals(1, s.search(new TermQuery(id), 10).totalHits); } finally { - nrtNoDeletes.getSearcherManager().release(s); + nrtNoDeletes.release(s); } } lastGens.set(gen); @@ -152,14 +152,14 @@ System.out.println(Thread.currentThread().getName() + ": nrt: verify " + id); } nrtDeletes.waitForGeneration(gen); - final IndexSearcher s = nrtDeletes.getSearcherManager().acquire(); + final IndexSearcher s = nrtDeletes.acquire(); if (VERBOSE) { System.out.println(Thread.currentThread().getName() + ": nrt: got searcher=" + s); } try { assertEquals(1, s.search(new TermQuery(id), 10).totalHits); } finally { - nrtDeletes.getSearcherManager().release(s); + nrtDeletes.release(s); } } lastGens.set(gen); @@ -174,14 +174,14 @@ System.out.println(Thread.currentThread().getName() + ": nrt: verify del " + id); } nrtDeletes.waitForGeneration(gen); - final IndexSearcher s = nrtDeletes.getSearcherManager().acquire(); + final IndexSearcher s = nrtDeletes.acquire(); if (VERBOSE) { System.out.println(Thread.currentThread().getName() + ": nrt: got searcher=" + s); } try { assertEquals(0, s.search(new TermQuery(id), 10).totalHits); } finally { - nrtDeletes.getSearcherManager().release(s); + nrtDeletes.release(s); } } lastGens.set(gen); @@ -265,7 +265,7 @@ nrt = nrtNoDeletes; } - return nrt.getSearcherManager().acquire(); + return nrt.acquire(); } @Override @@ -274,7 +274,7 @@ // against the same NRT mgr you acquired from... but // both impls just decRef the underlying reader so we // can get away w/ cheating: - nrtNoDeletes.getSearcherManager().release(s); + nrtNoDeletes.release(s); } @Override @@ -304,15 +304,15 @@ Document doc = new Document(); doc.add(newField("test","test", TextField.TYPE_STORED)); long gen = writer.addDocument(doc); - manager.maybeReopen(); + manager.maybeRefresh(); assertFalse(gen < manager.getCurrentSearchingGen()); Thread t = new Thread() { public void run() { try { signal.await(); - manager.maybeReopen(); + manager.maybeRefresh(); writer.deleteDocuments(new TermQuery(new Term("foo", "barista"))); - manager.maybeReopen(); // kick off another reopen so we inc. the internal gen + manager.maybeRefresh(); // kick off another reopen so we inc. the internal gen } catch (Exception e) { e.printStackTrace(); } finally { @@ -324,13 +324,13 @@ _writer.waitAfterUpdate = true; // wait in addDocument to let some reopens go through final long lastGen = writer.updateDocument(new Term("foo", "bar"), doc); // once this returns the doc is already reflected in the last reopen - assertFalse(manager.getSearcherManager().isSearcherCurrent()); // false since there is a delete in the queue + assertFalse(manager.isSearcherCurrent()); // false since there is a delete in the queue - IndexSearcher acquire = manager.getSearcherManager().acquire(); + IndexSearcher acquire = manager.acquire(); try { assertEquals(2, acquire.getIndexReader().numDocs()); } finally { - manager.getSearcherManager().release(acquire); + manager.release(acquire); } NRTManagerReopenThread thread = new NRTManagerReopenThread(manager, 0.01, 0.01); thread.start(); // start reopening @@ -346,7 +346,7 @@ } }; waiter.start(); - manager.maybeReopen(); + manager.maybeRefresh(); waiter.join(1000); if (!finished.get()) { waiter.interrupt(); Index: lucene/core/src/java/org/apache/lucene/search/ReferenceManager.java =================================================================== --- lucene/core/src/java/org/apache/lucene/search/ReferenceManager.java (revision 1244909) +++ lucene/core/src/java/org/apache/lucene/search/ReferenceManager.java (working copy) @@ -56,7 +56,7 @@ current = newReference; release(oldReference); } - + /** Decrement reference counting on the given reference. */ protected abstract void decRef(G reference) throws IOException; @@ -94,7 +94,7 @@ * affected, and they should still be {@link #release released} when they are * not needed anymore. */ - public final synchronized void close() throws IOException { + public synchronized void close() throws IOException { if (current != null) { // make sure we can call this more than once // closeable javadoc says: @@ -116,13 +116,16 @@ * refresh to complete. * *

- * This method returns true if the reference was in fact refreshed, or if the - * current reference has no pending changes. + * If this method returns true it means the calling thread either refreshed + * or that there were no changes to refresh. If it returns false it means another + * thread is currently refreshing. */ public final boolean maybeRefresh() throws IOException { ensureOpen(); + // Ensure only 1 thread does reopen at once; other threads just return immediately: - if (reopenLock.tryAcquire()) { + final boolean doTryRefresh = reopenLock.tryAcquire(); + if (doTryRefresh) { try { final G reference = acquire(); try { @@ -142,15 +145,20 @@ } finally { release(reference); } - return true; + afterRefresh(); } finally { reopenLock.release(); } - } else { - return false; } + + return doTryRefresh; } + /** Called after swapReference has installed a new + * instance. */ + protected void afterRefresh() { + } + /** * Release the refernce previously obtained via {@link #acquire()}. *

@@ -160,5 +168,4 @@ assert reference != null; decRef(reference); } - } Index: lucene/core/src/java/org/apache/lucene/search/NRTManagerReopenThread.java =================================================================== --- lucene/core/src/java/org/apache/lucene/search/NRTManagerReopenThread.java (revision 1244909) +++ lucene/core/src/java/org/apache/lucene/search/NRTManagerReopenThread.java (working copy) @@ -181,7 +181,7 @@ lastReopenStartNS = System.nanoTime(); try { //final long t0 = System.nanoTime(); - manager.maybeReopen(); + manager.maybeRefresh(); //System.out.println("reopen took " + ((System.nanoTime()-t0)/1000000.0) + " msec"); } catch (IOException ioe) { //System.out.println(Thread.currentThread().getName() + ": IOE"); Index: lucene/core/src/java/org/apache/lucene/search/SearcherManager.java =================================================================== --- lucene/core/src/java/org/apache/lucene/search/SearcherManager.java (revision 1244909) +++ lucene/core/src/java/org/apache/lucene/search/SearcherManager.java (working copy) @@ -97,8 +97,8 @@ @Override protected IndexSearcher refreshIfNeeded(IndexSearcher referenceToRefresh) throws IOException { final IndexReader r = referenceToRefresh.getIndexReader(); - final IndexReader newReader = (r instanceof DirectoryReader) ? - DirectoryReader.openIfChanged((DirectoryReader) r) : null; + assert r instanceof DirectoryReader; + final IndexReader newReader = DirectoryReader.openIfChanged((DirectoryReader) r); if (newReader == null) { return null; } else { @@ -137,12 +137,10 @@ final IndexSearcher searcher = acquire(); try { final IndexReader r = searcher.getIndexReader(); - return r instanceof DirectoryReader ? - ((DirectoryReader ) r).isCurrent() : - true; + assert r instanceof DirectoryReader; + return ((DirectoryReader) r).isCurrent(); } finally { release(searcher); } } - } Index: lucene/core/src/java/org/apache/lucene/search/NRTManager.java =================================================================== --- lucene/core/src/java/org/apache/lucene/search/NRTManager.java (revision 1244909) +++ lucene/core/src/java/org/apache/lucene/search/NRTManager.java (working copy) @@ -17,7 +17,6 @@ * limitations under the License. */ -import java.io.Closeable; import java.io.IOException; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; @@ -28,6 +27,7 @@ import org.apache.lucene.analysis.Analyzer; import org.apache.lucene.index.CorruptIndexException; +import org.apache.lucene.index.DirectoryReader; import org.apache.lucene.index.IndexReader; // javadocs import org.apache.lucene.index.IndexWriter; import org.apache.lucene.index.IndexableField; @@ -71,14 +71,14 @@ * @lucene.experimental */ -public class NRTManager implements Closeable { +public class NRTManager extends ReferenceManager { private static final long MAX_SEARCHER_GEN = Long.MAX_VALUE; private final TrackingIndexWriter writer; private final List waitingListeners = new CopyOnWriteArrayList(); - private final ReentrantLock reopenLock = new ReentrantLock(); - private final Condition newGeneration = reopenLock.newCondition(); + private final ReentrantLock genLock = new ReentrantLock();; + private final Condition newGeneration = genLock.newCondition(); + private final SearcherFactory searcherFactory; - private final SearcherManager mgr; private volatile long searchingGen; /** @@ -101,24 +101,25 @@ * apply deletes. This is useful for cases where certain * uses can tolerate seeing some deleted docs, since * reopen time is faster if deletes need not be applied. */ - public NRTManager(TrackingIndexWriter writer, SearcherFactory searcherFactory, boolean applyDeletes) throws IOException { + public NRTManager(TrackingIndexWriter writer, SearcherFactory searcherFactory, boolean applyAllDeletes) throws IOException { this.writer = writer; - mgr = new SearcherManager(writer.getIndexWriter(), applyDeletes, searcherFactory); + if (searcherFactory == null) { + searcherFactory = new SearcherFactory(); + } + this.searcherFactory = searcherFactory; + current = searcherFactory.newSearcher(DirectoryReader.open(writer.getIndexWriter(), applyAllDeletes)); } - /** - * Returns the {@link SearcherManager} you should use to - * acquire/release searchers. - * - *

NOTE: Never call maybeReopen on the returned - * SearcherManager; only call this NRTManager's {@link - * #maybeReopen}. Otherwise threads waiting for a - * generation may never return. - */ - public SearcherManager getSearcherManager() { - return mgr; + @Override + protected void decRef(IndexSearcher reference) throws IOException { + reference.getIndexReader().decRef(); } + @Override + protected boolean tryIncRef(IndexSearcher reference) { + return reference.getIndexReader().tryIncRef(); + } + /** NRTManager invokes this interface to notify it when a * caller is waiting for a specific generation searcher * to be visible. */ @@ -296,7 +297,7 @@ if (targetGen > curGen) { throw new IllegalArgumentException("targetGen=" + targetGen + " was never returned by this NRTManager instance (current gen=" + curGen + ")"); } - reopenLock.lockInterruptibly(); + genLock.lockInterruptibly(); try { if (targetGen > searchingGen) { for (WaitingListener listener : waitingListeners) { @@ -309,7 +310,7 @@ } } } finally { - reopenLock.unlock(); + genLock.unlock(); } } catch (InterruptedException ie) { throw new ThreadInterruptedException(ie); @@ -318,7 +319,7 @@ private boolean waitOnGenCondition(long time, TimeUnit unit) throws InterruptedException { - assert reopenLock.isHeldByCurrentThread(); + assert genLock.isHeldByCurrentThread(); if (time < 0) { newGeneration.await(); return true; @@ -332,31 +333,45 @@ return searchingGen; } - public void maybeReopen() throws IOException { - if (reopenLock.tryLock()) { - try { - // Mark gen as of when reopen started: - final long newSearcherGen = writer.getAndIncrementGeneration(); - if (searchingGen == MAX_SEARCHER_GEN) { - newGeneration.signalAll(); // wake up threads if we have a new generation - return; - } - boolean setSearchGen; - if (!mgr.isSearcherCurrent()) { - setSearchGen = mgr.maybeRefresh(); - } else { - setSearchGen = true; - } - if (setSearchGen) { - searchingGen = newSearcherGen;// update searcher gen - newGeneration.signalAll(); // wake up threads if we have a new generation - } - } finally { - reopenLock.unlock(); + private long lastRefreshGen; + + @Override + protected IndexSearcher refreshIfNeeded(IndexSearcher referenceToRefresh) throws IOException { + // Record gen as of when reopen started: + lastRefreshGen = writer.getAndIncrementGeneration(); + final IndexReader r = referenceToRefresh.getIndexReader(); + assert r instanceof DirectoryReader; + final IndexSearcher newSearcher; + if (!((DirectoryReader) r).isCurrent()) { + final IndexReader newReader = DirectoryReader.openIfChanged((DirectoryReader) r); + if (newReader != null) { + newSearcher = searcherFactory.newSearcher(newReader); + } else { + newSearcher = null; } + } else { + newSearcher = null; } + + return newSearcher; } + @Override + protected void afterRefresh() { + genLock.lock(); + try { + if (searchingGen != MAX_SEARCHER_GEN) { + // update searchingGen: + assert lastRefreshGen >= searchingGen; + searchingGen = lastRefreshGen; + } + // wake up threads if we have a new generation: + newGeneration.signalAll(); + } finally { + genLock.unlock(); + } + } + /** * Close this NRTManager to future searching. Any searches still in process in * other threads won't be affected, and they should still call @@ -365,18 +380,35 @@ *

* NOTE: caller must separately close the writer. */ - public void close() throws IOException { - reopenLock.lock(); + @Override + public synchronized void close() throws IOException { + // max it out to make sure nobody can wait on another gen try { + super.close(); + } finally { + genLock.lock(); try { - // max it out to make sure nobody can wait on another gen searchingGen = MAX_SEARCHER_GEN; - mgr.close(); - } finally { // make sure we signal even if close throws an exception newGeneration.signalAll(); + } finally { + genLock.unlock(); } + } + } + + /** + * Returns true if no changes have occured since this searcher + * ie. reader was opened, otherwise false. + * @see DirectoryReader#isCurrent() + */ + public boolean isSearcherCurrent() throws IOException { + final IndexSearcher searcher = acquire(); + try { + final IndexReader r = searcher.getIndexReader(); + assert r instanceof DirectoryReader; + return ((DirectoryReader) r).isCurrent(); } finally { - reopenLock.unlock(); + release(searcher); } } } Index: lucene/CHANGES.txt =================================================================== --- lucene/CHANGES.txt (revision 1244909) +++ lucene/CHANGES.txt (working copy) @@ -815,6 +815,10 @@ create two NRTManagers (one always applying deletes and the other never applying deletes). (MJB, Shai Erera, Mike McCandless) +* LUCENE-3776: You now acquire/release the IndexSearcher directly from + NRTManager. (Mike McCandless) + + New Features * LUCENE-3593: Added a FieldValueFilter that accepts all documents that either