Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixup rare terms (backport of #64366) #64416

Merged
merged 3 commits into from
Oct 30, 2020
Merged

Conversation

nik9000
Copy link
Member

@nik9000 nik9000 commented Oct 30, 2020

In #64016 I caused a bug in rare_terms where we would rewrite the
current leaf and remove all hits from it, causing us to trip an
assertion. This didn't happen before because previously we never rewrote
the current leaf. The fix involves cleaning up the state that the
deferring collector uses if we end up removing all hits.

This requires some super intimate knowledge of how
BestBucketsDeferringCollector works so I decided to move the
implementation of that mergin from MergingBucketsDeferringCollector
into BestBucketsDeferringCollector. This means that
MergingBucketsDeferringCollector is pretty much empty now. So I've
deprecated it and will remove it entirely in an subsequent change.

Closes #64356

In elastic#64016 I caused a bug in rare_terms where we would rewrite the
current leaf and remove all hits from it, causing us to trip an
assertion. This didn't happen before because previously we never rewrote
the current leaf. The fix involves cleaning up the state that the
deferring collector uses if we end up removing all hits.

This requires some super intimate knowledge of how
`BestBucketsDeferringCollector` works so I decided to move the
implementation of that mergin from `MergingBucketsDeferringCollector`
into `BestBucketsDeferringCollector`. This means that
`MergingBucketsDeferringCollector` is pretty much empty now. So I've
deprecated it and will remove it entirely in an subsequent change.

Closes elastic#64356
@nik9000
Copy link
Member Author

nik9000 commented Oct 30, 2020

@elasticmachine update branch

@nik9000 nik9000 merged commit 1181b96 into elastic:7.x Oct 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants