From c8988b1d61b1df29332bc21a73731a8c768c3c6b Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Wed, 5 Dec 2018 22:48:57 +0100 Subject: [PATCH 1/4] Always set total hits to null when track total hits is false --- .../elasticsearch/action/search/SearchPhaseController.java | 2 +- .../elasticsearch/search/query/TopDocsCollectorContext.java | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) 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 28a7193646001..5713dc4764bc2 100644 --- a/server/src/main/java/org/elasticsearch/action/search/SearchPhaseController.java +++ b/server/src/main/java/org/elasticsearch/action/search/SearchPhaseController.java @@ -761,7 +761,7 @@ static final class TopDocsStats { TopDocsStats(boolean trackTotalHits) { this.trackTotalHits = trackTotalHits; this.totalHits = 0; - this.totalHitsRelation = trackTotalHits ? Relation.EQUAL_TO : Relation.GREATER_THAN_OR_EQUAL_TO; + this.totalHitsRelation = Relation.EQUAL_TO; } TotalHits getTotalHits() { 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 fcf70a4f98c05..cfb6ff9ab5199 100644 --- a/server/src/main/java/org/elasticsearch/search/query/TopDocsCollectorContext.java +++ b/server/src/main/java/org/elasticsearch/search/query/TopDocsCollectorContext.java @@ -111,8 +111,7 @@ private EmptyTopDocsCollectorContext(IndexReader reader, Query query, } } else { this.collector = new EarlyTerminatingCollector(new TotalHitCountCollector(), 0, false); - // for bwc hit count is set to 0, it will be converted to -1 by the coordinating node - this.hitCountSupplier = () -> new TotalHits(0, TotalHits.Relation.GREATER_THAN_OR_EQUAL_TO); + this.hitCountSupplier = () -> null; } } @@ -223,8 +222,7 @@ private SimpleTopDocsCollectorContext(IndexReader reader, topDocsCollector = createCollector(sortAndFormats, numHits, searchAfter, 1); // don't compute hit counts via the collector topDocsSupplier = new CachedSupplier<>(topDocsCollector::topDocs); if (hitCount == -1) { - assert trackTotalHits == false; - totalHitsSupplier = () -> new TotalHits(0, TotalHits.Relation.GREATER_THAN_OR_EQUAL_TO); + totalHitsSupplier = () -> null; } else { totalHitsSupplier = () -> new TotalHits(hitCount, TotalHits.Relation.EQUAL_TO); } From bae0663a42b865e6f5287a247622f0d5bd0de475 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Wed, 5 Dec 2018 23:43:43 +0100 Subject: [PATCH 2/4] handle shard transport serialization when total hits is null --- .../elasticsearch/common/lucene/Lucene.java | 34 ++++++++++++++----- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/lucene/Lucene.java b/server/src/main/java/org/elasticsearch/common/lucene/Lucene.java index 38ab9a6d7d48c..1335d6445625a 100644 --- a/server/src/main/java/org/elasticsearch/common/lucene/Lucene.java +++ b/server/src/main/java/org/elasticsearch/common/lucene/Lucene.java @@ -299,12 +299,17 @@ public static boolean exists(IndexSearcher searcher, Query query) throws IOExcep } private static TotalHits readTotalHits(StreamInput in) throws IOException { - long totalHits = in.readVLong(); - TotalHits.Relation totalHitsRelation = TotalHits.Relation.EQUAL_TO; if (in.getVersion().onOrAfter(org.elasticsearch.Version.V_7_0_0)) { - totalHitsRelation = in.readEnum(TotalHits.Relation.class); + if (in.readBoolean()) { + long totalHits = in.readVLong(); + TotalHits.Relation totalHitsRelation = in.readEnum(TotalHits.Relation.class); + return new TotalHits(totalHits, totalHitsRelation); + } else { + return null; + } + } else { + return new TotalHits(in.readVLong(), TotalHits.Relation.EQUAL_TO); } - return new TotalHits(totalHits, totalHitsRelation); } public static TopDocsAndMaxScore readTopDocs(StreamInput in) throws IOException { @@ -419,11 +424,24 @@ public static ScoreDoc readScoreDoc(StreamInput in) throws IOException { private static final Class GEO_DISTANCE_SORT_TYPE_CLASS = LatLonDocValuesField.newDistanceSort("some_geo_field", 0, 0).getClass(); private static void writeTotalHits(StreamOutput out, TotalHits totalHits) throws IOException { - out.writeVLong(totalHits.value); + boolean hasTotalHits = totalHits != null; if (out.getVersion().onOrAfter(org.elasticsearch.Version.V_7_0_0)) { - out.writeEnum(totalHits.relation); - } else if (totalHits.value > 0 && totalHits.relation != TotalHits.Relation.EQUAL_TO) { - throw new IllegalArgumentException("Cannot serialize approximate total hit counts to nodes that are on a version < 7.0.0"); + out.writeBoolean(hasTotalHits); + if (hasTotalHits) { + out.writeVLong(totalHits.value); + out.writeEnum(totalHits.relation); + } + } else { + if (hasTotalHits) { + out.writeVLong(totalHits.value); + if (totalHits.value > 0 && totalHits.relation != TotalHits.Relation.EQUAL_TO) { + throw new IllegalArgumentException("Cannot serialize approximate total hit counts to nodes that " + + "are on a version < 7.0.0"); + } + } else { + // for bwc we use 0 but this will be translated to -1 by the coordinating node + out.writeVLong(0); + } } } From 0be716b960615f012ffd58c5ab2922e3a559f9a5 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Thu, 6 Dec 2018 00:02:43 +0100 Subject: [PATCH 3/4] set total hits to null when track_total_hits is false even if we can infer the total number of hits --- .../org/elasticsearch/search/query/TopDocsCollectorContext.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 cfb6ff9ab5199..d7258d81adcf7 100644 --- a/server/src/main/java/org/elasticsearch/search/query/TopDocsCollectorContext.java +++ b/server/src/main/java/org/elasticsearch/search/query/TopDocsCollectorContext.java @@ -221,7 +221,7 @@ private SimpleTopDocsCollectorContext(IndexReader reader, } else { topDocsCollector = createCollector(sortAndFormats, numHits, searchAfter, 1); // don't compute hit counts via the collector topDocsSupplier = new CachedSupplier<>(topDocsCollector::topDocs); - if (hitCount == -1) { + if (trackTotalHits == false) { totalHitsSupplier = () -> null; } else { totalHitsSupplier = () -> new TotalHits(hitCount, TotalHits.Relation.EQUAL_TO); From 809af6ed2cb98ac8ec4bc0eb22ec1c309ad46ad9 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Thu, 6 Dec 2018 01:16:14 +0100 Subject: [PATCH 4/4] reuse Lucene#read/writeTotalHits --- .../elasticsearch/common/lucene/Lucene.java | 38 +++++-------------- .../org/elasticsearch/search/SearchHits.java | 17 ++------- .../search/query/TopDocsCollectorContext.java | 6 +-- 3 files changed, 16 insertions(+), 45 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/lucene/Lucene.java b/server/src/main/java/org/elasticsearch/common/lucene/Lucene.java index 1335d6445625a..f515db43ab9a5 100644 --- a/server/src/main/java/org/elasticsearch/common/lucene/Lucene.java +++ b/server/src/main/java/org/elasticsearch/common/lucene/Lucene.java @@ -298,18 +298,13 @@ public static boolean exists(IndexSearcher searcher, Query query) throws IOExcep return false; } - private static TotalHits readTotalHits(StreamInput in) throws IOException { + public static TotalHits readTotalHits(StreamInput in) throws IOException { + long totalHits = in.readVLong(); + TotalHits.Relation totalHitsRelation = TotalHits.Relation.EQUAL_TO; if (in.getVersion().onOrAfter(org.elasticsearch.Version.V_7_0_0)) { - if (in.readBoolean()) { - long totalHits = in.readVLong(); - TotalHits.Relation totalHitsRelation = in.readEnum(TotalHits.Relation.class); - return new TotalHits(totalHits, totalHitsRelation); - } else { - return null; - } - } else { - return new TotalHits(in.readVLong(), TotalHits.Relation.EQUAL_TO); + totalHitsRelation = in.readEnum(TotalHits.Relation.class); } + return new TotalHits(totalHits, totalHitsRelation); } public static TopDocsAndMaxScore readTopDocs(StreamInput in) throws IOException { @@ -423,25 +418,12 @@ public static ScoreDoc readScoreDoc(StreamInput in) throws IOException { private static final Class GEO_DISTANCE_SORT_TYPE_CLASS = LatLonDocValuesField.newDistanceSort("some_geo_field", 0, 0).getClass(); - private static void writeTotalHits(StreamOutput out, TotalHits totalHits) throws IOException { - boolean hasTotalHits = totalHits != null; + public static void writeTotalHits(StreamOutput out, TotalHits totalHits) throws IOException { + out.writeVLong(totalHits.value); if (out.getVersion().onOrAfter(org.elasticsearch.Version.V_7_0_0)) { - out.writeBoolean(hasTotalHits); - if (hasTotalHits) { - out.writeVLong(totalHits.value); - out.writeEnum(totalHits.relation); - } - } else { - if (hasTotalHits) { - out.writeVLong(totalHits.value); - if (totalHits.value > 0 && totalHits.relation != TotalHits.Relation.EQUAL_TO) { - throw new IllegalArgumentException("Cannot serialize approximate total hit counts to nodes that " + - "are on a version < 7.0.0"); - } - } else { - // for bwc we use 0 but this will be translated to -1 by the coordinating node - out.writeVLong(0); - } + out.writeEnum(totalHits.relation); + } else if (totalHits.value > 0 && totalHits.relation != TotalHits.Relation.EQUAL_TO) { + throw new IllegalArgumentException("Cannot serialize approximate total hit counts to nodes that are on a version < 7.0.0"); } } diff --git a/server/src/main/java/org/elasticsearch/search/SearchHits.java b/server/src/main/java/org/elasticsearch/search/SearchHits.java index 01f4e9f880e54..397091aa6f6a9 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchHits.java +++ b/server/src/main/java/org/elasticsearch/search/SearchHits.java @@ -27,6 +27,7 @@ import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Streamable; import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.lucene.Lucene; import org.elasticsearch.common.xcontent.ToXContentFragment; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; @@ -276,14 +277,7 @@ private static class Total implements Writeable, ToXContentFragment { final TotalHits in; Total(StreamInput in) throws IOException { - final long value = in.readVLong(); - final Relation relation; - if (in.getVersion().onOrAfter(Version.V_7_0_0)) { - relation = in.readEnum(Relation.class); - } else { - relation = Relation.EQUAL_TO; - } - this.in = new TotalHits(value, relation); + this.in = Lucene.readTotalHits(in); } Total(TotalHits in) { @@ -306,12 +300,7 @@ public int hashCode() { @Override public void writeTo(StreamOutput out) throws IOException { - out.writeVLong(in.value); - if (out.getVersion().onOrAfter(Version.V_7_0_0)) { - out.writeEnum(in.relation); - } else { - assert in.relation == Relation.EQUAL_TO; - } + Lucene.writeTotalHits(out, in); } @Override 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 d7258d81adcf7..3241c888b23e4 100644 --- a/server/src/main/java/org/elasticsearch/search/query/TopDocsCollectorContext.java +++ b/server/src/main/java/org/elasticsearch/search/query/TopDocsCollectorContext.java @@ -107,11 +107,11 @@ private EmptyTopDocsCollectorContext(IndexReader reader, Query query, this.hitCountSupplier = () -> new TotalHits(hitCountCollector.getTotalHits(), TotalHits.Relation.EQUAL_TO); } else { this.collector = new EarlyTerminatingCollector(hitCountCollector, 0, false); - this.hitCountSupplier = () -> new TotalHits(hitCount, TotalHits.Relation.EQUAL_TO); + this.hitCountSupplier = () -> new TotalHits(hitCount, TotalHits.Relation.GREATER_THAN_OR_EQUAL_TO); } } else { this.collector = new EarlyTerminatingCollector(new TotalHitCountCollector(), 0, false); - this.hitCountSupplier = () -> null; + this.hitCountSupplier = () -> new TotalHits(0, TotalHits.Relation.EQUAL_TO); } } @@ -222,7 +222,7 @@ private SimpleTopDocsCollectorContext(IndexReader reader, topDocsCollector = createCollector(sortAndFormats, numHits, searchAfter, 1); // don't compute hit counts via the collector topDocsSupplier = new CachedSupplier<>(topDocsCollector::topDocs); if (trackTotalHits == false) { - totalHitsSupplier = () -> null; + totalHitsSupplier = () -> new TotalHits(0, TotalHits.Relation.GREATER_THAN_OR_EQUAL_TO); } else { totalHitsSupplier = () -> new TotalHits(hitCount, TotalHits.Relation.EQUAL_TO); }