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

@@ -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

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;
+  }
 }

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<N_DOCS; i++) {
       add(docText[i%docText.length], iw);
@@ -81,7 +82,7 @@ public class TestTimeLimitedCollector extends LuceneTestCase {
     for (int i = 0; i < docText.length; i++) {
       qtxt += ' ' + docText[i]; // large query so that search will be longer
     }
-    QueryParser queryParser = new QueryParser(FIELD_NAME, new StandardAnalyzer());
+    QueryParser queryParser = new QueryParser(FIELD_NAME, new WhitespaceAnalyzer());
     query = queryParser.parse(qtxt);
     
     // warm the searcher
@@ -119,7 +120,8 @@ public class TestTimeLimitedCollector extends LuceneTestCase {
       totalResults = myHc.hitCount();
       
       myHc = new MyHitCollector();
-      HitCollector tlCollector = new TimeLimitedCollector(myHc, 3600000); // 1 hour
+      long oneHour = 3600000;
+      HitCollector tlCollector = createTimedCollector(myHc, oneHour, false);
       search(tlCollector);
       totalTLCResults = myHc.hitCount();
     } catch (Exception e) {
@@ -128,37 +130,67 @@ public class TestTimeLimitedCollector extends LuceneTestCase {
     assertEquals( "Wrong number of results!", totalResults, totalTLCResults );
   }
 
+  private HitCollector createTimedCollector(MyHitCollector hc, long timeAllowed, boolean greedy) {
+    TimeLimitedCollector res = new TimeLimitedCollector(hc, timeAllowed);
+    res.setGreedy(greedy); // set to true to make sure at least one doc is collected.
+    return res;
+  }
+
   /**
    * Test that timeout is obtained, and soon enough!
    */
-  public void testTimeout() {
+  public void testTimeoutGreedy() {
-    doTestTimeout(false);
+    doTestTimeout(false, true);
   }
   
-  private void doTestTimeout(boolean multiThreaded) {
+  /**
+   * Test that timeout is obtained, and soon enough!
+   */
+  public void testTimeoutNotGreedy() {
+    doTestTimeout(false, false);
+  }
+
+  private void doTestTimeout(boolean multiThreaded, boolean greedy) {
+    // setup
     MyHitCollector myHc = new MyHitCollector();
     myHc.setSlowDown(SLOW_DOWN);
-    HitCollector tlCollector = new TimeLimitedCollector(myHc, TIME_ALLOWED);
+    HitCollector tlCollector = createTimedCollector(myHc, TIME_ALLOWED, greedy);
 
-    TimeLimitedCollector.TimeExceededException exception = null;
+    // search
+    TimeLimitedCollector.TimeExceededException timoutException = null;
     try {
       search(tlCollector);
     } catch (TimeLimitedCollector.TimeExceededException x) {
-      exception = x;
+      timoutException = x;
     } catch (Exception e) {
       assertTrue("Unexpected exception: "+e, false); //==fail
     }
-    assertNotNull( "Timeout expected!", exception );
+    
-    assertTrue( "no hits found!", myHc.hitCount() > 0 );
+    // must get exception
-    assertTrue( "last doc collected cannot be 0!", exception.getLastDocCollected() > 0 );
+    assertNotNull( "Timeout expected!", timoutException );
-    assertEquals( exception.getTimeAllowed(), TIME_ALLOWED);
+
-    assertTrue ( "elapsed="+exception.getTimeElapsed()+" <= (allowed-resolution)="+(TIME_ALLOWED-TimeLimitedCollector.getResolution()),
+    // greediness affect last doc collected
-        exception.getTimeElapsed() > TIME_ALLOWED-TimeLimitedCollector.getResolution());
+    int exceptionDoc = timoutException.getLastDocCollected();
-    assertTrue ( "lastDoc="+exception.getLastDocCollected()+
+    int lastCollected = myHc.getLastDocCollected(); 
-        " ,&& allowed="+exception.getTimeAllowed() +
+    assertTrue( "doc collected at timeout must be > 0!", exceptionDoc > 0 );
-        " ,&& elapsed="+exception.getTimeElapsed() +
+    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;
+    }
     
   }