You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by jp...@apache.org on 2015/01/15 20:18:55 UTC
svn commit: r1652239 - in /lucene/dev/branches/branch_5x/lucene:
core/src/java/org/apache/lucene/search/
test-framework/src/java/org/apache/lucene/search/
Author: jpountz
Date: Thu Jan 15 19:18:55 2015
New Revision: 1652239
URL: http://svn.apache.org/r1652239
Log:
LUCENE-6185: Fix IndexSearcher with threads to not collect documents out of order.
Added:
lucene/dev/branches/branch_5x/lucene/test-framework/src/java/org/apache/lucene/search/AssertingCollector.java (with props)
Modified:
lucene/dev/branches/branch_5x/lucene/core/src/java/org/apache/lucene/search/IndexSearcher.java
lucene/dev/branches/branch_5x/lucene/test-framework/src/java/org/apache/lucene/search/AssertingIndexSearcher.java
lucene/dev/branches/branch_5x/lucene/test-framework/src/java/org/apache/lucene/search/AssertingLeafCollector.java
Modified: lucene/dev/branches/branch_5x/lucene/core/src/java/org/apache/lucene/search/IndexSearcher.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/lucene/core/src/java/org/apache/lucene/search/IndexSearcher.java?rev=1652239&r1=1652238&r2=1652239&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/lucene/core/src/java/org/apache/lucene/search/IndexSearcher.java (original)
+++ lucene/dev/branches/branch_5x/lucene/core/src/java/org/apache/lucene/search/IndexSearcher.java Thu Jan 15 19:18:55 2015
@@ -18,26 +18,22 @@ 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.document.Document;
-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.StoredFieldVisitor;
import org.apache.lucene.index.Term;
@@ -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;
}
}
Added: lucene/dev/branches/branch_5x/lucene/test-framework/src/java/org/apache/lucene/search/AssertingCollector.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/lucene/test-framework/src/java/org/apache/lucene/search/AssertingCollector.java?rev=1652239&view=auto
==============================================================================
--- lucene/dev/branches/branch_5x/lucene/test-framework/src/java/org/apache/lucene/search/AssertingCollector.java (added)
+++ lucene/dev/branches/branch_5x/lucene/test-framework/src/java/org/apache/lucene/search/AssertingCollector.java Thu Jan 15 19:18:55 2015
@@ -0,0 +1,63 @@
+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.IOException;
+import java.util.Random;
+
+import org.apache.lucene.index.LeafReaderContext;
+
+/**
+ * A collector that asserts that it is used correctly.
+ */
+class AssertingCollector extends FilterCollector {
+
+ private final Random random;
+ private int maxDoc = -1;
+
+ /** Wrap the given collector in order to add assertions. */
+ public static Collector wrap(Random random, Collector in) {
+ if (in instanceof AssertingCollector) {
+ return in;
+ }
+ return new AssertingCollector(random, in);
+ }
+
+ private AssertingCollector(Random random, Collector in) {
+ super(in);
+ this.random = random;
+ }
+
+ @Override
+ public LeafCollector getLeafCollector(LeafReaderContext context) throws IOException {
+ final LeafCollector in = super.getLeafCollector(context);
+ final int docBase = context.docBase;
+ return new AssertingLeafCollector(random, in, DocIdSetIterator.NO_MORE_DOCS) {
+ @Override
+ public void collect(int doc) throws IOException {
+ // check that documents are scored in order globally,
+ // not only per segment
+ assert docBase + doc >= maxDoc : "collection is not in order: current doc="
+ + (docBase + doc) + " while " + maxDoc + " has already been collected";
+ super.collect(doc);
+ maxDoc = docBase + doc;
+ }
+ };
+ }
+
+}
Modified: lucene/dev/branches/branch_5x/lucene/test-framework/src/java/org/apache/lucene/search/AssertingIndexSearcher.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/lucene/test-framework/src/java/org/apache/lucene/search/AssertingIndexSearcher.java?rev=1652239&r1=1652238&r2=1652239&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/lucene/test-framework/src/java/org/apache/lucene/search/AssertingIndexSearcher.java (original)
+++ lucene/dev/branches/branch_5x/lucene/test-framework/src/java/org/apache/lucene/search/AssertingIndexSearcher.java Thu Jan 15 19:18:55 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/branches/branch_5x/lucene/test-framework/src/java/org/apache/lucene/search/AssertingLeafCollector.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/lucene/test-framework/src/java/org/apache/lucene/search/AssertingLeafCollector.java?rev=1652239&r1=1652238&r2=1652239&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/lucene/test-framework/src/java/org/apache/lucene/search/AssertingLeafCollector.java (original)
+++ lucene/dev/branches/branch_5x/lucene/test-framework/src/java/org/apache/lucene/search/AssertingLeafCollector.java Thu Jan 15 19:18:55 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;