lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Simon Willnauer <simon.willna...@googlemail.com>
Subject Re: svn commit: r1244000 - in /lucene/dev/trunk: ./ lucene/ lucene/core/src/java/org/apache/lucene/search/ lucene/core/src/test/org/apache/lucene/search/ lucene/test-framework/src/java/org/apache/lucene/search/
Date Tue, 14 Feb 2012 16:18:11 GMT
shai, seems like you missed the CHANGES.TXT from branch_3x?

simon

On Tue, Feb 14, 2012 at 4:36 PM,  <shaie@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-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


Mime
View raw message