Skip to content

Commit 7d3c7bb

Browse files
thecoopthecoop
authored
Catch and re-throw Throwable rather than using a success boolean (#14633)
The use of a boolean success parameter is common in the Lucene codebase. This can be replaced with a catch (Throwable t) {...; throw t} pattern that means a boolean doesn't need to be used at all, and it generally results in less code. This is helped by some new methods on IOUtils. As a side-effect, any problems that occur whilst closing/deleting during exception handling will now be added to the top-level rethrown exception, rather than being swallowed and disappearing. I've converted a few here to show the pattern - if this is a direction we want to go in, I can convert more.
1 parent 9640dd8 commit 7d3c7bb

File tree

37 files changed

+465
-475
lines changed

37 files changed

+465
-475
lines changed

‎lucene/analysis/common/src/java/org/apache/lucene/analysis/hunspell/SortingStrategy.java‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -120,17 +120,13 @@ private String sortWordsOffline() throws IOException {
120120
var sorter = new OfflineSorter(tempDir, tempFileNamePrefix, BytesRefComparator.NATURAL);
121121

122122
String sorted;
123-
boolean success = false;
124123
try {
125124
sorted = sorter.sort(output.getName());
126-
success = true;
127-
} finally {
128-
if (success) {
129-
tempDir.deleteFile(output.getName());
130-
} else {
131-
IOUtils.deleteFilesIgnoringExceptions(tempDir, output.getName());
132-
}
125+
} catch (Throwable t) {
126+
IOUtils.deleteFilesSuppressingExceptions(t, tempDir, output.getName());
127+
throw t;
133128
}
129+
tempDir.deleteFile(output.getName());
134130
return sorted;
135131
}
136132
};

‎lucene/backward-codecs/src/java/org/apache/lucene/backward_codecs/lucene101/Lucene101PostingsFormat.java‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -389,15 +389,11 @@ public FieldsConsumer fieldsConsumer(SegmentWriteState state) throws IOException
389389
@Override
390390
public FieldsProducer fieldsProducer(SegmentReadState state) throws IOException {
391391
PostingsReaderBase postingsReader = new Lucene101PostingsReader(state);
392-
boolean success = false;
393392
try {
394-
FieldsProducer ret = new Lucene90BlockTreeTermsReader(postingsReader, state);
395-
success = true;
396-
return ret;
397-
} finally {
398-
if (!success) {
399-
IOUtils.closeWhileHandlingException(postingsReader);
400-
}
393+
return new Lucene90BlockTreeTermsReader(postingsReader, state);
394+
} catch (Throwable t) {
395+
IOUtils.closeWhileSuppressingExceptions(t, postingsReader);
396+
throw t;
401397
}
402398
}
403399

‎lucene/backward-codecs/src/java/org/apache/lucene/backward_codecs/lucene101/Lucene101PostingsReader.java‎

