lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Uwe Schindler" <...@thetaphi.de>
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:31:18 GMT
When you merge from 3.x to trunk or the other way round it will
automatically merge the entries. Most of us always merge also the changes.
E.g. if I do a change that's released for the first time in 3.x, I do the
change first on trunk, but already put it in trunk into the changes. Then I
merge to 3.x and all is fine, no conflicts, always works.

 

I am not sure why you always fix bugs on 3.x and then merge to trunk, but
that should not matter for that.

 

Uwe

 

-----

Uwe Schindler

H.-H.-Meier-Allee 63, D-28213 Bremen

 <http://www.thetaphi.de/> http://www.thetaphi.de

eMail: uwe@thetaphi.de

 

From: Shai Erera [mailto:serera@gmail.com] 
Sent: Tuesday, February 14, 2012 6:19 PM
To: dev@lucene.apache.org; simon.willnauer@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/

 

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
<http://svn.apache.org/viewvc?rev=1244000&view=rev> &view=rev
> Log:
> LUCENE-3761: generalize SearcherManager (port to trunk)
>
> Added:
>
lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/ReferenceMana
ger.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.ja
va
>
lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/SearcherManag
er.java
>
lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/search/TestSearcherM
anager.java
>
lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/search/Sha
rdSearchingTestBase.java
>
> Modified:
lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/NRTManager.ja
va
> URL:
http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/java/org/apach
e/lucene/search/NRTManager.java?rev=1244000
<http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/java/org/apac
he/lucene/search/NRTManager.java?rev=1244000&r1=1243999&r2=1244000&view=diff
> &r1=1243999&r2=1244000&view=diff
>
============================================================================
==
> ---
lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/NRTManager.ja
va (original)
> +++
lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/NRTManager.ja
va 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/SearcherManag
er.java
> URL:
http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/java/org/apach
e/lucene/search/SearcherManager.java?rev=1244000
<http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/java/org/apac
he/lucene/search/SearcherManager.java?rev=1244000&r1=1243999&r2=1244000&view
=diff> &r1=1243999&r2=1244000&view=diff
>
============================================================================
==
> ---
lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/SearcherManag
er.java (original)
> +++
lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/SearcherManag
er.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/TestSearcherM
anager.java
> URL:
http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/test/org/apach
e/lucene/search/TestSearcherManager.java?rev=1244000
<http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/test/org/apac
he/lucene/search/TestSearcherManager.java?rev=1244000&r1=1243999&r2=1244000&
view=diff> &r1=1243999&r2=1244000&view=diff
>
============================================================================
==
> ---
lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/search/TestSearcherM
anager.java (original)
> +++
lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/search/TestSearcherM
anager.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/Sha
rdSearchingTestBase.java
> URL:
http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/test-framework/src/java
/org/apache/lucene/search/ShardSearchingTestBase.java?rev=1244000
<http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/test-framework/src/jav
a/org/apache/lucene/search/ShardSearchingTestBase.java?rev=1244000&r1=124399
9&r2=1244000&view=diff> &r1=1243999&r2=1244000&view=diff
>
============================================================================
==
> ---
lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/search/Sha
rdSearchingTestBase.java (original)
> +++
lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/search/Sha
rdSearchingTestBase.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