Fix filter by filter execution with doc_count #68930
Conversation
This fixed "filter by filter" execution order so it doesn't ignore `doc_count`. The "filter by filter" execution is fairly performance sensitive but when I reran performance numbers everything looked fine.
elasticmachine
added
the
Team:Analytics
|
Pinging @elastic/es-analytics-geo (Team:Analytics) |
| - match: { aggregations.f.buckets.foo.doc_count: 8 } | ||
| - match: { aggregations.f.buckets.xyz.doc_count: 5 } | ||
| - match: { profile.shards.0.aggregations.0.type: FiltersAggregator.FilterByFilter } | ||
| - gte: { profile.shards.0.aggregations.0.debug.segments_with_doc_count: 1 } |
There was a problem hiding this comment.
I figure it'll be nice to see this in the profile so if it is 0 we can know we didn't need to do anything fancy to get the doc count.
|
|
||
| @Override | ||
| public String toString() { | ||
| return "doc counts are " + (alwaysOne() ? "always one" : "based on " + docCountPostings); |
There was a problem hiding this comment.
I used this while debugging. We certainly don't need it but if folks don't object I'd like to keep it. I'm sure I'll bump into this class later on and this helps.
| Counter counter = new Counter(docCountProvider); | ||
| if (false == docCountProvider.alwaysOne()) { | ||
| segmentsWithDocCount++; | ||
| } |
There was a problem hiding this comment.
It's possible to use a different Counter implementation that always returns 1 when the provider will always return one. I trust the jvm to inline the calls on it regardless because counter is effectively final. It'd save a couple of comparisons in this very very hot section. But performance tests seemed fine so I didn't end up doing it.
|
Thanks @not-napoleon and @csoulios! I've merged and will backport. |
4 participants
This fixed "filter by filter" execution order so it doesn't ignore
doc_count. The "filter by filter" execution is fairly performancesensitive but when I reran performance numbers everything looked fine.