From 95479f17667c7d27fc480c260a1c380d681d8b93 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Wed, 9 Jan 2019 22:57:34 +0100 Subject: [PATCH] Ensure that a non static top docs is created during the search phase 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 --- .../org/elasticsearch/join/query/ChildQuerySearchIT.java | 2 -- .../action/search/SearchPhaseController.java | 8 +------- .../search/query/TopDocsCollectorContext.java | 7 +++---- 3 files changed, 4 insertions(+), 13 deletions(-) diff --git a/modules/parent-join/src/test/java/org/elasticsearch/join/query/ChildQuerySearchIT.java b/modules/parent-join/src/test/java/org/elasticsearch/join/query/ChildQuerySearchIT.java index d92ff102fe8..8520b632056 100644 --- a/modules/parent-join/src/test/java/org/elasticsearch/join/query/ChildQuerySearchIT.java +++ b/modules/parent-join/src/test/java/org/elasticsearch/join/query/ChildQuerySearchIT.java @@ -59,7 +59,6 @@ import java.util.Locale; 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; @@ -86,7 +85,6 @@ import static org.hamcrest.Matchers.containsString; 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 { diff --git a/server/src/main/java/org/elasticsearch/action/search/SearchPhaseController.java b/server/src/main/java/org/elasticsearch/action/search/SearchPhaseController.java index 8fada8938f8..418d95b2077 100644 --- a/server/src/main/java/org/elasticsearch/action/search/SearchPhaseController.java +++ b/server/src/main/java/org/elasticsearch/action/search/SearchPhaseController.java @@ -760,13 +760,7 @@ public final class SearchPhaseController { 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) { diff --git a/server/src/main/java/org/elasticsearch/search/query/TopDocsCollectorContext.java b/server/src/main/java/org/elasticsearch/search/query/TopDocsCollectorContext.java index 4f648666f55..6312fd3856b 100644 --- a/server/src/main/java/org/elasticsearch/search/query/TopDocsCollectorContext.java +++ b/server/src/main/java/org/elasticsearch/search/query/TopDocsCollectorContext.java @@ -264,8 +264,7 @@ abstract class TopDocsCollectorContext extends QueryCollectorContext { @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); } @@ -292,11 +291,11 @@ abstract class TopDocsCollectorContext extends QueryCollectorContext { @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