Add support for dynamic pruning to cardinality aggregations on low-cardinality keyword fields. #92060
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…rdinality keyword fields. On low-cardinality keyword fields, the `cardinality` aggregation currently uses the `global_ordinals` execution mode most of the time, which consists of collecting all documents that match the query, reading ordinals of the values that these documents contain, and setting bits in a bitset for these ordinals. This commit introduces a feedback loop between the query and the `cardinality` aggregator, which allows the query to skip documents that only contain values that have already been seen by the `cardinality` aggregator. On the `nyc_taxis` dataset, a `match_all` query and the `vendor_id` field (2 unique values), the `cardinality` aggregation went from 3s to 3ms. The speedup would certainly not be as good in all cases, but I would still expect in to be very significant in many cases.
|
Hi @jpountz, I've created a changelog YAML for you. |
elasticsearchmachine
added
the
Team:Analytics
|
Pinging @elastic/es-analytics-geo (Team:Analytics) |
|
I've run some benchmarks on a synthetically generated index that has 100M documents with the following values for the i-th indexed document:
Then the following queries:
{
"size": 0,
"track_total_hits": false,
"aggs": {
"k2_cardinality": {
"cardinality": {
"field": "k2",
"execution_hint": "global_ordinals"
}
}
}
}
{
"size": 0,
"track_total_hits": false,
"query": {
"term": {
"k1": "0"
}
},
"aggs": {
"k2_cardinality": {
"cardinality": {
"field": "k2",
"execution_hint": "global_ordinals"
}
}
}
}Here is what Rally reported: The query on I also pushed some more tests that make sure that the optimization kicks in by using the debug info, so I think it's ready for someone to have a closer look at how it works. |
|
@elasticmachine update branch |
not-napoleon
approved these changes
Apr 18, 2023
| // we'd be paying the overhead of dynamic pruning without getting any benefits. | ||
| private static final int MAX_FIELD_CARDINALITY_FOR_DYNAMIC_PRUNING = 1024; | ||
|
|
||
| // Only start dynamic pruning when 128 ordinals or less have not been seen yet. |
There was a problem hiding this comment.
I'm confused by this comment. Do we prune when we have less than 128 or more than 128 ordinals?
There was a problem hiding this comment.
The way I understand this, is that if we have less then or equal to 128 unseen ordinals then we prune.
|
@elasticmachine update branch |
martijnvg
approved these changes
May 1, 2023
| // we'd be paying the overhead of dynamic pruning without getting any benefits. | ||
| private static final int MAX_FIELD_CARDINALITY_FOR_DYNAMIC_PRUNING = 1024; | ||
|
|
||
| // Only start dynamic pruning when 128 ordinals or less have not been seen yet. |
There was a problem hiding this comment.
The way I understand this, is that if we have less then or equal to 128 unseen ordinals then we prune.
| void startPruning() throws IOException { | ||
| dynamicPruningSuccess++; | ||
| nonVisitedOrds = new HashMap<>(); | ||
| // TODO: iterate the bitset using a `nextClearBit` operation? |
There was a problem hiding this comment.
Letting this loop be lead by nextClearBit() is a good idea 👍
I think we can add that method to bitset and make use of that in a follow up change.
| noData++; | ||
| return LeafBucketCollector.NO_OP_COLLECTOR; | ||
| } | ||
| // Otherwise we might be aggregating e.g. an IP field, which indexes data using points rather than an inverted index. |
There was a problem hiding this comment.
Maybe in a follow up change, we can also do the same trick here with points?
| final CompetitiveIterator competitiveIterator; | ||
|
|
||
| { | ||
| // This optimization only works for top-level cardinality aggregations that collect bucket 0, so we can retrieve |
There was a problem hiding this comment.
If this is the only current limitation then I think we can get around this by creating CompetitiveIterator instance lazily? We would then need a CompetitiveIterator per bucket ordinal. Not sure if this can work. But could be explored in a follow up PR.
| this.docsWithField = docsWithField; | ||
| } | ||
|
|
||
| private Map<Long, PostingsEnum> nonVisitedOrds; |
There was a problem hiding this comment.
Given that this optimisation is limited in its use, only if on fields with <= 1024 terms, the need to use LongObjectPagedHashMap isn't needed here.
| if (indexTerms != null) { | ||
| BitArray bits = visitedOrds.get(0); | ||
| final int numNonVisitedOrds = maxOrd - (bits == null ? 0 : (int) bits.cardinality()); | ||
| if (maxOrd <= MAX_FIELD_CARDINALITY_FOR_DYNAMIC_PRUNING || numNonVisitedOrds <= MAX_TERMS_FOR_DYNAMIC_PRUNING) { |
There was a problem hiding this comment.
So the optimisation also kicks in on fields with more than 1024 unique values, if there 128 or less terms to be processed. The brute force leaf bucket collector implementation does update the bit sets used here to determine numNonVisitedOrds.
|
This improvement also yielded a significant speedup for another benchmark that is under development. This search request counts the number of unique deployments using the This search request has a top level |
7 tasks
kkrik-es
added a commit
to kkrik-es/elasticsearch
that referenced
this pull request
Sep 22, 2023
const_keyword fields don't show up in the leafReader, since they have a const value. elastic#92060 modified the logic to return no results in case the leaf reader contains no information about the requested field in a cardinality aggregation. This is wrong for const_keyword fields, as they contain up to 1 distinct value. To fix this, we fall back to the old logic in this case that can handle const_keyword fields properly. Fixes elastic#99776
kkrik-es
added a commit
that referenced
this pull request
Sep 25, 2023
* Fix cardinality agg for const_keyword const_keyword fields don't show up in the leafReader, since they have a const value. #92060 modified the logic to return no results in case the leaf reader contains no information about the requested field in a cardinality aggregation. This is wrong for const_keyword fields, as they contain up to 1 distinct value. To fix this, we fall back to the old logic in this case that can handle const_keyword fields properly. Fixes #99776 * Update docs/changelog/99814.yaml * Update skip ranges for broken releases.
kkrik-es
added a commit
to kkrik-es/elasticsearch
that referenced
this pull request
Sep 25, 2023
* Fix cardinality agg for const_keyword const_keyword fields don't show up in the leafReader, since they have a const value. elastic#92060 modified the logic to return no results in case the leaf reader contains no information about the requested field in a cardinality aggregation. This is wrong for const_keyword fields, as they contain up to 1 distinct value. To fix this, we fall back to the old logic in this case that can handle const_keyword fields properly. Fixes elastic#99776 * Update docs/changelog/99814.yaml * Update skip ranges for broken releases.
elasticsearchmachine
pushed a commit
that referenced
this pull request
Sep 25, 2023
* Fix cardinality agg for const_keyword const_keyword fields don't show up in the leafReader, since they have a const value. #92060 modified the logic to return no results in case the leaf reader contains no information about the requested field in a cardinality aggregation. This is wrong for const_keyword fields, as they contain up to 1 distinct value. To fix this, we fall back to the old logic in this case that can handle const_keyword fields properly. Fixes #99776 * Update docs/changelog/99814.yaml * Update skip ranges for broken releases.
floragunncom
pushed a commit
to floragunncom/search-guard
that referenced
this pull request
Mar 3, 2024
floragunncom
pushed a commit
to floragunncom/search-guard
that referenced
this pull request
Mar 3, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
On low-cardinality keyword fields, the
cardinalityaggregation currently uses theglobal_ordinalsexecution mode most of the time, which consists of collecting all documents that match the query, reading ordinals of the values that these documents contain, and setting bits in a bitset for these ordinals.This commit introduces a feedback loop between the query and the
cardinalityaggregator, which allows the query to skip documents that only contain values that have already been seen by thecardinalityaggregator. On thenyc_taxisdataset, amatch_allquery and thevendor_idfield (2 unique values), thecardinalityaggregation went from 3s to 3ms. The speedup would certainly not be as good in all cases, but I would still expect it to be very significant in many cases.