Check if merge is aborted before executing file integrity checks#16264
Conversation
| * The merge that this state is associated with, or {@code null} if this merge state is not | ||
| * associated with an {@link IndexWriter} merge (e.g. for addIndexes). | ||
| */ | ||
| public final MergePolicy.OneMerge oneMerge; |
There was a problem hiding this comment.
oneMerge is public, do we really want to expose the raw OneMerge on MergeState's public API? checkAborted() method is all the codecs need, and that's already public..making the field package-private (or at least @lucene.internal) would avoid leaking merge internals to custom codec implementations!
There was a problem hiding this comment.
Thanks, that's a valid point.
I added the @lucene.internal annotation in db5d684 since org.apache.lucene.codecs.perfield.PerFieldMergeState sits in a different package and requires oneMerge to be public in order to propagate it when creating a restricted MergeState.
romseygeek
left a comment
romseygeek
left a comment
There was a problem hiding this comment.
Makes sense to me, and is a nice compromise for the moment.
Can you add a CHANGES entry for 10.5?
|
Thanks for the review @romseygeek ! I pushed 4ee6459 for the change log. |
…che#16264) Adds OneMerge to the MergeState so that merge writers and consumers can check the abort flag before starting each reader's checkIntegrity(). This avoids kicking off costly full-file checksums on segments when the merge has already been aborted.
3 participants
This pull request adds
OneMergeto theMergeStateso that merge writers and consumers can check the abort flag before starting each reader's checkIntegrity(). This avoids kicking off costly full-file checksums on segments when the merge has already been aborted.Relates #16086