From b46802cabf1fef377d203b937eeb441f30b3a7a5 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Thu, 26 Apr 2018 09:52:35 +0200 Subject: [PATCH 1/4] Build terms bucket from matching ordinals The global ordinals terms aggregator has an option to remap global ordinals to dense ordinal that match the request. This mode is automatically picked when the terms aggregator is a child of another bucket aggregator or when it needs to defer buckets to an aggregation that is used in the ordering of the terms. Though when building the final buckets, this aggregator loops over all possible global ordinals rather than using the hash map that was built to remap the ordinals. For fields with high cardinality this is highly inefficient and can lead to slow responses even when the number of terms that match the query is low. This change fixes this performance issue by using the hash table of matching ordinals to perform the pruning of the final buckets for the terms and significant_terms aggregation. I ran a simple benchmark with 1M documents containing 0 to 10 keywords randomly selected among 1M unique terms. This field is used to perform a multi-level terms aggregation using rally to collect the response times. The aggregation below is an example of a two-level terms aggregation that was used to perform the benchmark: ``` "aggregations":{ "1":{ "terms":{ "field":"keyword" }, "aggregations":{ "2":{ "terms":{ "field":"keyword" } } } } } ``` | Levels of aggregation | 50th percentile ms (master) | 50th percentile ms (patch) | | --- | --- | --- | | 2 | 640.41ms | 577.499ms | | 3 | 2239.66ms | 600.154ms | | 4 | 14141.2ms | 703.512ms | Closes #30117 --- ...balOrdinalsSignificantTermsAggregator.java | 19 ++++++++++--- .../GlobalOrdinalsStringTermsAggregator.java | 27 ++++++++++++------- 2 files changed, 32 insertions(+), 14 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/GlobalOrdinalsSignificantTermsAggregator.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/GlobalOrdinalsSignificantTermsAggregator.java index 66b8f8d5b15ed..a6ae8db6454e9 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/GlobalOrdinalsSignificantTermsAggregator.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/GlobalOrdinalsSignificantTermsAggregator.java @@ -103,11 +103,22 @@ public SignificantStringTerms buildAggregation(long owningBucketOrdinal) throws BucketSignificancePriorityQueue ordered = new BucketSignificancePriorityQueue<>(size); SignificantStringTerms.Bucket spare = null; - for (long globalTermOrd = 0; globalTermOrd < valueCount; ++globalTermOrd) { - if (includeExclude != null && !acceptedGlobalOrdinals.get(globalTermOrd)) { + boolean needsFullSan = bucketOrds == null || bucketCountThresholds.getMinDocCount() == 0; + long maxId = needsFullSan ? valueCount : bucketOrds.size(); + for (long ord = 0; ord < maxId; ord++) { + final long globalOrd; + final long bucketOrd; + if (needsFullSan) { + bucketOrd = bucketOrds == null ? ord : bucketOrds.find(ord); + globalOrd = ord; + } else { + assert bucketOrds != null; + bucketOrd = ord; + globalOrd = bucketOrds.get(ord); + } + if (includeExclude != null && !acceptedGlobalOrdinals.get(globalOrd)) { continue; } - final long bucketOrd = getBucketOrd(globalTermOrd); final int bucketDocCount = bucketOrd < 0 ? 0 : bucketDocCount(bucketOrd); if (bucketCountThresholds.getMinDocCount() > 0 && bucketDocCount == 0) { continue; @@ -120,7 +131,7 @@ public SignificantStringTerms buildAggregation(long owningBucketOrdinal) throws spare = new SignificantStringTerms.Bucket(new BytesRef(), 0, 0, 0, 0, null, format); } spare.bucketOrd = bucketOrd; - copy(lookupGlobalOrd.apply(globalTermOrd), spare.termBytes); + copy(lookupGlobalOrd.apply(globalOrd), spare.termBytes); spare.subsetDf = bucketDocCount; spare.subsetSize = subsetSize; spare.supersetDf = termsAggFactory.getBackgroundFrequency(spare.termBytes); diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/GlobalOrdinalsStringTermsAggregator.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/GlobalOrdinalsStringTermsAggregator.java index 6ad14b8d0f93a..a0ce52caab4f7 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/GlobalOrdinalsStringTermsAggregator.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/GlobalOrdinalsStringTermsAggregator.java @@ -71,7 +71,7 @@ public class GlobalOrdinalsStringTermsAggregator extends AbstractStringTermsAggr protected final long valueCount; protected final GlobalOrdLookupFunction lookupGlobalOrd; - private final LongHash bucketOrds; + protected final LongHash bucketOrds; public interface GlobalOrdLookupFunction { BytesRef apply(long ord) throws IOException; @@ -107,10 +107,6 @@ boolean remapGlobalOrds() { return bucketOrds != null; } - protected final long getBucketOrd(long globalOrd) { - return bucketOrds == null ? globalOrd : bucketOrds.find(globalOrd); - } - private void collectGlobalOrd(int doc, long globalOrd, LeafBucketCollector sub) throws IOException { if (bucketOrds == null) { collectExistingBucket(sub, doc, globalOrd); @@ -188,17 +184,28 @@ public InternalAggregation buildAggregation(long owningBucketOrdinal) throws IOE long otherDocCount = 0; BucketPriorityQueue ordered = new BucketPriorityQueue<>(size, order.comparator(this)); OrdBucket spare = new OrdBucket(-1, 0, null, showTermDocCountError, 0); - for (long globalTermOrd = 0; globalTermOrd < valueCount; ++globalTermOrd) { - if (includeExclude != null && !acceptedGlobalOrdinals.get(globalTermOrd)) { + boolean needsFullScan = bucketOrds == null || bucketCountThresholds.getMinDocCount() == 0; + long maxId = needsFullScan ? valueCount : bucketOrds.size(); + for (long ord = 0; ord < maxId; ord++) { + final long globalOrd; + final long bucketOrd; + if (needsFullScan) { + bucketOrd = bucketOrds == null ? ord : bucketOrds.find(ord); + globalOrd = ord; + } else { + assert bucketOrds != null; + bucketOrd = ord; + globalOrd = bucketOrds.get(ord); + } + if (includeExclude != null && !acceptedGlobalOrdinals.get(globalOrd)) { continue; } - final long bucketOrd = getBucketOrd(globalTermOrd); final int bucketDocCount = bucketOrd < 0 ? 0 : bucketDocCount(bucketOrd); if (bucketCountThresholds.getMinDocCount() > 0 && bucketDocCount == 0) { continue; } otherDocCount += bucketDocCount; - spare.globalOrd = globalTermOrd; + spare.globalOrd = globalOrd; spare.bucketOrd = bucketOrd; spare.docCount = bucketDocCount; if (bucketCountThresholds.getShardMinDocCount() <= spare.docCount) { @@ -378,7 +385,7 @@ private void mapSegmentCountsToGlobalCounts(LongUnaryOperator mapping) throws IO } final long ord = i - 1; // remember we do +1 when counting final long globalOrd = mapping.applyAsLong(ord); - long bucketOrd = getBucketOrd(globalOrd); + long bucketOrd = bucketOrds == null ? globalOrd : bucketOrds.find(globalOrd); incrementBucketDocCount(bucketOrd, inc); } } From 333e338def1ef52a1ba1ef603c6544460ef533fc Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Thu, 26 Apr 2018 09:52:49 +0200 Subject: [PATCH 2/4] unused import --- .../significant/GlobalOrdinalsSignificantTermsAggregator.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/GlobalOrdinalsSignificantTermsAggregator.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/GlobalOrdinalsSignificantTermsAggregator.java index a6ae8db6454e9..8deff19a5c6ab 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/GlobalOrdinalsSignificantTermsAggregator.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/GlobalOrdinalsSignificantTermsAggregator.java @@ -20,10 +20,8 @@ import org.apache.lucene.index.IndexReader; import org.apache.lucene.index.LeafReaderContext; -import org.apache.lucene.index.SortedSetDocValues; import org.apache.lucene.util.BytesRef; import org.elasticsearch.common.lease.Releasables; -import org.elasticsearch.common.util.LongHash; import org.elasticsearch.search.DocValueFormat; import org.elasticsearch.search.aggregations.Aggregator; import org.elasticsearch.search.aggregations.AggregatorFactories; From 5b6ed93f406b3f585aa24e2f643e4d2d12c7e002 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Thu, 26 Apr 2018 10:30:30 +0200 Subject: [PATCH 3/4] typos --- .../GlobalOrdinalsSignificantTermsAggregator.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/GlobalOrdinalsSignificantTermsAggregator.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/GlobalOrdinalsSignificantTermsAggregator.java index 8deff19a5c6ab..ff68b335962be 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/GlobalOrdinalsSignificantTermsAggregator.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/GlobalOrdinalsSignificantTermsAggregator.java @@ -101,12 +101,12 @@ public SignificantStringTerms buildAggregation(long owningBucketOrdinal) throws BucketSignificancePriorityQueue ordered = new BucketSignificancePriorityQueue<>(size); SignificantStringTerms.Bucket spare = null; - boolean needsFullSan = bucketOrds == null || bucketCountThresholds.getMinDocCount() == 0; - long maxId = needsFullSan ? valueCount : bucketOrds.size(); + boolean needsFullScan = bucketOrds == null || bucketCountThresholds.getMinDocCount() == 0; + long maxId = needsFullScan ? valueCount : bucketOrds.size(); for (long ord = 0; ord < maxId; ord++) { final long globalOrd; final long bucketOrd; - if (needsFullSan) { + if (needsFullScan) { bucketOrd = bucketOrds == null ? ord : bucketOrds.find(ord); globalOrd = ord; } else { From 69c81601065915996afccb5efe966dde44a83b48 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Thu, 26 Apr 2018 14:08:26 +0200 Subject: [PATCH 4/4] address review --- .../significant/GlobalOrdinalsSignificantTermsAggregator.java | 4 ++-- .../bucket/terms/GlobalOrdinalsStringTermsAggregator.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/GlobalOrdinalsSignificantTermsAggregator.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/GlobalOrdinalsSignificantTermsAggregator.java index ff68b335962be..25f83caa3eb92 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/GlobalOrdinalsSignificantTermsAggregator.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/GlobalOrdinalsSignificantTermsAggregator.java @@ -101,8 +101,8 @@ public SignificantStringTerms buildAggregation(long owningBucketOrdinal) throws BucketSignificancePriorityQueue ordered = new BucketSignificancePriorityQueue<>(size); SignificantStringTerms.Bucket spare = null; - boolean needsFullScan = bucketOrds == null || bucketCountThresholds.getMinDocCount() == 0; - long maxId = needsFullScan ? valueCount : bucketOrds.size(); + final boolean needsFullScan = bucketOrds == null || bucketCountThresholds.getMinDocCount() == 0; + final long maxId = needsFullScan ? valueCount : bucketOrds.size(); for (long ord = 0; ord < maxId; ord++) { final long globalOrd; final long bucketOrd; diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/GlobalOrdinalsStringTermsAggregator.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/GlobalOrdinalsStringTermsAggregator.java index a0ce52caab4f7..03eb00337e9c1 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/GlobalOrdinalsStringTermsAggregator.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/GlobalOrdinalsStringTermsAggregator.java @@ -184,8 +184,8 @@ public InternalAggregation buildAggregation(long owningBucketOrdinal) throws IOE long otherDocCount = 0; BucketPriorityQueue ordered = new BucketPriorityQueue<>(size, order.comparator(this)); OrdBucket spare = new OrdBucket(-1, 0, null, showTermDocCountError, 0); - boolean needsFullScan = bucketOrds == null || bucketCountThresholds.getMinDocCount() == 0; - long maxId = needsFullScan ? valueCount : bucketOrds.size(); + final boolean needsFullScan = bucketOrds == null || bucketCountThresholds.getMinDocCount() == 0; + final long maxId = needsFullScan ? valueCount : bucketOrds.size(); for (long ord = 0; ord < maxId; ord++) { final long globalOrd; final long bucketOrd;