Index: lucene/contrib/misc/src/test/org/apache/lucene/search/TestSearcherManager.java =================================================================== --- lucene/contrib/misc/src/test/org/apache/lucene/search/TestSearcherManager.java (revision 1178852) +++ lucene/contrib/misc/src/test/org/apache/lucene/search/TestSearcherManager.java (working copy) @@ -20,6 +20,8 @@ import java.io.IOException; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; +import java.util.List; +import java.util.ArrayList; import java.util.concurrent.atomic.AtomicBoolean; import org.apache.lucene.analysis.MockAnalyzer; @@ -35,7 +37,10 @@ boolean warmCalled; + private SearcherLifetimeManager.Pruner pruner; + public void testSearcherManager() throws Exception { + pruner = new SearcherLifetimeManager.PruneByAge(TEST_NIGHTLY ? 10*random.nextDouble() : random.nextDouble()); runTest("TestSearcherManager"); } @@ -47,6 +52,8 @@ } private SearcherManager mgr; + private SearcherLifetimeManager lifetimeMGR; + private final List pastSearchers = new ArrayList(); @Override protected void doAfterWriter(ExecutorService es) throws Exception { @@ -60,6 +67,8 @@ s.search(new TermQuery(new Term("body", "united")), 10); } }, es); + + lifetimeMGR = new SearcherLifetimeManager(); } @Override @@ -73,7 +82,9 @@ Thread.sleep(_TestUtil.nextInt(random, 1, 100)); writer.commit(); Thread.sleep(_TestUtil.nextInt(random, 1, 5)); - mgr.maybeReopen(); + if (mgr.maybeReopen()) { + lifetimeMGR.prune(pruner); + } } } catch (Throwable t) { System.out.println("TEST: reopen thread hit exc"); @@ -98,15 +109,48 @@ // synchronous to your search threads, but still we // test as apps will presumably do this for // simplicity: - mgr.maybeReopen(); + if (mgr.maybeReopen()) { + lifetimeMGR.prune(pruner); + } } - return mgr.acquire(); + IndexSearcher s = null; + + synchronized(pastSearchers) { + while (pastSearchers.size() != 0 && random.nextDouble() < 0.25) { + // 1/4 of the time pull an old searcher, ie, simulate + // a user doing a follow-on action on a previous + // search (drilling down/up, clicking next/prev page, + // etc.) + final Long token = pastSearchers.get(random.nextInt(pastSearchers.size())); + s = lifetimeMGR.acquire(token); + if (s == null) { + // Searcher was pruned + pastSearchers.remove(token); + } else { + break; + } + } + } + + if (s == null) { + s = mgr.acquire(); + if (s.getIndexReader().numDocs() != 0) { + Long token = lifetimeMGR.record(s); + synchronized(pastSearchers) { + if (!pastSearchers.contains(token)) { + pastSearchers.add(token); + } + } + } + } + + return s; } @Override protected void releaseSearcher(IndexSearcher s) throws Exception { - mgr.release(s); + s.getIndexReader().decRef(); } @Override @@ -116,6 +160,7 @@ System.out.println("TEST: now close SearcherManager"); } mgr.close(); + lifetimeMGR.close(); } public void testIntermediateClose() throws IOException, InterruptedException { Index: lucene/contrib/misc/src/java/org/apache/lucene/search/SearcherLifetimeManager.java =================================================================== --- lucene/contrib/misc/src/java/org/apache/lucene/search/SearcherLifetimeManager.java (revision 0) +++ lucene/contrib/misc/src/java/org/apache/lucene/search/SearcherLifetimeManager.java (revision 0) @@ -0,0 +1,272 @@ +package org.apache.lucene.search; + +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import java.io.Closeable; +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Iterator; +import java.util.List; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.Semaphore; + +import org.apache.lucene.index.NRTManager; // javadocs +import org.apache.lucene.store.AlreadyClosedException; +import org.apache.lucene.util.IOUtils; + +/** + * Keeps track of current plus old IndexSearchers, closing + * the old ones once they have timed out. + * + * Use it like this: + * + * 
+ *   SearcherLifetimeManager mgr = new SearcherLifetimeManager();
+ *
+ * + * Per search-request, if it's a "new" search request, then + * obtain the latest searcher you have (for example, by + * using {@link SearcherManager} or {@link NRTManager}), and + * then record this searcher: + * + * 
+ *   // Record token into user's search results, eg as a
+ *   // hidden HTML form field:
+ *   long token = mgr.record(searcher);
+ *
+ * + * When a follow-up search arrives, for example the user + * clicks next page, drills down/up, etc., take the token + * that you saved from the previous search and: + * + * 
+ *   // If possible, obtain same searcher version as last
+ *   // search:
+ *   IndexSearcher searcher = mgr.acquire(version);
+ *   if (searcher != null) {
+ *     // Searcher is still here
+ *     try {
+ *       // do searching...
+ *     } finally {
+ *       mgr.release(searcher);
+ *       // Do not use searcher after this!
+ *       searcher = null;
+ *     }
+ *   } else {
+ *     // Searcher was pruned -- notify user session timed
+ *     // out, or, pull fresh searcher again
+ *   }
+ *
+ * + * Finally, in a separate thread, ideally the same thread + * that's periodically reopening your searchers, you should + * periodically prune old searchers: + * + * 
+ *   mgr.prune(new PruneByAge(60.0);
+ *
+ */ + +public class SearcherLifetimeManager implements Closeable { + + private static class SearcherTracker implements Comparable { + public final IndexSearcher searcher; + public final double recordTimeSec; + public final long version; + + public SearcherTracker(IndexSearcher searcher) { + this.searcher = searcher; + version = searcher.getIndexReader().getVersion(); + searcher.getIndexReader().incRef(); + // Use nanoTime not currentTimeMillis since it [in + // theory] reduces risk from clock shift + recordTimeSec = System.nanoTime()/1000000000.0; + } + + // Newer searchers are sort before older ones: + @Override + public int compareTo(SearcherTracker other) { + // Be defensive: cannot subtract since it could + // technically overflow long, though, we'd never hit + // that in practice: + if (recordTimeSec < other.recordTimeSec) { + return 1; + } else if (other.recordTimeSec < recordTimeSec) { + return -1; + } else { + // Should not normally happen: + return 0; + } + } + } + + private volatile boolean closed; + + private final ConcurrentHashMap searchers = new ConcurrentHashMap(); + + private void ensureOpen() { + if (closed) { + throw new AlreadyClosedException("this SearcherLifetimeManager instance is closed"); + } + } + + /** Records that you are now using this IndexSearcher. + * Always call this when you've obtained a possibly new + * {@link IndexSearcher}, for example from one of the + * get methods in {@link NRTManager} or {@link + * SearcherManager}. It's fine if you already passed the + * same searcher to this method before. + * + *

This returns the long token that you can later pass + * to {@link #acquire} to retrieve the same IndexSearcher. + * You should record this long token in the search results + * sent to your user, such that if the use performs a + * follow-on action (clicks next page, drills down, etc.) + * the token is returned. */ + public long record(IndexSearcher searcher) throws IOException { + ensureOpen(); + // TODO: we don't have to use IR.getVersion to track; + // could be risky (if it's buggy); we could get better + // bug isolation if we assign our own private ID: + final long version = searcher.getIndexReader().getVersion(); + SearcherTracker tracker = searchers.get(version); + if (tracker == null) { + tracker = new SearcherTracker(searcher); + if (searchers.putIfAbsent(version, tracker) != null) { + // Another thread beat us -- must decRef to undo + // incRef done by SearcherTracker ctor: + searcher.getIndexReader().decRef(); + } + } else { + assert tracker.searcher == searcher; + } + + return version; + } + + /** Retrieve a previously recorded {@link IndexSearcher}, if it + * has not yet been closed + * + *

NOTE: this may return null when the + * requested searcher has already timed out. When this + * happens you should notify your user that their session + * timed out and that they'll have to restart their + * search. + * + *

If this returns a non-null result, you must match + * later call {@link #release} on this searcher, best + * from a finally clause. */ + public IndexSearcher acquire(long version) { + ensureOpen(); + final SearcherTracker tracker = searchers.get(version); + if (tracker != null && + tracker.searcher.getIndexReader().tryIncRef()) { + return tracker.searcher; + } + + return null; + } + + /** NOTE: it's fine to call this after close. */ + public void release(IndexSearcher s) throws IOException { + // nocommit -- maybe make it 'public' that you just decRef? + s.getIndexReader().decRef(); + } + + /** See {@link #prune}. */ + public interface Pruner { + /** Return true if this searcher should be removed. + * @param ageSec how long ago this searcher was + * recorded vs the most recently recorded + * searcher + * @param searcher Searcher + **/ + public boolean doPrune(double ageSec, IndexSearcher searcher); + } + + /** Simple pruner that drops any searcher older by + * more than the specified seconds, than the newest + * searcher. */ + public final static class PruneByAge implements Pruner { + private final double maxAgeSec; + + public PruneByAge(double maxAgeSec) { + this.maxAgeSec = maxAgeSec; + } + + @Override + public boolean doPrune(double ageSec, IndexSearcher searcher) { + return ageSec > maxAgeSec; + } + } + + /** Calls provided {@link Pruner} to prune entries. The + * entries are passed to the Pruner in sorted (newest to + * oldest IndexSearcher) order. + * + *

NOTE: you must peridiocally call this, ideally + * from the same background thread that opens new + * searchers. */ + public synchronized void prune(Pruner pruner) throws IOException { + final List trackers = new ArrayList(searchers.values()); + Collections.sort(trackers); + final double newestSec = trackers.isEmpty() ? 0.0 : trackers.get(0).recordTimeSec; + for (SearcherTracker tracker: trackers) { + assert newestSec >= tracker.recordTimeSec; + if (pruner.doPrune(newestSec - tracker.recordTimeSec, tracker.searcher)) { + searchers.remove(tracker.version); + tracker.searcher.getIndexReader().decRef(); + } + } + } + + /** Close this 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. + * + *

NOTE: you must ensure no other threads are + * using other methods when you call close(), otherwise + * it's posisble not all searcher references have been + * freed. */ + @Override + public synchronized void close() throws IOException { + Throwable firstExc = null; + closed = true; + final Iterator it = searchers.values().iterator(); + while (it.hasNext()) { + final SearcherTracker tracker = it.next(); + try { + tracker.searcher.getIndexReader().decRef(); + } catch (Throwable t) { + IOUtils.addSuppressed(firstExc, t); + if (firstExc == null) { + firstExc = t; + } + } + it.remove(); + } + if (firstExc != null) { + if (firstExc instanceof IOException) throw (IOException) firstExc; + if (firstExc instanceof RuntimeException) throw (RuntimeException) firstExc; + if (firstExc instanceof Error) throw (Error) firstExc; + throw new RuntimeException(firstExc); + } + } +} Property changes on: lucene/contrib/misc/src/java/org/apache/lucene/search/SearcherLifetimeManager.java ___________________________________________________________________ Added: svn:eol-style + native Index: lucene/src/java/org/apache/lucene/index/IndexReader.java =================================================================== --- lucene/src/java/org/apache/lucene/index/IndexReader.java (revision 1178852) +++ lucene/src/java/org/apache/lucene/index/IndexReader.java (working copy) @@ -233,7 +233,7 @@ public boolean tryIncRef() { int count; while ((count = refCount.get()) > 0) { - if(refCount.compareAndSet(count, count+1)) { + if (refCount.compareAndSet(count, count+1)) { return true; } } Index: lucene/src/java/org/apache/lucene/util/IOUtils.java =================================================================== --- lucene/src/java/org/apache/lucene/util/IOUtils.java (revision 1178852) +++ lucene/src/java/org/apache/lucene/util/IOUtils.java (working copy) @@ -210,8 +210,9 @@ /** adds a Throwable to the list of suppressed Exceptions of the first Throwable (if Java 7 is detected) * @param exception this exception should get the suppressed one added * @param suppressed the suppressed exception + * @lucene.internal */ - private static final void addSuppressed(Throwable exception, Throwable suppressed) { + public static final void addSuppressed(Throwable exception, Throwable suppressed) { if (SUPPRESS_METHOD != null && exception != null && suppressed != null) { try { SUPPRESS_METHOD.invoke(exception, suppressed);