Skip to content

Commit

Permalink
Ensure that a non static top docs is created during the search phase
Browse files Browse the repository at this point in the history
This change fixes an unreleased bug that trips an assertion because a static instance
shared among threads is modified during the search. This commit copies the static
instance in order to ensure that each thread can modify the value without modifying
the other instances.

Closes #37179
Closes #37266
  • Loading branch information
jimczi committed Jan 9, 2019
1 parent 1958730 commit 95479f1
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@
import java.util.Map;
import java.util.Set;

import static org.apache.lucene.util.LuceneTestCase.AwaitsFix;
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.elasticsearch.common.xcontent.support.XContentMapValues.extractValue;
import static org.elasticsearch.index.query.QueryBuilders.boolQuery;
Expand All @@ -86,7 +85,6 @@
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;

@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/37266")
public class ChildQuerySearchIT extends ParentChildTestCase {

public void testMultiLevelChild() throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -760,13 +760,7 @@ TotalHits getTotalHits() {
if (trackTotalHitsUpTo == SearchContext.TRACK_TOTAL_HITS_DISABLED) {
return null;
} else if (trackTotalHitsUpTo == SearchContext.TRACK_TOTAL_HITS_ACCURATE) {
// NORELEASE The assertion below has been replaced by a runtime exception in order to debug
// https://github.com/elastic/elasticsearch/issues/37179.
// The assertion should be restored and the exception removed when this issue is solved.
// assert totalHitsRelation == Relation.EQUAL_TO;
if (totalHitsRelation != Relation.EQUAL_TO) {
throw new IllegalStateException("Expected accurate total hits but got " + new TotalHits(totalHits, totalHitsRelation));
}
assert totalHitsRelation == Relation.EQUAL_TO;
return new TotalHits(totalHits, totalHitsRelation);
} else {
if (totalHits < trackTotalHitsUpTo) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -264,8 +264,7 @@ Collector create(Collector in) {

@Override
void postProcess(QuerySearchResult result) throws IOException {
final TopDocs topDocs = topDocsSupplier.get();
topDocs.totalHits = totalHitsSupplier.get();
final TopDocs topDocs = new TopDocs(totalHitsSupplier.get(), topDocsSupplier.get().scoreDocs);
result.topDocs(new TopDocsAndMaxScore(topDocs, maxScoreSupplier.get()),
sortAndFormats == null ? null : sortAndFormats.formats);
}
Expand All @@ -292,11 +291,11 @@ private ScrollingTopDocsCollectorContext(IndexReader reader,

@Override
void postProcess(QuerySearchResult result) throws IOException {
final TopDocs topDocs = topDocsSupplier.get();
final TopDocs topDocs = new TopDocs(totalHitsSupplier.get(), topDocsSupplier.get().scoreDocs);
final float maxScore;
if (scrollContext.totalHits == null) {
// first round
topDocs.totalHits = scrollContext.totalHits = totalHitsSupplier.get();
scrollContext.totalHits = topDocs.totalHits;
maxScore = scrollContext.maxScore = maxScoreSupplier.get();
} else {
// subsequent round: the total number of hits and
Expand Down

0 comments on commit 95479f1

Please sign in to comment.