benwtrent commented on code in PR #15952:
URL: https://github.com/apache/lucene/pull/15952#discussion_r3095313208


##########
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:
   1. 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...
   2. 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.
   
   I want to be very careful about adding an expert level API that Apache 
Lucene itself doesn't actually use.



-- 
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]

Reply via email to