lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Shai Erera <ser...@gmail.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 17:18:38 GMT
I wasn't sure if the CHANGES is needed in trunk as well, since the change
was also done in 3x ... so I asked Robert and he said that he doesn't think
that we need an entry in trunk as well, which makes sense to me because
trunk is not yet released ..

What's our policy about this? because I keep asking myself in every commit
whether I need to port CHANGES to trunk too ...

Shai

On Tue, Feb 14, 2012 at 6:18 PM, Simon Willnauer <
simon.willnauer@googlemail.com> wrote:

> 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