Skip to content

Fixup rare terms (backport of #64366)#64416

Merged
nik9000 merged 3 commits intoelastic:7.xfrom
nik9000:fixup_rare_terms_7_x
Oct 30, 2020
Merged

Fixup rare terms (backport of #64366)#64416
nik9000 merged 3 commits intoelastic:7.xfrom
nik9000:fixup_rare_terms_7_x

Conversation

@nik9000
Copy link
Copy Markdown
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
Copy Markdown
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