-
Notifications
You must be signed in to change notification settings - Fork 982
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
Add exponential growth to TimeLimitingBulkScorer #11984
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a question about the test but otherwise it looks great, I like that the test checks a long range of doc IDs without having to actually index docs.
expectedInterval = lastInterval; | ||
} | ||
// keep going or finish the test? | ||
return --runs == 0 ? DocIdSetIterator.NO_MORE_DOCS : 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would we end prematurely instead of doing the checks on the full range? Is it to avoid the corner case of the last range, which might be smaller than the expected interval? Maybe the bulk scorer could record the segment's maxDoc
and allow the difference to be less than expectedInterval
if the max
doc is equal to maxDoc
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Can you add a CHANGES entry under 9.5?
Done. |
Thanks @costin! |
Increase the timeout check inside TimeLimitBulkScorer at exponential rate. Fix #11676
Increase the timeout check inside TimeLimitBulkScorer at exponential rate.
Fix #11676