lucene-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jpou...@apache.org
Subject svn commit: r1652242 - in /lucene/dev/trunk: ./ lucene/ lucene/core/ lucene/core/src/java/org/apache/lucene/search/ lucene/test-framework/ lucene/test-framework/src/java/org/apache/lucene/search/
Date Thu, 15 Jan 2015 19:21:40 GMT
Author: jpountz
Date: Thu Jan 15 19:21:40 2015
New Revision: 1652242

URL: http://svn.apache.org/r1652242
Log:
LUCENE-6185: Fix IndexSearcher with threads to not collect documents out of order.

Added:
    lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/search/AssertingCollector.java
      - copied unchanged from r1652239, lucene/dev/branches/branch_5x/lucene/test-framework/src/java/org/apache/lucene/search/AssertingCollector.java
Modified:
    lucene/dev/trunk/   (props changed)
    lucene/dev/trunk/lucene/   (props changed)
    lucene/dev/trunk/lucene/core/   (props changed)
    lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/IndexSearcher.java
    lucene/dev/trunk/lucene/test-framework/   (props changed)
    lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/search/AssertingIndexSearcher.java
    lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/search/AssertingLeafCollector.java

Modified: lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/IndexSearcher.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/IndexSearcher.java?rev=1652242&r1=1652241&r2=1652242&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/IndexSearcher.java (original)
+++ lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/IndexSearcher.java Thu
Jan 15 19:21:40 2015
@@ -18,25 +18,21 @@ package org.apache.lucene.search;
  */
 
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Iterator;
 import java.util.List;
-import java.util.NoSuchElementException;
 import java.util.Set;
 import java.util.concurrent.Callable;
-import java.util.concurrent.CompletionService;
 import java.util.concurrent.ExecutionException;
-import java.util.concurrent.Executor;
-import java.util.concurrent.ExecutorCompletionService;
 import java.util.concurrent.ExecutorService;
-import java.util.concurrent.locks.Lock;
-import java.util.concurrent.locks.ReentrantLock;
+import java.util.concurrent.Future;
 
-import org.apache.lucene.index.LeafReaderContext;
 import org.apache.lucene.index.DirectoryReader; // javadocs
 import org.apache.lucene.index.IndexReader;
-import org.apache.lucene.index.MultiFields;
 import org.apache.lucene.index.IndexReaderContext;
+import org.apache.lucene.index.IndexWriter; // javadocs
+import org.apache.lucene.index.LeafReaderContext;
+import org.apache.lucene.index.MultiFields;
 import org.apache.lucene.index.ReaderUtil;
 import org.apache.lucene.index.StoredDocument;
 import org.apache.lucene.index.StoredFieldVisitor;
@@ -47,7 +43,6 @@ import org.apache.lucene.search.similari
 import org.apache.lucene.search.similarities.Similarity;
 import org.apache.lucene.store.NIOFSDirectory;    // javadoc
 import org.apache.lucene.util.ThreadInterruptedException;
