shai, seems like you missed the CHANGES.TXT from branch_3x? simon
On Tue, Feb 14, 2012 at 4:36 PM, <sh...@apache.org> wrote: > Author: shaie > Date: Tue Feb 14 15:36:14 2012 > New Revision: 1244000 > > URL: http://svn.apache.org/viewvc?rev=1244000&view=rev > Log: > LUCENE-3761: generalize SearcherManager (port to trunk) > > Added: > > lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/ReferenceManager.java > - copied unchanged from r1243906, > lucene/dev/branches/branch_3x/lucene/core/src/java/org/apache/lucene/search/ReferenceManager.java > Modified: > lucene/dev/trunk/ (props changed) > lucene/dev/trunk/lucene/ (props changed) > > lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/NRTManager.java > > lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/SearcherManager.java > > lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/search/TestSearcherManager.java > > lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/search/ShardSearchingTestBase.java > > Modified: > lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/NRTManager.java > URL: > http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/NRTManager.java?rev=1244000&r1=1243999&r2=1244000&view=diff > ============================================================================== > --- > lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/NRTManager.java > (original) > +++ > lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/NRTManager.java > Tue Feb 14 15:36:14 2012 > @@ -343,7 +343,7 @@ public class NRTManager implements Close > } > boolean setSearchGen; > if (!mgr.isSearcherCurrent()) { > - setSearchGen = mgr.maybeReopen(); > + setSearchGen = mgr.maybeRefresh(); > } else { > setSearchGen = true; > } > > Modified: > lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/SearcherManager.java > URL: > http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/SearcherManager.java?rev=1244000&r1=1243999&r2=1244000&view=diff > ============================================================================== > --- > lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/SearcherManager.java > (original) > +++ > lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/SearcherManager.java > Tue Feb 14 15:36:14 2012 > @@ -17,17 +17,11 @@ package org.apache.lucene.search; > * limitations under the License. > */ > > -import java.io.Closeable; > import java.io.IOException; > -import java.util.concurrent.Semaphore; > > -import org.apache.lucene.index.CorruptIndexException; > import org.apache.lucene.index.IndexReader; > import org.apache.lucene.index.DirectoryReader; > import org.apache.lucene.index.IndexWriter; > -import org.apache.lucene.search.NRTManager; // javadocs > -import org.apache.lucene.search.IndexSearcher; // javadocs > -import org.apache.lucene.store.AlreadyClosedException; > import org.apache.lucene.store.Directory; > > /** > @@ -51,42 +45,40 @@ import org.apache.lucene.store.Directory > * </pre> > * > * <p> > - * In addition you should periodically call {@link #maybeReopen}. While it's > + * In addition you should periodically call {@link #maybeRefresh}. While it's > * possible to call this just before running each query, this is discouraged > * since it penalizes the unlucky queries that do the reopen. It's better to > use > * a separate background thread, that periodically calls maybeReopen. Finally, > * be sure to call {@link #close} once you are done. > * > - * <p> > - * <b>NOTE</b>: if you have an {@link IndexWriter}, it's better to use > - * {@link NRTManager} since that class pulls near-real-time readers from the > - * IndexWriter. > - * > * @see SearcherFactory > * > * @lucene.experimental > */ > +public final class SearcherManager extends ReferenceManager<IndexSearcher> { > > -public final class SearcherManager implements Closeable { > - > - private volatile IndexSearcher currentSearcher; > private final SearcherFactory searcherFactory; > - private final Semaphore reopenLock = new Semaphore(1); > - > + > /** > - * Creates and returns a new SearcherManager from the given {@link > IndexWriter}. > - * @param writer the IndexWriter to open the IndexReader from. > - * @param applyAllDeletes If <code>true</code>, all buffered deletes will > - * be applied (made visible) in the {@link IndexSearcher} / {@link > DirectoryReader}. > - * If <code>false</code>, the deletes may or may not be applied, > but remain buffered > - * (in IndexWriter) so that they will be applied in the future. > - * Applying deletes can be costly, so if your app can tolerate > deleted documents > - * being returned you might gain some performance by passing > <code>false</code>. > - * See {@link DirectoryReader#openIfChanged(DirectoryReader, > IndexWriter, boolean)}. > - * @param searcherFactory An optional {@link SearcherFactory}. Pass > - * <code>null</code> if you don't require the searcher to be warmed > - * before going live or other custom behavior. > - * > + * Creates and returns a new SearcherManager from the given > + * {@link IndexWriter}. > + * > + * @param writer > + * the IndexWriter to open the IndexReader from. > + * @param applyAllDeletes > + * If <code>true</code>, all buffered deletes will be applied > (made > + * visible) in the {@link IndexSearcher} / {@link > DirectoryReader}. > + * If <code>false</code>, the deletes may or may not be applied, > but > + * remain buffered (in IndexWriter) so that they will be applied > in > + * the future. Applying deletes can be costly, so if your app can > + * tolerate deleted documents being returned you might gain some > + * performance by passing <code>false</code>. See > + * {@link DirectoryReader#openIfChanged(DirectoryReader, > IndexWriter, boolean)}. > + * @param searcherFactory > + * An optional {@link SearcherFactory}. Pass <code>null</code> if > you > + * don't require the searcher to be warmed before going live or > other > + * custom behavior. > + * > * @throws IOException > */ > public SearcherManager(IndexWriter writer, boolean applyAllDeletes, > SearcherFactory searcherFactory) throws IOException { > @@ -94,7 +86,29 @@ public final class SearcherManager imple > searcherFactory = new SearcherFactory(); > } > this.searcherFactory = searcherFactory; > - currentSearcher = > searcherFactory.newSearcher(DirectoryReader.open(writer, applyAllDeletes)); > + current = searcherFactory.newSearcher(DirectoryReader.open(writer, > applyAllDeletes)); > + } > + > + @Override > + protected void decRef(IndexSearcher reference) throws IOException { > + reference.getIndexReader().decRef(); > + } > + > + @Override > + protected IndexSearcher refreshIfNeeded(IndexSearcher referenceToRefresh) > throws IOException { > + final IndexReader r = referenceToRefresh.getIndexReader(); > + final IndexReader newReader = (r instanceof DirectoryReader) ? > + DirectoryReader.openIfChanged((DirectoryReader) r) : null; > + if (newReader == null) { > + return null; > + } else { > + return searcherFactory.newSearcher(newReader); > + } > + } > + > + @Override > + protected boolean tryIncRef(IndexSearcher reference) { > + return reference.getIndexReader().tryIncRef(); > } > > /** > @@ -111,75 +125,15 @@ public final class SearcherManager imple > searcherFactory = new SearcherFactory(); > } > this.searcherFactory = searcherFactory; > - currentSearcher = searcherFactory.newSearcher(DirectoryReader.open(dir)); > + current = searcherFactory.newSearcher(DirectoryReader.open(dir)); > } > > /** > - * You must call this, periodically, to perform a reopen. This calls > - * {@link DirectoryReader#openIfChanged(DirectoryReader)} with the > underlying reader, and if that returns a > - * new reader, it's warmed (if you provided a {@link SearcherFactory} and > then > - * swapped into production. > - * > - * <p> > - * <b>Threads</b>: it's fine for more than one thread to call this at once. > - * Only the first thread will attempt the reopen; subsequent threads will > see > - * that another thread is already handling reopen and will return > immediately. > - * Note that this means if another thread is already reopening then > subsequent > - * threads will return right away without waiting for the reader reopen to > - * complete. > - * </p> > - * > - * <p> > - * This method returns true if a new reader was in fact opened or > - * if the current searcher has no pending changes. > - * </p> > - */ > - public boolean maybeReopen() throws IOException { > - ensureOpen(); > - // Ensure only 1 thread does reopen at once; other > - // threads just return immediately: > - if (reopenLock.tryAcquire()) { > - try { > - // IR.openIfChanged preserves NRT and applyDeletes > - // in the newly returned reader: > - final IndexReader newReader; > - final IndexSearcher searcherToReopen = acquire(); > - try { > - final IndexReader r = searcherToReopen.getIndexReader(); > - newReader = (r instanceof DirectoryReader) ? > - DirectoryReader.openIfChanged((DirectoryReader) r) : > - null; > - } finally { > - release(searcherToReopen); > - } > - if (newReader != null) { > - final IndexSearcher newSearcher = > searcherFactory.newSearcher(newReader); > - boolean success = false; > - try { > - swapSearcher(newSearcher); > - success = true; > - } finally { > - if (!success) { > - release(newSearcher); > - } > - } > - } > - return true; > - } finally { > - reopenLock.release(); > - } > - } else { > - return false; > - } > - } > - > - /** > * Returns <code>true</code> if no changes have occured since this searcher > * ie. reader was opened, otherwise <code>false</code>. > * @see DirectoryReader#isCurrent() > */ > - public boolean isSearcherCurrent() throws CorruptIndexException, > - IOException { > + public boolean isSearcherCurrent() throws IOException { > final IndexSearcher searcher = acquire(); > try { > final IndexReader r = searcher.getIndexReader(); > @@ -190,56 +144,5 @@ public final class SearcherManager imple > release(searcher); > } > } > - > - /** > - * Release the searcher previously obtained with {@link #acquire}. > - * > - * <p> > - * <b>NOTE</b>: it's safe to call this after {@link #close}. > - */ > - public void release(IndexSearcher searcher) throws IOException { > - assert searcher != null; > - searcher.getIndexReader().decRef(); > - } > - > - /** > - * Close this SearcherManager to future searching. Any searches still in > - * process in other threads won't be affected, and they should still call > - * {@link #release} after they are done. > - */ > - public synchronized void close() throws IOException { > - if (currentSearcher != null) { > - // make sure we can call this more than once > - // closeable javadoc says: > - // if this is already closed then invoking this method has no effect. > - swapSearcher(null); > - } > - } > - > - /** > - * Obtain the current IndexSearcher. You must match every call to acquire > with > - * one call to {@link #release}; it's best to do so in a finally clause. > - */ > - public IndexSearcher acquire() { > - IndexSearcher searcher; > - do { > - if ((searcher = currentSearcher) == null) { > - throw new AlreadyClosedException("this SearcherManager is closed"); > - } > - } while (!searcher.getIndexReader().tryIncRef()); > - return searcher; > - } > - > - private void ensureOpen() { > - if (currentSearcher == null) { > - throw new AlreadyClosedException("this SearcherManager is closed"); > - } > - } > - > - private synchronized void swapSearcher(IndexSearcher newSearcher) throws > IOException { > - ensureOpen(); > - final IndexSearcher oldSearcher = currentSearcher; > - currentSearcher = newSearcher; > - release(oldSearcher); > - } > + > } > > Modified: > lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/search/TestSearcherManager.java > URL: > http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/search/TestSearcherManager.java?rev=1244000&r1=1243999&r2=1244000&view=diff > ============================================================================== > --- > lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/search/TestSearcherManager.java > (original) > +++ > lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/search/TestSearcherManager.java > Tue Feb 14 15:36:14 2012 > @@ -31,9 +31,9 @@ import org.apache.lucene.document.Docume > import org.apache.lucene.index.ConcurrentMergeScheduler; > import org.apache.lucene.index.IndexReader; > import org.apache.lucene.index.IndexWriter; > +import org.apache.lucene.index.IndexWriterConfig; > import org.apache.lucene.index.Term; > import org.apache.lucene.index.ThreadedIndexingAndSearchingTestCase; > -import org.apache.lucene.search.SearcherFactory; > import org.apache.lucene.store.AlreadyClosedException; > import org.apache.lucene.store.Directory; > import org.apache.lucene.util.LuceneTestCase.UseNoMemoryExpensiveCodec; > @@ -57,7 +57,7 @@ public class TestSearcherManager extends > if (!isNRT) { > writer.commit(); > } > - assertTrue(mgr.maybeReopen() || mgr.isSearcherCurrent()); > + assertTrue(mgr.maybeRefresh() || mgr.isSearcherCurrent()); > return mgr.acquire(); > } > > @@ -105,7 +105,7 @@ public class TestSearcherManager extends > Thread.sleep(_TestUtil.nextInt(random, 1, 100)); > writer.commit(); > Thread.sleep(_TestUtil.nextInt(random, 1, 5)); > - if (mgr.maybeReopen()) { > + if (mgr.maybeRefresh()) { > lifetimeMGR.prune(pruner); > } > } > @@ -134,7 +134,7 @@ public class TestSearcherManager extends > // synchronous to your search threads, but still we > // test as apps will presumably do this for > // simplicity: > - if (mgr.maybeReopen()) { > + if (mgr.maybeRefresh()) { > lifetimeMGR.prune(pruner); > } > } > @@ -237,7 +237,7 @@ public class TestSearcherManager extends > if (VERBOSE) { > System.out.println("NOW call maybeReopen"); > } > - searcherManager.maybeReopen(); > + searcherManager.maybeRefresh(); > success.set(true); > } catch (AlreadyClosedException e) { > // expected > @@ -280,4 +280,41 @@ public class TestSearcherManager extends > es.awaitTermination(1, TimeUnit.SECONDS); > } > } > + > + public void testCloseTwice() throws Exception { > + // test that we can close SM twice (per Closeable's contract). > + Directory dir = newDirectory(); > + new IndexWriter(dir, new IndexWriterConfig(TEST_VERSION_CURRENT, > null)).close(); > + SearcherManager sm = new SearcherManager(dir, null); > + sm.close(); > + sm.close(); > + dir.close(); > + } > + > + public void testEnsureOpen() throws Exception { > + Directory dir = newDirectory(); > + new IndexWriter(dir, new IndexWriterConfig(TEST_VERSION_CURRENT, > null)).close(); > + SearcherManager sm = new SearcherManager(dir, null); > + IndexSearcher s = sm.acquire(); > + sm.close(); > + > + // this should succeed; > + sm.release(s); > + > + try { > + // this should fail > + sm.acquire(); > + } catch (AlreadyClosedException e) { > + // ok > + } > + > + try { > + // this should fail > + sm.maybeRefresh(); > + } catch (AlreadyClosedException e) { > + // ok > + } > + dir.close(); > + } > + > } > > Modified: > lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/search/ShardSearchingTestBase.java > URL: > http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/search/ShardSearchingTestBase.java?rev=1244000&r1=1243999&r2=1244000&view=diff > ============================================================================== > --- > lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/search/ShardSearchingTestBase.java > (original) > +++ > lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/search/ShardSearchingTestBase.java > Tue Feb 14 15:36:14 2012 > @@ -485,7 +485,7 @@ public abstract class ShardSearchingTest > final IndexSearcher before = mgr.acquire(); > mgr.release(before); > > - mgr.maybeReopen(); > + mgr.maybeRefresh(); > final IndexSearcher after = mgr.acquire(); > try { > if (after != before) { > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
