Add PhraseQuery.Builder.setMaxTerms() method to limit the maximum number of terms and excessive memory use #15332
Conversation
… the problem of excessive memory usage in ultra-long text search case
…lve the problem of excessive memory usage in ultra-long text search case
…lve the problem of excessive memory usage in ultra-long text search case
|
This PR does not have an entry in lucene/CHANGES.txt. Consider adding one. If the PR doesn't need a changelog entry, then add the skip-changelog label to it and you will stop receiving this reminder on future updates to the PR. |
…lve the problem of excessive memory usage in ultra-long text search case
|
This PR does not have an entry in lucene/CHANGES.txt. Consider adding one. If the PR doesn't need a changelog entry, then add the skip-changelog label to it and you will stop receiving this reminder on future updates to the PR. |
| } | ||
| if (termThreshold > 0 && terms.size() >= termThreshold) { | ||
| throw new IllegalArgumentException( | ||
| "The current value of terms is " |
There was a problem hiding this comment.
| "The current value of terms is " | |
| "The current number of terms is " |
There was a problem hiding this comment.
Thanks for the suggestion, Dawid! 👍
|
|
||
| public void testPhraseQueryTermLimit() throws Exception { | ||
| PhraseQuery.Builder builder = new PhraseQuery.Builder(); | ||
| int termLimit = 1000; |
There was a problem hiding this comment.
Use a smaller termLimit (5) - it makes little sense to make the test run for 1000 terms. Also, I'd change the loops to not use positions 0..termLimit-1 and then check for failure on termLimit - seems more intuitive.
There was a problem hiding this comment.
Good points, @dweiss! I've made the following changes based on your feedback:
- Reduced
termLimitfrom 1000 to 5 to make the test more lightweight - Changed the position logic to use
0..termLimit-1and check for failure at positiontermLimit
This does make the test more intuitive. Thanks for the review!
There was a problem hiding this comment.
I'm sorry this comes in multiple requests. Could we also rename the issue and changes entry to make it clear to people as to what's changed? I'd suggest "Add PhraseQuery.Builder.setMaxTerms() method to limit the maximum number of terms and excessive memory use."
I would also rename the method accordingly and add javadocs to it that say what happens when you exceed the threshold (IllegalArgumentException is thrown). Thank you.
linyunanit
changed the title
…ber of terms and excessive memory use.
Thanks for the review, @dweiss! Great suggestions. I've made the following changes:
Looking forward to getting this merged! |
…ber of terms and excessive memory use (#15332) * perf: Added configurable limit for PhraseQuery#builder terms to solve the problem of excessive memory usage in ultra-long text search case * perf: Added configurable term threshold for PhraseQuery#Builder to solve the problem of excessive memory usage in ultra-long text search case * perf: Added configurable term threshold for PhraseQuery#Builder to solve the problem of excessive memory usage in ultra-long text search case * perf: Added configurable term threshold for PhraseQuery#Builder to solve the problem of excessive memory usage in ultra-long text search case * add changes entry * Add PhraseQuery.Builder.setMaxTerms() method to limit the maximum number of terms and excessive memory use. * Optimizing test cases：testPhraseQueryMaxTerms#testPhraseQueryTermLimit --------- Co-authored-by: nickyulin <nickyulin@tencent.com>
2 participants
Description
Add configurable term threshold for PhraseQuery#Builder to solve the problem of excessive memory usage in ultra-long text search case.
Key Changes