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:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
