[GitHub] [lucene-solr] msokolov commented on a change in pull request #2404: LUCENE-9639: Add unit tests for SimpleTextVector format

[GitBox <gi...@apache.org>]

msokolov commented on a change in pull request #2404:
URL: https://github.com/apache/lucene-solr/pull/2404#discussion_r580512737



##########
File path: lucene/core/src/java/org/apache/lucene/codecs/lucene90/Lucene90VectorReader.java
##########
@@ -120,6 +120,7 @@ private static IndexInput openDataInput(
 
     String fileName =
         IndexFileNames.segmentFileName(state.segmentInfo.name, state.segmentSuffix, fileExtension);
+    // nocommit should the openInput call be put in try-finally as well for IOException handling?

Review comment:
       I think it's OK - the exception handling got moved to the outer scope in the PR I referenced above

##########
File path: lucene/codecs/src/java/org/apache/lucene/codecs/simpletext/SimpleTextVectorReader.java
##########
@@ -93,22 +94,35 @@
             readState.segmentInfo.name,
             readState.segmentSuffix,
             SimpleTextVectorFormat.VECTOR_EXTENSION);
-    dataIn = readState.directory.openInput(vectorFileName, IOContext.DEFAULT);
+
+    boolean success = false;

Review comment:
       I'm concerned this might lead to test fails (and theoretically production issues) caused by exceptions raised during this constructor leading to an inconsistent state. See https://github.com/apache/lucene-solr/commit/4cdfbbb95be1b25adb839ee8d1fe61052a53a4a3 where opening the two index files were both pushed under the same exception handler.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org