Skip to content

Commit e565840

Browse files
committed
Use global_ordinals_hash execution mode when sorting by sub aggregations. (#26014)
This is a safer default since sorting by sub aggregations prevents these aggregations from being deferred. `global_ordinals_hash` will at least make sure that we do not use memory for buckets that are not collected. Closes #24359
1 parent b0877db commit e565840

File tree

1 file changed

+20
-1
lines changed

1 file changed

+20
-1
lines changed

core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregatorFactory.java

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import org.elasticsearch.search.aggregations.BucketOrder;
3131
import org.elasticsearch.search.aggregations.InternalAggregation;
3232
import org.elasticsearch.search.aggregations.InternalOrder;
33+
import org.elasticsearch.search.aggregations.InternalOrder.CompoundOrder;
3334
import org.elasticsearch.search.aggregations.NonCollectingAggregator;
3435
import org.elasticsearch.search.aggregations.bucket.BucketUtils;
3536
import org.elasticsearch.search.aggregations.bucket.terms.TermsAggregator.BucketCountThresholds;
@@ -93,6 +94,17 @@ public InternalAggregation buildEmptyAggregation() {
9394
};
9495
}
9596

97+
private static boolean isAggregationSort(BucketOrder order) {
98+
if (order instanceof InternalOrder.Aggregation) {
99+
return true;
100+
} else if (order instanceof InternalOrder.CompoundOrder) {
101+
InternalOrder.CompoundOrder compoundOrder = (CompoundOrder) order;
102+
return compoundOrder.orderElements().stream().anyMatch(TermsAggregatorFactory::isAggregationSort);
103+
} else {
104+
return false;
105+
}
106+
}
107+
96108
@Override
97109
protected Aggregator doCreateInternal(ValuesSource valuesSource, Aggregator parent, boolean collectsFromSingleBucket,
98110
List<PipelineAggregator> pipelineAggregators, Map<String, Object> metaData) throws IOException {
@@ -139,10 +151,17 @@ protected Aggregator doCreateInternal(ValuesSource valuesSource, Aggregator pare
139151
// to be unbounded and most instances may only aggregate few
140152
// documents, so use hashed based
141153
// global ordinals to keep the bucket ords dense.
154+
142155
// Additionally, if using partitioned terms the regular global
143156
// ordinals would be sparse so we opt for hash
157+
158+
// Finally if we are sorting by sub aggregations, then these
159+
// aggregations cannot be deferred, so global_ordinals_hash is
160+
// a safer choice as we won't use memory for sub aggregations
161+
// for buckets that are not collected.
144162
if (Aggregator.descendsFromBucketAggregator(parent) ||
145-
(includeExclude != null && includeExclude.isPartitionBased())) {
163+
(includeExclude != null && includeExclude.isPartitionBased()) ||
164+
isAggregationSort(order)) {
146165
execution = ExecutionMode.GLOBAL_ORDINALS_HASH;
147166
} else {
148167
if (factories == AggregatorFactories.EMPTY) {

0 commit comments

Comments
 (0)