Lines changed: 34 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -90,53 +90,44 @@ public Lucene101PostingsReader(SegmentReadState state) throws IOException {
9090
IndexFileNames.segmentFileName(
9191
state.segmentInfo.name, state.segmentSuffix, Lucene101PostingsFormat.META_EXTENSION);
9292
final long expectedDocFileLength, expectedPosFileLength, expectedPayFileLength;
93-
ChecksumIndexInput metaIn = null;
94-
boolean success = false;
9593
int version;
96-
try {
97-
metaIn = state.directory.openChecksumInput(metaName);
98-
version =
99-
CodecUtil.checkIndexHeader(
100-
metaIn,
101-
META_CODEC,
102-
VERSION_START,
103-
VERSION_CURRENT,
104-
state.segmentInfo.getId(),
105-
state.segmentSuffix);
106-
maxNumImpactsAtLevel0 = metaIn.readInt();
107-
maxImpactNumBytesAtLevel0 = metaIn.readInt();
108-
maxNumImpactsAtLevel1 = metaIn.readInt();
109-
maxImpactNumBytesAtLevel1 = metaIn.readInt();
110-
expectedDocFileLength = metaIn.readLong();
111-
if (state.fieldInfos.hasProx()) {
112-
expectedPosFileLength = metaIn.readLong();
113-
if (state.fieldInfos.hasPayloads() || state.fieldInfos.hasOffsets()) {
114-
expectedPayFileLength = metaIn.readLong();
94+
try (ChecksumIndexInput metaIn = state.directory.openChecksumInput(metaName)) {
95+
try {
96+
version =
97+
CodecUtil.checkIndexHeader(
98+
metaIn,
99+
META_CODEC,
100+
VERSION_START,
101+
VERSION_CURRENT,
102+
state.segmentInfo.getId(),
103+
state.segmentSuffix);
104+
maxNumImpactsAtLevel0 = metaIn.readInt();
105+
maxImpactNumBytesAtLevel0 = metaIn.readInt();
106+
maxNumImpactsAtLevel1 = metaIn.readInt();
107+
maxImpactNumBytesAtLevel1 = metaIn.readInt();
108+
expectedDocFileLength = metaIn.readLong();
109+
if (state.fieldInfos.hasProx()) {
110+
expectedPosFileLength = metaIn.readLong();
111+
if (state.fieldInfos.hasPayloads() || state.fieldInfos.hasOffsets()) {
112+
expectedPayFileLength = metaIn.readLong();
113+
} else {
114+
expectedPayFileLength = -1;
115+
}
115116
} else {
117+
expectedPosFileLength = -1;
116118
expectedPayFileLength = -1;
117119
}
118-
} else {
119-
expectedPosFileLength = -1;
120-
expectedPayFileLength = -1;
121-
}
122-
CodecUtil.checkFooter(metaIn, null);
123-
success = true;
124-
} catch (Throwable t) {
125-
if (metaIn != null) {
126-
CodecUtil.checkFooter(metaIn, t);
127-
throw new AssertionError("unreachable");
128-
} else {
129-
throw t;
130-
}
131-
} finally {
132-
if (success) {
133-
metaIn.close();
134-
} else {
135-
IOUtils.closeWhileHandlingException(metaIn);
120+
CodecUtil.checkFooter(metaIn, null);
121+
} catch (Throwable t) {
122+
if (metaIn != null) {
123+
CodecUtil.checkFooter(metaIn, t);
124+
throw new AssertionError("unreachable");
125+
} else {
126+
throw t;
127+
}
136128
}
137129
}
138130

139-
success = false;
140131
IndexInput docIn = null;
141132
IndexInput posIn = null;
142133
IndexInput payIn = null;
@@ -182,11 +173,9 @@ public Lucene101PostingsReader(SegmentReadState state) throws IOException {
182173
this.docIn = docIn;
183174
this.posIn = posIn;
184175
this.payIn = payIn;
185-
success = true;
186-
} finally {
187-
if (!success) {
188-
IOUtils.closeWhileHandlingException(docIn, posIn, payIn);
189-
}
176+
} catch (Throwable t) {
177+
IOUtils.closeWhileSuppressingExceptions(t, docIn, posIn, payIn);
178+
throw t;
190179
}
191180
}
192181

‎lucene/backward-codecs/src/java/org/apache/lucene/backward_codecs/lucene50/compressing/FieldsIndexReader.java‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.apache.lucene.store.IOContext;
2828
import org.apache.lucene.store.IndexInput;
2929
import org.apache.lucene.store.RandomAccessInput;
30+
import org.apache.lucene.util.IOUtils;
3031

3132
final class FieldsIndexReader extends FieldsIndex {
3233

@@ -68,16 +69,13 @@ final class FieldsIndexReader extends FieldsIndex {
6869
indexInput =
6970
EndiannessReverserUtil.openInput(
7071
dir, IndexFileNames.segmentFileName(name, suffix, extension), IOContext.DEFAULT);
71-
boolean success = false;
7272
try {
7373
CodecUtil.checkIndexHeader(
7474
indexInput, codecName + "Idx", VERSION_START, VERSION_CURRENT, id, suffix);
7575
CodecUtil.retrieveChecksum(indexInput);
76-
success = true;
77-
} finally {
78-
if (success == false) {
79-
indexInput.close();
80-
}
76+
} catch (Throwable t) {
77+
IOUtils.closeWhileSuppressingExceptions(t, indexInput);
78+
throw t;
8179
}
8280
final RandomAccessInput docsSlice =
8381
indexInput.randomAccessSlice(docsStartPointer, docsEndPointer - docsStartPointer);

‎lucene/backward-codecs/src/java/org/apache/lucene/backward_codecs/lucene95/Lucene95HnswVectorsReader.java‎

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,6 @@ public final class Lucene95HnswVectorsReader extends KnnVectorsReader implements
7171
Lucene95HnswVectorsReader(SegmentReadState state) throws IOException {
7272
this.fieldInfos = state.fieldInfos;
7373
int versionMeta = readMetadata(state);
74-
boolean success = false;
7574
try {
7675
vectorData =
7776
openDataInput(
@@ -85,11 +84,9 @@ public final class Lucene95HnswVectorsReader extends KnnVectorsReader implements
8584
versionMeta,
8685
Lucene95HnswVectorsFormat.VECTOR_INDEX_EXTENSION,
8786
Lucene95HnswVectorsFormat.VECTOR_INDEX_CODEC_NAME);
88-
success = true;
89-
} finally {
90-
if (success == false) {
91-
IOUtils.closeWhileHandlingException(this);
92-
}
87+
} catch (Throwable t) {
88+
IOUtils.closeWhileSuppressingExceptions(t, this);
89+
throw t;
9390
}
9491
}
9592

@@ -125,7 +122,6 @@ private static IndexInput openDataInput(
125122
String fileName =
126123
IndexFileNames.segmentFileName(state.segmentInfo.name, state.segmentSuffix, fileExtension);
127124
IndexInput in = state.directory.openInput(fileName, state.context);
128-
boolean success = false;
129125
try {
130126
int versionVectorData =
131127
CodecUtil.checkIndexHeader(
@@ -146,12 +142,10 @@ private static IndexInput openDataInput(
146142
in);
147143
}
148144
CodecUtil.retrieveChecksum(in);
149-
success = true;
150145
return in;
151-
} finally {
152-
if (success == false) {
153-
IOUtils.closeWhileHandlingException(in);
154-
}
146+
} catch (Throwable t) {
147+
IOUtils.closeWhileSuppressingExceptions(t, in);
148+
throw t;
155149
}
156150
}
157151

‎lucene/backward-codecs/src/java/org/apache/lucene/backward_codecs/lucene99/Lucene99PostingsFormat.java‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -390,15 +390,11 @@ public FieldsConsumer fieldsConsumer(SegmentWriteState state) throws IOException
390390
@Override
391391
public FieldsProducer fieldsProducer(SegmentReadState state) throws IOException {
392392
PostingsReaderBase postingsReader = new Lucene99PostingsReader(state);
393-
boolean success = false;
394393
try {
395-
FieldsProducer ret = new Lucene90BlockTreeTermsReader(postingsReader, state);
396-
success = true;
397-
return ret;
398-
} finally {
399-
if (!success) {
400-
IOUtils.closeWhileHandlingException(postingsReader);
401-
}
394+
return new Lucene90BlockTreeTermsReader(postingsReader, state);
395+
} catch (Throwable t) {
396+
IOUtils.closeWhileSuppressingExceptions(t, postingsReader);
397+
throw t;
402398
}
403399
}
404400

‎lucene/backward-codecs/src/java/org/apache/lucene/backward_codecs/lucene99/Lucene99PostingsReader.java‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,6 @@ public final class Lucene99PostingsReader extends PostingsReaderBase {
6666

6767
/** Sole constructor. */
6868
public Lucene99PostingsReader(SegmentReadState state) throws IOException {
69-
boolean success = false;
7069
IndexInput docIn = null;
7170
IndexInput posIn = null;
7271
IndexInput payIn = null;
@@ -118,11 +117,9 @@ public Lucene99PostingsReader(SegmentReadState state) throws IOException {
118117
this.docIn = docIn;
119118
this.posIn = posIn;
120119
this.payIn = payIn;
121-
success = true;
122-
} finally {
123-
if (!success) {
124-
IOUtils.closeWhileHandlingException(docIn, posIn, payIn);
125-
}
120+
} catch (Throwable t) {
121+
IOUtils.closeWhileSuppressingExceptions(t, docIn, posIn, payIn);
122+
throw t;
126123
}
127124
}
128125

‎lucene/core/src/java/org/apache/lucene/codecs/lucene102/Lucene102BinaryQuantizedVectorsReader.java‎

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,6 @@ class Lucene102BinaryQuantizedVectorsReader extends FlatVectorsReader {
7878
state.segmentInfo.name,
7979
state.segmentSuffix,
8080
Lucene102BinaryQuantizedVectorsFormat.META_EXTENSION);
81-
boolean success = false;
8281
try (ChecksumIndexInput meta = state.directory.openChecksumInput(metaFileName)) {
8382
Throwable priorE = null;
8483
try {
@@ -106,11 +105,9 @@ class Lucene102BinaryQuantizedVectorsReader extends FlatVectorsReader {
106105
// graph.
107106
state.context.withHints(
108107
FileTypeHint.DATA, FileDataHint.KNN_VECTORS, DataAccessHint.RANDOM));
109-
success = true;
110-
} finally {
111-
if (success == false) {
112-
IOUtils.closeWhileHandlingException(this);
113-
}
108+
} catch (Throwable t) {
109+
IOUtils.closeWhileSuppressingExceptions(t, this);
110+
throw t;
114111
}
115112
}
116113

@@ -294,7 +291,6 @@ private static IndexInput openDataInput(
294291
String fileName =
295292
IndexFileNames.segmentFileName(state.segmentInfo.name, state.segmentSuffix, fileExtension);
296293
IndexInput in = state.directory.openInput(fileName, context);
297-
boolean success = false;
298294
try {
299295
int versionVectorData =
300296
CodecUtil.checkIndexHeader(
@@ -315,12 +311,10 @@ private static IndexInput openDataInput(
315311
in);
316312
}
317313
CodecUtil.retrieveChecksum(in);
318-
success = true;
319314
return in;
320-
} finally {
321-
if (success == false) {
322-
IOUtils.closeWhileHandlingException(in);
323-
}
315+
} catch (Throwable t) {
316+
IOUtils.closeWhileSuppressingExceptions(t, in);
317+
throw t;
324318
}
325319
}
326320

‎lucene/core/src/java/org/apache/lucene/codecs/lucene102/Lucene102BinaryQuantizedVectorsWriter.java‎

Lines changed: 24 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,6 @@ protected Lucene102BinaryQuantizedVectorsWriter(
9999
state.segmentSuffix,
100100
Lucene102BinaryQuantizedVectorsFormat.VECTOR_DATA_EXTENSION);
101101
this.rawVectorDelegate = rawVectorDelegate;
102-
boolean success = false;
103102
try {
104103
meta = state.directory.createOutput(metaFileName, state.context);
105104
binarizedVectorData =
@@ -117,11 +116,9 @@ protected Lucene102BinaryQuantizedVectorsWriter(
117116
Lucene102BinaryQuantizedVectorsFormat.VERSION_CURRENT,
118117
state.segmentInfo.getId(),
119118
state.segmentSuffix);
120-
success = true;
121-
} finally {
122-
if (success == false) {
123-
IOUtils.closeWhileHandlingException(this);
124-
}
119+
} catch (Throwable t) {
120+
IOUtils.closeWhileSuppressingExceptions(t, this);
121+
throw t;
125122
}
126123
}
127124

@@ -452,7 +449,6 @@ private CloseableRandomVectorScorerSupplier mergeOneFieldToIndex(
452449
IndexOutput tempScoreQuantizedVectorData = null;
453450
IndexInput binarizedDataInput = null;
454451
IndexInput binarizedScoreDataInput = null;
455-
boolean success = false;
456452
OptimizedScalarQuantizer quantizer =
457453
new OptimizedScalarQuantizer(fieldInfo.getVectorSimilarityFunction());
458454
try {
@@ -497,9 +493,14 @@ private CloseableRandomVectorScorerSupplier mergeOneFieldToIndex(
497493
centroid,
498494
cDotC,
499495
docsWithField);
500-
success = true;
496+
501497
final IndexInput finalBinarizedDataInput = binarizedDataInput;
502498
final IndexInput finalBinarizedScoreDataInput = binarizedScoreDataInput;
499+
tempQuantizedVectorData = null;
500+
tempScoreQuantizedVectorData = null;
501+
binarizedDataInput = null;
502+
binarizedScoreDataInput = null;
503+
503504
OffHeapBinarizedVectorValues vectorValues =
504505
new OffHeapBinarizedVectorValues.DenseOffHeapVectorValues(
505506
fieldInfo.getVectorDimension(),
@@ -526,22 +527,22 @@ private CloseableRandomVectorScorerSupplier mergeOneFieldToIndex(
526527
IOUtils.deleteFilesIgnoringExceptions(
527528
segmentWriteState.directory, tempQuantizedVectorName, tempScoreQuantizedVectorName);
528529
});
529-
} finally {
530-
if (success == false) {
531-
IOUtils.closeWhileHandlingException(
532-
tempQuantizedVectorData,
533-
tempScoreQuantizedVectorData,
534-
binarizedDataInput,
535-
binarizedScoreDataInput);
536-
if (tempQuantizedVectorData != null) {
537-
IOUtils.deleteFilesIgnoringExceptions(
538-
segmentWriteState.directory, tempQuantizedVectorData.getName());
539-
}
540-
if (tempScoreQuantizedVectorData != null) {
541-
IOUtils.deleteFilesIgnoringExceptions(
542-
segmentWriteState.directory, tempScoreQuantizedVectorData.getName());
543-
}
530+
} catch (Throwable t) {
531+
IOUtils.closeWhileSuppressingExceptions(
532+
t,
533+
tempQuantizedVectorData,
534+
tempScoreQuantizedVectorData,
535+
binarizedDataInput,
536+
binarizedScoreDataInput);
537+
if (tempQuantizedVectorData != null) {
538+
IOUtils.deleteFilesSuppressingExceptions(
539+
t, segmentWriteState.directory, tempQuantizedVectorData.getName());
540+
}
541+
if (tempScoreQuantizedVectorData != null) {
542+
IOUtils.deleteFilesSuppressingExceptions(
543+
t, segmentWriteState.directory, tempScoreQuantizedVectorData.getName());
544544
}
545+
throw t;
545546
}
546547
}
547548

0 commit comments

Comments
 (0)