jpountz edited a comment on pull request #446: URL: https://github.com/apache/lucene/pull/446#issuecomment-1054266698
> I also noticed that in IndexWriter where we call findFullFlushMerges, we only do so for merge triggers GET_READER and COMMIT, but not for trigger FULL_FLUSH, which seems quite confusing. I wonder if we could find a better name for findFullFlushMerges. I agree the naming makes it a bit confusing. One name that came to mind was `findPointInTimeMerges` since these two merge triggers map to merges that must run before creating a new point-in-time view of the index, while FULL_FLUSH runs after the new point-in-time view has been created. Clarifying ordering in the `MergeTrigger` javadocs would probably help too. > given that both findMerges and findFullFlushMerges are both called from the same switch statement, and for different triggers, and the trigger is passed in as an argument -- we could get rid of findFullFlushMerges, always call findMerges, and let the merge policy decide what to do based on the value of trigger. @s1monw WDTY? FWIW I don't dislike the current approach, as I would expect merge policies to generally ignore the `mergeTrigger` parameter as it makes sense to always make the same decisions for the triggers that are covered by `findFullFlushMerges` on the one hand, and by `findMerges` on the other hand, but it would be wrong to make the same decisions in `findFullFlushMerges` and `findMerges` as it would force reopens to wait for `maxFullFlushMergeWaitMillis` millis every time a non-trivial merge is computed? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