-import org.apache.lucene.index.IndexWriter; // javadocs
 
 /** Implements search over a single IndexReader.
  *
@@ -439,28 +434,21 @@ public class IndexSearcher {
     if (executor == null) {
       return search(leafContexts, weight, after, nDocs);
     } else {
-      final HitQueue hq = new HitQueue(nDocs, false);
-      final Lock lock = new ReentrantLock();
-      final ExecutionHelper<TopDocs> runner = new ExecutionHelper<>(executor);
-    
-      for (int i = 0; i < leafSlices.length; i++) { // search each sub
-        runner.submit(new SearcherCallableNoSort(lock, this, leafSlices[i], weight, after,
nDocs, hq));
+      final List<Future<TopDocs>> topDocsFutures = new ArrayList<>(leafSlices.length);
+      for (int i = 0; i < leafSlices.length; i++) { // search each leaf slice
+        topDocsFutures.add(executor.submit(new SearcherCallableNoSort(this, leafSlices[i],
weight, after, nDocs)));
       }
-
-      int totalHits = 0;
-      float maxScore = Float.NEGATIVE_INFINITY;
-      for (final TopDocs topDocs : runner) {
-        if(topDocs.totalHits != 0) {
-          totalHits += topDocs.totalHits;
-          maxScore = Math.max(maxScore, topDocs.getMaxScore());
+      final TopDocs[] topDocs = new TopDocs[leafSlices.length];
+      for (int i = 0; i < leafSlices.length; i++) {
+        try {
+          topDocs[i] = topDocsFutures.get(i).get();
+        } catch (InterruptedException e) {
+          throw new ThreadInterruptedException(e);
+        } catch (ExecutionException e) {
+          throw new RuntimeException(e);
         }
       }
-
-      final ScoreDoc[] scoreDocs = new ScoreDoc[hq.size()];
-      for (int i = hq.size() - 1; i >= 0; i--) // put docs in array
-        scoreDocs[i] = hq.pop();
-
-      return new TopDocs(totalHits, scoreDocs, maxScore);
+      return TopDocs.merge(null, nDocs, topDocs);
     }
   }
 
@@ -524,30 +512,21 @@ public class IndexSearcher {
       // use all leaves here!
       return search(leafContexts, weight, after, nDocs, sort, fillFields, doDocScores, doMaxScore);
     } else {
-      final TopFieldCollector topCollector = TopFieldCollector.create(sort, nDocs,
-                                                                      after,
-                                                                      fillFields,
-                                                                      doDocScores,
-                                                                      doMaxScore);
-
-      final Lock lock = new ReentrantLock();
-      final ExecutionHelper<TopFieldDocs> runner = new ExecutionHelper<>(executor);
+      final List<Future<TopFieldDocs>> topDocsFutures = new ArrayList<>(leafSlices.length);
       for (int i = 0; i < leafSlices.length; i++) { // search each leaf slice
-        runner.submit(
-                      new SearcherCallableWithSort(lock, this, leafSlices[i], weight, after,
nDocs, topCollector, sort, doDocScores, doMaxScore));
+        topDocsFutures.add(executor.submit(new SearcherCallableWithSort(this, leafSlices[i],
weight, after, nDocs, sort, doDocScores, doMaxScore)));
       }
-      int totalHits = 0;
-      float maxScore = Float.NEGATIVE_INFINITY;
-      for (final TopFieldDocs topFieldDocs : runner) {
-        if (topFieldDocs.totalHits != 0) {
-          totalHits += topFieldDocs.totalHits;
-          maxScore = Math.max(maxScore, topFieldDocs.getMaxScore());
+      final TopFieldDocs[] topDocs = new TopFieldDocs[leafSlices.length];
+      for (int i = 0; i < leafSlices.length; i++) {
+        try {
+          topDocs[i] = topDocsFutures.get(i).get();
+        } catch (InterruptedException e) {
+          throw new ThreadInterruptedException(e);
+        } catch (ExecutionException e) {
+          throw new RuntimeException(e);
         }
       }
-
-      final TopFieldDocs topDocs = (TopFieldDocs) topCollector.topDocs();
-
-      return new TopFieldDocs(totalHits, topDocs.scoreDocs, topDocs.fields, topDocs.getMaxScore());
+      return (TopFieldDocs) TopDocs.merge(sort, nDocs, topDocs);
     }
   }
   
@@ -697,42 +676,24 @@ public class IndexSearcher {
    */
   private static final class SearcherCallableNoSort implements Callable<TopDocs> {
 
-    private final Lock lock;
     private final IndexSearcher searcher;
     private final Weight weight;
     private final ScoreDoc after;
     private final int nDocs;
-    private final HitQueue hq;
     private final LeafSlice slice;
 
-    public SearcherCallableNoSort(Lock lock, IndexSearcher searcher, LeafSlice slice,  Weight
weight,
-        ScoreDoc after, int nDocs, HitQueue hq) {
-      this.lock = lock;
+    public SearcherCallableNoSort(IndexSearcher searcher, LeafSlice slice, Weight weight,
+        ScoreDoc after, int nDocs) {
       this.searcher = searcher;
       this.weight = weight;
       this.after = after;
       this.nDocs = nDocs;
-      this.hq = hq;
       this.slice = slice;
     }
 
     @Override
     public TopDocs call() throws IOException {
-      final TopDocs docs = searcher.search(Arrays.asList(slice.leaves), weight, after, nDocs);
-      final ScoreDoc[] scoreDocs = docs.scoreDocs;
-      //it would be so nice if we had a thread-safe insert 
-      lock.lock();
-      try {
-        for (int j = 0; j < scoreDocs.length; j++) { // merge scoreDocs into hq
-          final ScoreDoc scoreDoc = scoreDocs[j];
-          if (scoreDoc == hq.insertWithOverflow(scoreDoc)) {
-            break;
-          }
-        }
-      } finally {
-        lock.unlock();
-      }
-      return docs;
+      return searcher.search(Arrays.asList(slice.leaves), weight, after, nDocs);
     }
   }
 
@@ -742,25 +703,21 @@ public class IndexSearcher {
    */
   private static final class SearcherCallableWithSort implements Callable<TopFieldDocs>
{
 
-    private final Lock lock;
     private final IndexSearcher searcher;
     private final Weight weight;
     private final int nDocs;
-    private final TopFieldCollector hq;
     private final Sort sort;
     private final LeafSlice slice;
     private final FieldDoc after;
     private final boolean doDocScores;
     private final boolean doMaxScore;
 
-    public SearcherCallableWithSort(Lock lock, IndexSearcher searcher, LeafSlice slice, Weight
weight,
-                                    FieldDoc after, int nDocs, TopFieldCollector hq, Sort
sort,
+    public SearcherCallableWithSort(IndexSearcher searcher, LeafSlice slice, Weight weight,
+                                    FieldDoc after, int nDocs, Sort sort,
                                     boolean doDocScores, boolean doMaxScore) {
-      this.lock = lock;
       this.searcher = searcher;
       this.weight = weight;
       this.nDocs = nDocs;
-      this.hq = hq;
       this.sort = sort;
       this.slice = slice;
       this.after = after;
@@ -768,85 +725,11 @@ public class IndexSearcher {
       this.doMaxScore = doMaxScore;
     }
 
-    private final FakeScorer fakeScorer = new FakeScorer();
-
     @Override
     public TopFieldDocs call() throws IOException {
       assert slice.leaves.length == 1;
-      final TopFieldDocs docs = searcher.search(Arrays.asList(slice.leaves),
+      return searcher.search(Arrays.asList(slice.leaves),
           weight, after, nDocs, sort, true, doDocScores || sort.needsScores(), doMaxScore);
-      lock.lock();
-      try {
-        final LeafReaderContext ctx = slice.leaves[0];
-        final int base = ctx.docBase;
-        final LeafCollector collector = hq.getLeafCollector(ctx);
-        collector.setScorer(fakeScorer);
-        for(ScoreDoc scoreDoc : docs.scoreDocs) {
-          fakeScorer.doc = scoreDoc.doc - base;
-          fakeScorer.score = scoreDoc.score;
-          collector.collect(scoreDoc.doc-base);
-        }
-
-        // Carry over maxScore from sub:
-        if (doMaxScore && docs.getMaxScore() > hq.maxScore) {
-          hq.maxScore = docs.getMaxScore();
-        }
-      } finally {
-        lock.unlock();
-      }
-      return docs;
-    }
-  }
-
-  /**
-   * A helper class that wraps a {@link CompletionService} and provides an
-   * iterable interface to the completed {@link Callable} instances.
-   * 
-   * @param <T>
-   *          the type of the {@link Callable} return value
-   */
-  private static final class ExecutionHelper<T> implements Iterator<T>, Iterable<T>
{
-    private final CompletionService<T> service;
-    private int numTasks;
-
-    ExecutionHelper(final Executor executor) {
-      this.service = new ExecutorCompletionService<>(executor);
-    }
-
-    @Override
-    public boolean hasNext() {
-      return numTasks > 0;
-    }
-
-    public void submit(Callable<T> task) {
-      this.service.submit(task);
-      ++numTasks;
-    }
-
-    @Override
-    public T next() {
-      if(!this.hasNext()) 
-        throw new NoSuchElementException("next() is called but hasNext() returned false");
-      try {
-        return service.take().get();
-      } catch (InterruptedException e) {
-        throw new ThreadInterruptedException(e);
-      } catch (ExecutionException e) {
-        throw new RuntimeException(e);
-      } finally {
-        --numTasks;
-      }
-    }
-
-    @Override
-    public void remove() {
-      throw new UnsupportedOperationException();
-    }
-
-    @Override
-    public Iterator<T> iterator() {
-      // use the shortcut here - this is only used in a private context
-      return this;
     }
   }
 

Modified: lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/search/AssertingIndexSearcher.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/search/AssertingIndexSearcher.java?rev=1652242&r1=1652241&r2=1652242&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/search/AssertingIndexSearcher.java
(original)
+++ lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/search/AssertingIndexSearcher.java
Thu Jan 15 19:21:40 2015
@@ -91,7 +91,7 @@ public class AssertingIndexSearcher exte
   @Override
   protected void search(List<LeafReaderContext> leaves, Weight weight, Collector collector)
throws IOException {
     // TODO: shouldn't we AssertingCollector.wrap(collector) here?
-    super.search(leaves, AssertingWeight.wrap(random, weight), collector);
+    super.search(leaves, AssertingWeight.wrap(random, weight), AssertingCollector.wrap(random,
collector));
   }
 
   @Override

Modified: lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/search/AssertingLeafCollector.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/search/AssertingLeafCollector.java?rev=1652242&r1=1652241&r2=1652242&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/search/AssertingLeafCollector.java
(original)
+++ lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/search/AssertingLeafCollector.java
Thu Jan 15 19:21:40 2015
@@ -22,7 +22,7 @@ import java.util.Random;
 
 /** Wraps another Collector and checks that
  *  order is respected. */
-final class AssertingLeafCollector extends FilterLeafCollector {
+class AssertingLeafCollector extends FilterLeafCollector {
 
   private final Random random;
   private final int max;



Mime
View raw message