From 4bd9dba46f9aac8717ca7008f0c14947a6d1d250 Mon Sep 17 00:00:00 2001 From: Doron Cohen Date: Tue, 18 Mar 2008 21:08:08 +0000 Subject: [PATCH] LUCENE-1238: Fixed intermittent failures of TestTimeLimitedCollector.testTimeoutMultiThreaded. git-svn-id: https://svn.apache.org/repos/asf/lucene/java/trunk@638568 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES.txt | 4 + .../lucene/search/TimeLimitedCollector.java | 40 ++++++++- .../search/TestTimeLimitedCollector.java | 90 +++++++++++++------ 3 files changed, 107 insertions(+), 27 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 31a82afb23b..d6d3ed7476e 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -171,6 +171,10 @@ Build Test Cases + 1. LUCENE-1238: Fixed intermittent failures of TestTimeLimitedCollector.testTimeoutMultiThreaded. + Within this fix, "greedy" flag was added to TimeLimitedCollector, to allow the wrapped + collector to collect also the last doc, after allowed-tTime passed. (Doron Cohen) + ======================= Release 2.3.1 2008-02-22 ======================= Bug fixes diff --git a/src/java/org/apache/lucene/search/TimeLimitedCollector.java b/src/java/org/apache/lucene/search/TimeLimitedCollector.java index ab2201f355d..09a89fb3917 100755 --- a/src/java/org/apache/lucene/search/TimeLimitedCollector.java +++ b/src/java/org/apache/lucene/search/TimeLimitedCollector.java @@ -31,7 +31,15 @@ public class TimeLimitedCollector extends HitCollector { */ public static final int DEFAULT_RESOLUTION = 20; - private static long resolution = DEFAULT_RESOLUTION; + /** + * Default for {@link #isGreedy()}. + * @see #isGreedy() + */ + public boolean DEFAULT_GREEDY = false; + + private static long resolution = DEFAULT_RESOLUTION; + + private boolean greedy = DEFAULT_GREEDY ; private static class TimerThread extends Thread { @@ -132,6 +140,11 @@ public class TimeLimitedCollector extends HitCollector { private final long timeout; private final HitCollector hc; + /** + * Create a TimeLimitedCollector wrapper over another HitCollector with a specified timeout. + * @param hc the wrapped HitCollector + * @param timeAllowed max time allowed for collecting hits after which {@link TimeExceededException} is thrown + */ public TimeLimitedCollector( final HitCollector hc, final long timeAllowed ) { this.hc = hc; t0 = TIMER_THREAD.getMilliseconds(); @@ -146,6 +159,10 @@ public class TimeLimitedCollector extends HitCollector { public void collect( final int doc, final float score ) { long time = TIMER_THREAD.getMilliseconds(); if( timeout < time) { + if (greedy) { + //System.out.println(this+" greedy: before failing, collecting doc: "+doc+" "+(time-t0)); + hc.collect( doc, score ); + } //System.out.println(this+" failing on: "+doc+" "+(time-t0)); throw new TimeExceededException( timeout-t0, time-t0, doc ); } @@ -178,4 +195,25 @@ public class TimeLimitedCollector extends HitCollector { public static void setResolution(long newResolution) { resolution = Math.max(newResolution,5); // 5 milliseconds is about the minimum reasonable time for a Object.wait(long) call. } + + /** + * Checks if this time limited collector is greedy in collecting the last hit. + * A non greedy collector, upon a timeout, would throw a {@link TimeExceededException} + * without allowing the wrapped collector to collect current doc. A greedy one would + * first allow the wrapped hit collector to collect current doc and only then + * throw a {@link TimeExceededException}. + * @see #setGreedy(boolean) + */ + public boolean isGreedy() { + return greedy; + } + + /** + * Sets whether this time limited collector is greedy. + * @param greedy true to make this time limited greedy + * @see #isGreedy() + */ + public void setGreedy(boolean greedy) { + this.greedy = greedy; + } } diff --git a/src/test/org/apache/lucene/search/TestTimeLimitedCollector.java b/src/test/org/apache/lucene/search/TestTimeLimitedCollector.java index 85553a5c163..7907043599e 100755 --- a/src/test/org/apache/lucene/search/TestTimeLimitedCollector.java +++ b/src/test/org/apache/lucene/search/TestTimeLimitedCollector.java @@ -17,7 +17,7 @@ package org.apache.lucene.search; * limitations under the License. */ -import org.apache.lucene.analysis.standard.StandardAnalyzer; +import org.apache.lucene.analysis.WhitespaceAnalyzer; import org.apache.lucene.document.Document; import org.apache.lucene.document.Field; import org.apache.lucene.index.IndexWriter; @@ -37,12 +37,12 @@ import java.util.BitSet; */ public class TestTimeLimitedCollector extends LuceneTestCase { private static final int SLOW_DOWN = 47; - private static final int TIME_ALLOWED = 17 * SLOW_DOWN; // so searches can find about 17 docs. + private static final long TIME_ALLOWED = 17 * SLOW_DOWN; // so searches can find about 17 docs. // max time allowed is relaxed for multithreading tests. // the multithread case fails when setting this to 1 (no slack) and launching many threads (>2000). // but this is not a real failure, just noise. - private static final double MULTI_THREAD_SLACK = 3; + private static final double MULTI_THREAD_SLACK = 7; private static final int N_DOCS = 3000; private static final int N_THREADS = 50; @@ -60,6 +60,7 @@ public class TestTimeLimitedCollector extends LuceneTestCase { */ protected void setUp() throws Exception { final String docText[] = { + "docThatNeverMatchesSoWeCanRequireLastDocCollectedToBeGreaterThanZero", "one blah three", "one foo three multiOne", "one foobar three multiThree", @@ -69,7 +70,7 @@ public class TestTimeLimitedCollector extends LuceneTestCase { "blueberry pizza", }; Directory directory = new RAMDirectory(); - IndexWriter iw = new IndexWriter(directory, new StandardAnalyzer(), true, MaxFieldLength.UNLIMITED); + IndexWriter iw = new IndexWriter(directory, new WhitespaceAnalyzer(), true, MaxFieldLength.UNLIMITED); for (int i=0; i 0 ); - assertTrue( "last doc collected cannot be 0!", exception.getLastDocCollected() > 0 ); - assertEquals( exception.getTimeAllowed(), TIME_ALLOWED); - assertTrue ( "elapsed="+exception.getTimeElapsed()+" <= (allowed-resolution)="+(TIME_ALLOWED-TimeLimitedCollector.getResolution()), - exception.getTimeElapsed() > TIME_ALLOWED-TimeLimitedCollector.getResolution()); - assertTrue ( "lastDoc="+exception.getLastDocCollected()+ - " ,&& allowed="+exception.getTimeAllowed() + - " ,&& elapsed="+exception.getTimeElapsed() + + + // must get exception + assertNotNull( "Timeout expected!", timoutException ); + + // greediness affect last doc collected + int exceptionDoc = timoutException.getLastDocCollected(); + int lastCollected = myHc.getLastDocCollected(); + assertTrue( "doc collected at timeout must be > 0!", exceptionDoc > 0 ); + if (greedy) { + assertTrue("greedy="+greedy+" exceptionDoc="+exceptionDoc+" != lastCollected="+lastCollected, exceptionDoc==lastCollected); + assertTrue("greedy, but no hits found!", myHc.hitCount() > 0 ); + } else { + assertTrue("greedy="+greedy+" exceptionDoc="+exceptionDoc+" not > lastCollected="+lastCollected, exceptionDoc>lastCollected); + } + + // verify that elapsed time at exception is within valid limits + assertEquals( timoutException.getTimeAllowed(), TIME_ALLOWED); + // a) Not too early + assertTrue ( "elapsed="+timoutException.getTimeElapsed()+" <= (allowed-resolution)="+(TIME_ALLOWED-TimeLimitedCollector.getResolution()), + timoutException.getTimeElapsed() > TIME_ALLOWED-TimeLimitedCollector.getResolution()); + // b) Not too late (this part might be problematic in a busy system, consider removing it if it raises false test failures. + assertTrue ( "lastDoc="+exceptionDoc+ + " ,&& allowed="+timoutException.getTimeAllowed() + + " ,&& elapsed="+timoutException.getTimeElapsed() + " >= " + maxTimeStr(multiThreaded), - exception.getTimeElapsed() < maxTime(multiThreaded)); + timoutException.getTimeElapsed() < maxTime(multiThreaded)); } private long maxTime(boolean multiThreaded) { @@ -190,17 +222,17 @@ public class TestTimeLimitedCollector extends LuceneTestCase { long resolution = 20 * TimeLimitedCollector.DEFAULT_RESOLUTION; //400 TimeLimitedCollector.setResolution(resolution); assertEquals(resolution, TimeLimitedCollector.getResolution()); - doTestTimeout(false); + doTestTimeout(false,true); // decrease much and test resolution = 5; TimeLimitedCollector.setResolution(resolution); assertEquals(resolution, TimeLimitedCollector.getResolution()); - doTestTimeout(false); + doTestTimeout(false,true); // return to default and test resolution = TimeLimitedCollector.DEFAULT_RESOLUTION; TimeLimitedCollector.setResolution(resolution); assertEquals(resolution, TimeLimitedCollector.getResolution()); - doTestTimeout(false); + doTestTimeout(false,true); } finally { TimeLimitedCollector.setResolution(TimeLimitedCollector.DEFAULT_RESOLUTION); } @@ -228,7 +260,7 @@ public class TestTimeLimitedCollector extends LuceneTestCase { threadArray[num] = new Thread() { public void run() { if (withTimeout) { - doTestTimeout(true); + doTestTimeout(true,true); } else { doTestSearch(); } @@ -260,6 +292,7 @@ public class TestTimeLimitedCollector extends LuceneTestCase { { private final BitSet bits = new BitSet(); private int slowdown = 0; + private int lastDocCollected = -1; /** * amount of time to wait on each collect to simulate a long iteration @@ -278,11 +311,16 @@ public class TestTimeLimitedCollector extends LuceneTestCase { } } bits.set( doc ); + lastDocCollected = doc; } public int hitCount() { return bits.cardinality(); } + + public int getLastDocCollected() { + return lastDocCollected; + } }