MrFlap commented on code in PR #15952:
URL: https://github.com/apache/lucene/pull/15952#discussion_r3095438520
##########
lucene/core/src/java/org/apache/lucene/codecs/KnnVectorsWriter.java:
##########
@@ -96,18 +107,22 @@ public final void merge(MergeState mergeState) throws
IOException {
}
}
- for (FieldInfo fieldInfo : mergeState.mergeFieldInfos) {
- if (fieldInfo.hasVectorValues()) {
- if (mergeState.infoStream.isEnabled("VV")) {
- mergeState.infoStream.message("VV", "merging " +
mergeState.segmentInfo);
- }
+ try {
+ for (FieldInfo fieldInfo : mergeState.mergeFieldInfos) {
+ if (fieldInfo.hasVectorValues()) {
+ if (mergeState.infoStream.isEnabled("VV")) {
+ mergeState.infoStream.message("VV", "merging " +
mergeState.segmentInfo);
+ }
- mergeOneField(fieldInfo, mergeState);
+ mergeOneField(fieldInfo, mergeState);
- if (mergeState.infoStream.isEnabled("VV")) {
- mergeState.infoStream.message("VV", "merge done " +
mergeState.segmentInfo);
+ if (mergeState.infoStream.isEnabled("VV")) {
+ mergeState.infoStream.message("VV", "merge done " +
mergeState.segmentInfo);
+ }
}
}
+ } finally {
+ afterMerge();
}
finish();
Review Comment:
> On an exception with merge, all files, etc. should be closed, do we want
to move finish into the finally? There may be edge cases, for sure...
I'm not sure I can give a completely educated call on this, but it seems
like the right thing to do. If `mergeOneField` throws mid-way, the `finish()`
call will be skipped.
> Are we sure extending PerFieldKnnVectorsFormat is the way forward here? My
point is that this type of thing is doable via experts in Lucene in different
ways than adding another top-level API to the KnnVectorFormat which then
impacts all formats.
Understandable. I'm not a Lucene expert, but I don't think there is an easy
way for `PerFieldKnnVectorsFormat` to know when a merge completes. I'll
continue looking into this. cc @navneet1v
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
