From 8e9dc954ec96db016e3acdee724b719177ba0538 Mon Sep 17 00:00:00 2001 From: Simon Cooper Date: Wed, 20 Aug 2025 16:47:05 +0100 Subject: [PATCH 1/2] Remove more boolean success flags --- .../analysis/hunspell/SortingStrategy.java | 9 +- .../index/DocumentsWriterFlushQueue.java | 9 +- .../lucene/index/SegmentCoreReaders.java | 124 ++++++++-------- .../lucene/store/NRTCachingDirectory.java | 18 +-- .../lucene/facet/taxonomy/TaxonomyReader.java | 11 +- .../directory/DirectoryTaxonomyReader.java | 9 +- ...tAlwaysRefreshDirectoryTaxonomyReader.java | 14 +- .../replicator/nrt/TestSimpleServer.java | 17 +-- .../search/spell/PlainTextDictionary.java | 9 +- .../search/suggest/SortedInputIterator.java | 17 +-- .../analyzing/AnalyzingInfixSuggester.java | 23 ++- .../suggest/analyzing/FreeTextSuggester.java | 140 ++++++++---------- .../document/CompletionFieldsConsumer.java | 9 +- .../document/CompletionFieldsProducer.java | 20 +-- .../search/suggest/fst/ExternalRefSorter.java | 20 +-- 15 files changed, 181 insertions(+), 268 deletions(-) diff --git a/lucene/analysis/common/src/java/org/apache/lucene/analysis/hunspell/SortingStrategy.java b/lucene/analysis/common/src/java/org/apache/lucene/analysis/hunspell/SortingStrategy.java index acf8a534d3de..b1e220fe6bc0 100644 --- a/lucene/analysis/common/src/java/org/apache/lucene/analysis/hunspell/SortingStrategy.java +++ b/lucene/analysis/common/src/java/org/apache/lucene/analysis/hunspell/SortingStrategy.java @@ -87,8 +87,6 @@ public EntrySupplier finishAndSort() throws IOException { ByteSequencesReader reader = new ByteSequencesReader(tempDir.openChecksumInput(sortedFile), sortedFile); return new EntrySupplier() { - boolean success = false; - @Override public int wordCount() { return wordCount; @@ -98,7 +96,6 @@ public int wordCount() { public String next() throws IOException { BytesRef scratch = reader.next(); if (scratch == null) { - success = true; return null; } return scratch.utf8ToString(); @@ -107,11 +104,7 @@ public String next() throws IOException { @Override public void close() throws IOException { reader.close(); - if (success) { - tempDir.deleteFile(sortedFile); - } else { - IOUtils.deleteFilesIgnoringExceptions(tempDir, sortedFile); - } + tempDir.deleteFile(sortedFile); } }; } diff --git a/lucene/core/src/java/org/apache/lucene/index/DocumentsWriterFlushQueue.java b/lucene/core/src/java/org/apache/lucene/index/DocumentsWriterFlushQueue.java index d71c1771c2cd..68a5c20fe719 100644 --- a/lucene/core/src/java/org/apache/lucene/index/DocumentsWriterFlushQueue.java +++ b/lucene/core/src/java/org/apache/lucene/index/DocumentsWriterFlushQueue.java @@ -38,19 +38,16 @@ final class DocumentsWriterFlushQueue { synchronized FlushTicket addTicket(Supplier ticketSupplier) throws IOException { // first inc the ticket count - freeze opens a window for #anyChanges to fail incTickets(); - boolean success = false; try { FlushTicket ticket = ticketSupplier.get(); if (ticket != null) { // no need to publish anything if we don't have any frozen updates queue.add(ticket); - success = true; } return ticket; - } finally { - if (!success) { - decTickets(); - } + } catch (Throwable t) { + decTickets(); + throw t; } } diff --git a/lucene/core/src/java/org/apache/lucene/index/SegmentCoreReaders.java b/lucene/core/src/java/org/apache/lucene/index/SegmentCoreReaders.java index c11433f6635d..a9c2657e8ed7 100644 --- a/lucene/core/src/java/org/apache/lucene/index/SegmentCoreReaders.java +++ b/lucene/core/src/java/org/apache/lucene/index/SegmentCoreReaders.java @@ -74,80 +74,78 @@ final class SegmentCoreReaders { SegmentCoreReaders(Directory dir, SegmentCommitInfo si, IOContext context) throws IOException { final Codec codec = si.info.getCodec(); - final Directory - cfsDir; // confusing name: if (cfs) it's the cfsdir, otherwise it's the segment's directory. - boolean success = false; + // confusing name: if (cfs) it's the cfsdir, otherwise it's the segment's directory: + final Directory cfsDir; try { - if (si.info.getUseCompoundFile()) { - cfsDir = cfsReader = codec.compoundFormat().getCompoundReader(dir, si.info); - } else { - cfsReader = null; - cfsDir = dir; - } + try { + if (si.info.getUseCompoundFile()) { + cfsDir = cfsReader = codec.compoundFormat().getCompoundReader(dir, si.info); + } else { + cfsReader = null; + cfsDir = dir; + } - segment = si.info.name; + segment = si.info.name; - coreFieldInfos = codec.fieldInfosFormat().read(cfsDir, si.info, "", context); + coreFieldInfos = codec.fieldInfosFormat().read(cfsDir, si.info, "", context); - final SegmentReadState segmentReadState = - new SegmentReadState(cfsDir, si.info, coreFieldInfos, context); - if (coreFieldInfos.hasPostings()) { - final PostingsFormat format = codec.postingsFormat(); - // Ask codec for its Fields - fields = format.fieldsProducer(segmentReadState); - assert fields != null; - } else { - fields = null; - } - // ask codec for its Norms: - // TODO: since we don't write any norms file if there are no norms, - // kinda jaky to assume the codec handles the case of no norms file at all gracefully?! - - if (coreFieldInfos.hasNorms()) { - normsProducer = codec.normsFormat().normsProducer(segmentReadState); - assert normsProducer != null; - } else { - normsProducer = null; - } - - fieldsReaderOrig = - si.info - .getCodec() - .storedFieldsFormat() - .fieldsReader(cfsDir, si.info, coreFieldInfos, context); + final SegmentReadState segmentReadState = + new SegmentReadState(cfsDir, si.info, coreFieldInfos, context); + if (coreFieldInfos.hasPostings()) { + final PostingsFormat format = codec.postingsFormat(); + // Ask codec for its Fields + fields = format.fieldsProducer(segmentReadState); + assert fields != null; + } else { + fields = null; + } + // ask codec for its Norms: + // TODO: since we don't write any norms file if there are no norms, + // kinda jaky to assume the codec handles the case of no norms file at all gracefully?! + + if (coreFieldInfos.hasNorms()) { + normsProducer = codec.normsFormat().normsProducer(segmentReadState); + assert normsProducer != null; + } else { + normsProducer = null; + } - if (coreFieldInfos.hasTermVectors()) { // open term vector files only as needed - termVectorsReaderOrig = + fieldsReaderOrig = si.info .getCodec() - .termVectorsFormat() - .vectorsReader(cfsDir, si.info, coreFieldInfos, context); - } else { - termVectorsReaderOrig = null; - } - - if (coreFieldInfos.hasPointValues()) { - pointsReader = codec.pointsFormat().fieldsReader(segmentReadState); - } else { - pointsReader = null; - } + .storedFieldsFormat() + .fieldsReader(cfsDir, si.info, coreFieldInfos, context); + + if (coreFieldInfos.hasTermVectors()) { // open term vector files only as needed + termVectorsReaderOrig = + si.info + .getCodec() + .termVectorsFormat() + .vectorsReader(cfsDir, si.info, coreFieldInfos, context); + } else { + termVectorsReaderOrig = null; + } - if (coreFieldInfos.hasVectorValues()) { - knnVectorsReader = codec.knnVectorsFormat().fieldsReader(segmentReadState); - } else { - knnVectorsReader = null; - } + if (coreFieldInfos.hasPointValues()) { + pointsReader = codec.pointsFormat().fieldsReader(segmentReadState); + } else { + pointsReader = null; + } - success = true; - } catch (EOFException | FileNotFoundException e) { - throw new CorruptIndexException("Problem reading index from " + dir, dir.toString(), e); - } catch (NoSuchFileException e) { - throw new CorruptIndexException("Problem reading index.", e.getFile(), e); - } finally { - if (!success) { - decRef(); + if (coreFieldInfos.hasVectorValues()) { + knnVectorsReader = codec.knnVectorsFormat().fieldsReader(segmentReadState); + } else { + knnVectorsReader = null; + } + } catch (EOFException | FileNotFoundException e) { + throw new CorruptIndexException("Problem reading index from " + dir, dir.toString(), e); + } catch (NoSuchFileException e) { + throw new CorruptIndexException("Problem reading index.", e.getFile(), e); } + } catch (Throwable t) { + decRef(); + throw t; } } diff --git a/lucene/core/src/java/org/apache/lucene/store/NRTCachingDirectory.java b/lucene/core/src/java/org/apache/lucene/store/NRTCachingDirectory.java index 35a300c1763a..e0cb4649e424 100644 --- a/lucene/core/src/java/org/apache/lucene/store/NRTCachingDirectory.java +++ b/lucene/core/src/java/org/apache/lucene/store/NRTCachingDirectory.java @@ -16,6 +16,7 @@ */ package org.apache.lucene.store; +import java.io.Closeable; import java.io.FileNotFoundException; import java.io.IOException; import java.io.UncheckedIOException; @@ -249,11 +250,9 @@ public IndexOutput createTempOutput(String prefix, String suffix, IOContext cont if (VERBOSE) { System.out.println("nrtdir.createTempOutput prefix=" + prefix + " suffix=" + suffix); } - Set toDelete = new HashSet<>(); - // This is very ugly/messy/dangerous (can in some disastrous case maybe create too many temp // files), but I don't know of a cleaner way: - boolean success = false; + Set toDelete = new HashSet<>(); Directory first; Directory second; @@ -266,7 +265,7 @@ public IndexOutput createTempOutput(String prefix, String suffix, IOContext cont } IndexOutput out = null; - try { + try (Closeable _ = () -> IOUtils.deleteFiles(first, toDelete)) { while (true) { out = first.createTempOutput(prefix, suffix, context); String name = out.getName(); @@ -275,17 +274,12 @@ public IndexOutput createTempOutput(String prefix, String suffix, IOContext cont out.close(); } else { toDelete.remove(name); - success = true; break; } } - } finally { - if (success) { - IOUtils.deleteFiles(first, toDelete); - } else { - IOUtils.closeWhileHandlingException(out); - IOUtils.deleteFilesIgnoringExceptions(first, toDelete); - } + } catch (Throwable t) { + IOUtils.closeWhileSuppressingExceptions(t, out); + throw t; } return out; diff --git a/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/TaxonomyReader.java b/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/TaxonomyReader.java index 7ef1c53d9bb5..e08afbfe91bb 100644 --- a/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/TaxonomyReader.java +++ b/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/TaxonomyReader.java @@ -155,16 +155,13 @@ public final void decRef() throws IOException { ensureOpen(); final int rc = refCount.decrementAndGet(); if (rc == 0) { - boolean success = false; try { doClose(); closed = true; - success = true; - } finally { - if (!success) { - // Put reference back on failure - refCount.incrementAndGet(); - } + } catch (Throwable t) { + // Put reference back on failure + refCount.incrementAndGet(); + throw t; } } else if (rc < 0) { throw new IllegalStateException( diff --git a/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/directory/DirectoryTaxonomyReader.java b/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/directory/DirectoryTaxonomyReader.java index 418fcceca14b..ad5a416167c7 100644 --- a/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/directory/DirectoryTaxonomyReader.java +++ b/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/directory/DirectoryTaxonomyReader.java @@ -188,7 +188,6 @@ protected DirectoryTaxonomyReader doOpenIfChanged() throws IOException { } // check if the taxonomy was recreated - boolean success = false; try { boolean recreated = false; if (taxoWriter == null) { @@ -222,12 +221,10 @@ protected DirectoryTaxonomyReader doOpenIfChanged() throws IOException { new DirectoryTaxonomyReader(r2, taxoWriter, ordinalCache, categoryCache, taxoArrays); } - success = true; return newTaxoReader; - } finally { - if (!success) { - IOUtils.closeWhileHandlingException(r2); - } + } catch (Throwable t) { + IOUtils.closeWhileSuppressingExceptions(t, r2); + throw t; } } diff --git a/lucene/facet/src/test/org/apache/lucene/facet/taxonomy/directory/TestAlwaysRefreshDirectoryTaxonomyReader.java b/lucene/facet/src/test/org/apache/lucene/facet/taxonomy/directory/TestAlwaysRefreshDirectoryTaxonomyReader.java index 3b683dc1c6a4..599813fcf139 100644 --- a/lucene/facet/src/test/org/apache/lucene/facet/taxonomy/directory/TestAlwaysRefreshDirectoryTaxonomyReader.java +++ b/lucene/facet/src/test/org/apache/lucene/facet/taxonomy/directory/TestAlwaysRefreshDirectoryTaxonomyReader.java @@ -167,8 +167,6 @@ private class AlwaysRefreshDirectoryTaxonomyReader extends DirectoryTaxonomyRead @Override protected DirectoryTaxonomyReader doOpenIfChanged() throws IOException { - boolean success = false; - // the getInternalIndexReader() function performs the ensureOpen() check final DirectoryReader reader = DirectoryReader.openIfChanged(super.getInternalIndexReader()); if (reader == null) { @@ -181,14 +179,10 @@ protected DirectoryTaxonomyReader doOpenIfChanged() throws IOException { // Returning a AlwaysRefreshDirectoryTaxonomyReader ensures that the recreated taxonomy // reader also uses the overridden doOpenIfChanged // method (that always recomputes values). - final AlwaysRefreshDirectoryTaxonomyReader newTaxonomyReader = - new AlwaysRefreshDirectoryTaxonomyReader(reader); - success = true; - return newTaxonomyReader; - } finally { - if (!success) { - IOUtils.closeWhileHandlingException(reader); - } + return new AlwaysRefreshDirectoryTaxonomyReader(reader); + } catch (Throwable t) { + IOUtils.closeWhileSuppressingExceptions(t, reader); + throw t; } } } diff --git a/lucene/replicator/src/test/org/apache/lucene/replicator/nrt/TestSimpleServer.java b/lucene/replicator/src/test/org/apache/lucene/replicator/nrt/TestSimpleServer.java index 70039abe06e8..42be34ecd323 100644 --- a/lucene/replicator/src/test/org/apache/lucene/replicator/nrt/TestSimpleServer.java +++ b/lucene/replicator/src/test/org/apache/lucene/replicator/nrt/TestSimpleServer.java @@ -85,8 +85,7 @@ public ClientHandler(ServerSocket ss, Node node, Socket socket) { @Override public void run() { - boolean success = false; - try { + try (socket) { // node.message("using stream buffer size=" + bufferSize); InputStream is = new BufferedInputStream(socket.getInputStream(), bufferSize); DataInput in = new InputStreamDataInput(is); @@ -104,30 +103,18 @@ public void run() { if (VERBOSE_CONNECTIONS) { node.message("bos.flush done"); } - - success = true; } catch (Throwable t) { if (t instanceof SocketException == false && t instanceof NodeCommunicationException == false) { node.message("unexpected exception handling client connection; now failing test:"); t.printStackTrace(System.out); - IOUtils.closeWhileHandlingException(ss); + IOUtils.closeWhileSuppressingExceptions(t, ss); // Test should fail with this: throw new RuntimeException(t); } else { node.message("exception handling client connection; ignoring:"); t.printStackTrace(System.out); } - } finally { - if (success) { - try { - IOUtils.close(socket); - } catch (IOException ioe) { - throw new RuntimeException(ioe); - } - } else { - IOUtils.closeWhileHandlingException(socket); - } } if (VERBOSE_CONNECTIONS) { node.message("socket.close done"); diff --git a/lucene/suggest/src/java/org/apache/lucene/search/spell/PlainTextDictionary.java b/lucene/suggest/src/java/org/apache/lucene/search/spell/PlainTextDictionary.java index a1ed97aa5705..ebda2d55dbf4 100644 --- a/lucene/suggest/src/java/org/apache/lucene/search/spell/PlainTextDictionary.java +++ b/lucene/suggest/src/java/org/apache/lucene/search/spell/PlainTextDictionary.java @@ -78,7 +78,6 @@ public BytesRef next() throws IOException { if (done) { return null; } - boolean success = false; BytesRef result; try { String line; @@ -90,11 +89,9 @@ public BytesRef next() throws IOException { IOUtils.close(in); result = null; } - success = true; - } finally { - if (!success) { - IOUtils.closeWhileHandlingException(in); - } + } catch (Throwable t) { + IOUtils.closeWhileSuppressingExceptions(t, in); + throw t; } return result; } diff --git a/lucene/suggest/src/java/org/apache/lucene/search/suggest/SortedInputIterator.java b/lucene/suggest/src/java/org/apache/lucene/search/suggest/SortedInputIterator.java index 888941121be2..cab58779d189 100644 --- a/lucene/suggest/src/java/org/apache/lucene/search/suggest/SortedInputIterator.java +++ b/lucene/suggest/src/java/org/apache/lucene/search/suggest/SortedInputIterator.java @@ -82,7 +82,6 @@ public SortedInputIterator( @Override public BytesRef next() throws IOException { - boolean success = false; if (done) { return null; } @@ -97,18 +96,16 @@ public BytesRef next() throws IOException { if (hasContexts) { contexts = decodeContexts(bytes, input); } - success = true; return bytes; } - close(); - success = done = true; - return null; - } finally { - if (!success) { - done = true; - close(); - } + } catch (Throwable t) { + done = true; + IOUtils.closeWhileSuppressingExceptions(t, this::close); + throw t; } + done = true; + close(); + return null; } @Override diff --git a/lucene/suggest/src/java/org/apache/lucene/search/suggest/analyzing/AnalyzingInfixSuggester.java b/lucene/suggest/src/java/org/apache/lucene/search/suggest/analyzing/AnalyzingInfixSuggester.java index da92df1fc715..020f65b6db71 100644 --- a/lucene/suggest/src/java/org/apache/lucene/search/suggest/analyzing/AnalyzingInfixSuggester.java +++ b/lucene/suggest/src/java/org/apache/lucene/search/suggest/analyzing/AnalyzingInfixSuggester.java @@ -332,7 +332,6 @@ public void build(InputIterator iter) throws IOException { writer = null; } - boolean success = false; try { // First pass: build a temporary normal Lucene index, // just indexing the suggestions as they iterate: @@ -360,19 +359,17 @@ public void build(InputIterator iter) throws IOException { commit(); } setAndCloseOldSearcherManager(new SearcherManager(writer, null)); - success = true; - } finally { - if (success) { - if (closeIndexWriterOnBuild) { - writer.close(); - writer = null; - } - } else { // failure - if (writer != null) { - writer.rollback(); - writer = null; - } + } catch (Throwable t) { + if (writer != null) { + writer.rollback(); + writer = null; } + throw t; + } + + if (closeIndexWriterOnBuild) { + writer.close(); + writer = null; } } } diff --git a/lucene/suggest/src/java/org/apache/lucene/search/suggest/analyzing/FreeTextSuggester.java b/lucene/suggest/src/java/org/apache/lucene/search/suggest/analyzing/FreeTextSuggester.java index ae40aeb7d5cb..9e94a685d461 100644 --- a/lucene/suggest/src/java/org/apache/lucene/search/suggest/analyzing/FreeTextSuggester.java +++ b/lucene/suggest/src/java/org/apache/lucene/search/suggest/analyzing/FreeTextSuggester.java @@ -251,32 +251,28 @@ public void build(InputIterator iterator, double ramBufferSizeMB) throws IOExcep throw new IllegalArgumentException("this suggester doesn't support contexts"); } - String prefix = getClass().getSimpleName(); - Path tempIndexPath = Files.createTempDirectory(prefix + ".index."); - - Directory dir = FSDirectory.open(tempIndexPath); - IndexWriterConfig iwc = new IndexWriterConfig(indexAnalyzer); iwc.setOpenMode(IndexWriterConfig.OpenMode.CREATE); iwc.setRAMBufferSizeMB(ramBufferSizeMB); - IndexWriter writer = new IndexWriter(dir, iwc); - FieldType ft = new FieldType(TextField.TYPE_NOT_STORED); - // TODO: if only we had IndexOptions.TERMS_ONLY... - ft.setIndexOptions(IndexOptions.DOCS_AND_FREQS); - ft.setOmitNorms(true); - ft.freeze(); + String prefix = getClass().getSimpleName(); + Path tempIndexPath = Files.createTempDirectory(prefix + ".index."); + try (Directory dir = FSDirectory.open(tempIndexPath); + IndexWriter writer = new IndexWriter(dir, iwc)) { - Document doc = new Document(); - Field field = new Field("body", "", ft); - doc.add(field); + FieldType ft = new FieldType(TextField.TYPE_NOT_STORED); + // TODO: if only we had IndexOptions.TERMS_ONLY... + ft.setIndexOptions(IndexOptions.DOCS_AND_FREQS); + ft.setOmitNorms(true); + ft.freeze(); - totTokens = 0; - IndexReader reader = null; + Document doc = new Document(); + Field field = new Field("body", "", ft); + doc.add(field); - boolean success = false; - long newCount = 0; - try { + totTokens = 0; + + long newCount = 0; while (true) { BytesRef surfaceForm = iterator.next(); if (surfaceForm == null) { @@ -286,70 +282,64 @@ public void build(InputIterator iterator, double ramBufferSizeMB) throws IOExcep writer.addDocument(doc); newCount++; } - reader = DirectoryReader.open(writer); + try (IndexReader reader = DirectoryReader.open(writer)) { - Terms terms = MultiTerms.getTerms(reader, "body"); - if (terms == null) { - throw new IllegalArgumentException("need at least one suggestion"); - } + Terms terms = MultiTerms.getTerms(reader, "body"); + if (terms == null) { + throw new IllegalArgumentException("need at least one suggestion"); + } - // Move all ngrams into an FST: - TermsEnum termsEnum = terms.iterator(); + // Move all ngrams into an FST: + TermsEnum termsEnum = terms.iterator(); - Outputs outputs = PositiveIntOutputs.getSingleton(); - FSTCompiler fstCompiler = - new FSTCompiler.Builder<>(FST.INPUT_TYPE.BYTE1, outputs).build(); + Outputs outputs = PositiveIntOutputs.getSingleton(); + FSTCompiler fstCompiler = + new FSTCompiler.Builder<>(FST.INPUT_TYPE.BYTE1, outputs).build(); - IntsRefBuilder scratchInts = new IntsRefBuilder(); - while (true) { - BytesRef term = termsEnum.next(); - if (term == null) { - break; - } - int ngramCount = countGrams(term); - if (ngramCount > grams) { - throw new IllegalArgumentException( - "tokens must not contain separator byte; got token=" - + term - + " but gramCount=" - + ngramCount - + ", which is greater than expected max ngram size=" - + grams); + IntsRefBuilder scratchInts = new IntsRefBuilder(); + while (true) { + BytesRef term = termsEnum.next(); + if (term == null) { + break; + } + int ngramCount = countGrams(term); + if (ngramCount > grams) { + throw new IllegalArgumentException( + "tokens must not contain separator byte; got token=" + + term + + " but gramCount=" + + ngramCount + + ", which is greater than expected max ngram size=" + + grams); + } + if (ngramCount == 1) { + totTokens += termsEnum.totalTermFreq(); + } + + fstCompiler.add( + Util.toIntsRef(term, scratchInts), encodeWeight(termsEnum.totalTermFreq())); } - if (ngramCount == 1) { - totTokens += termsEnum.totalTermFreq(); + + final FST newFst = + FST.fromFSTReader(fstCompiler.compile(), fstCompiler.getFSTReader()); + if (newFst == null) { + throw new IllegalArgumentException("need at least one suggestion"); } + fst = newFst; + count = newCount; - fstCompiler.add(Util.toIntsRef(term, scratchInts), encodeWeight(termsEnum.totalTermFreq())); - } + // System.out.println("FST: " + fst.getNodeCount() + " nodes"); - final FST newFst = FST.fromFSTReader(fstCompiler.compile(), fstCompiler.getFSTReader()); - if (newFst == null) { - throw new IllegalArgumentException("need at least one suggestion"); - } - fst = newFst; - count = newCount; - - // System.out.println("FST: " + fst.getNodeCount() + " nodes"); - - /* - PrintWriter pw = new PrintWriter("/x/tmp/out.dot"); - Util.toDot(fst, pw, true, true); - pw.close(); - */ - - // Writer was only temporary, to count up bigrams, - // which we transferred to the FST, so now we - // rollback: - writer.rollback(); - success = true; - } finally { - try { - if (success) { - IOUtils.close(reader, dir); - } else { - IOUtils.closeWhileHandlingException(reader, writer, dir); - } + /* + PrintWriter pw = new PrintWriter("/x/tmp/out.dot"); + Util.toDot(fst, pw, true, true); + pw.close(); + */ + + // Writer was only temporary, to count up bigrams, + // which we transferred to the FST, so now we + // rollback: + writer.rollback(); } finally { IOUtils.rm(tempIndexPath); } diff --git a/lucene/suggest/src/java/org/apache/lucene/search/suggest/document/CompletionFieldsConsumer.java b/lucene/suggest/src/java/org/apache/lucene/search/suggest/document/CompletionFieldsConsumer.java index 3c96151a55ed..f61de6e4341b 100644 --- a/lucene/suggest/src/java/org/apache/lucene/search/suggest/document/CompletionFieldsConsumer.java +++ b/lucene/suggest/src/java/org/apache/lucene/search/suggest/document/CompletionFieldsConsumer.java @@ -66,7 +66,6 @@ final class CompletionFieldsConsumer extends FieldsConsumer { this.state = state; String dictFile = IndexFileNames.segmentFileName(state.segmentInfo.name, state.segmentSuffix, DICT_EXTENSION); - boolean success = false; try { this.delegateFieldsConsumer = delegatePostingsFormat.fieldsConsumer(state); dictOut = state.directory.createOutput(dictFile, state.context); @@ -76,11 +75,9 @@ final class CompletionFieldsConsumer extends FieldsConsumer { COMPLETION_VERSION_CURRENT, state.segmentInfo.getId(), state.segmentSuffix); - success = true; - } finally { - if (success == false) { - IOUtils.closeWhileHandlingException(dictOut, delegateFieldsConsumer); - } + } catch (Throwable t) { + IOUtils.closeWhileSuppressingExceptions(t, dictOut, delegateFieldsConsumer); + throw t; } } diff --git a/lucene/suggest/src/java/org/apache/lucene/search/suggest/document/CompletionFieldsProducer.java b/lucene/suggest/src/java/org/apache/lucene/search/suggest/document/CompletionFieldsProducer.java index 310c896fa830..1a0d709bdec7 100644 --- a/lucene/suggest/src/java/org/apache/lucene/search/suggest/document/CompletionFieldsProducer.java +++ b/lucene/suggest/src/java/org/apache/lucene/search/suggest/document/CompletionFieldsProducer.java @@ -72,7 +72,6 @@ private CompletionFieldsProducer( IndexFileNames.segmentFileName( state.segmentInfo.name, state.segmentSuffix, INDEX_EXTENSION); delegateFieldsProducer = null; - boolean success = false; try (ChecksumIndexInput index = state.directory.openChecksumInput(indexFile)) { // open up dict file containing all fsts @@ -119,26 +118,15 @@ private CompletionFieldsProducer( fieldInfo.name, new CompletionsTermsReader(dictIn, offset, minWeight, maxWeight, type)); } CodecUtil.checkFooter(index); - success = true; - } finally { - if (success == false) { - IOUtils.closeWhileHandlingException(delegateFieldsProducer, dictIn); - } + } catch (Throwable t) { + IOUtils.closeWhileSuppressingExceptions(t, delegateFieldsProducer, dictIn); + throw t; } } @Override public void close() throws IOException { - boolean success = false; - try { - delegateFieldsProducer.close(); - IOUtils.close(dictIn); - success = true; - } finally { - if (success == false) { - IOUtils.closeWhileHandlingException(delegateFieldsProducer, dictIn); - } - } + IOUtils.close(delegateFieldsProducer, dictIn); } @Override diff --git a/lucene/suggest/src/java/org/apache/lucene/search/suggest/fst/ExternalRefSorter.java b/lucene/suggest/src/java/org/apache/lucene/search/suggest/fst/ExternalRefSorter.java index bf672593fafb..1c8ebdcbbb98 100644 --- a/lucene/suggest/src/java/org/apache/lucene/search/suggest/fst/ExternalRefSorter.java +++ b/lucene/suggest/src/java/org/apache/lucene/search/suggest/fst/ExternalRefSorter.java @@ -74,16 +74,9 @@ public ByteSequenceIterator iterator() throws IOException { closeWriter(); - boolean success = false; - try { + // delete temp file after this + try (Closeable _ = () -> sorter.getDirectory().deleteFile(tempOutput.getName())) { sortedOutput = sorter.sort(tempOutput.getName()); - success = true; - } finally { - if (success) { - sorter.getDirectory().deleteFile(tempOutput.getName()); - } else { - IOUtils.deleteFilesIgnoringExceptions(sorter.getDirectory(), tempOutput.getName()); - } } tempOutput = null; @@ -126,18 +119,15 @@ private ByteSequenceIterator(OfflineSorter.ByteSequencesReader reader) { @Override public BytesRef next() throws IOException { - boolean success = false; try { BytesRef scratch = reader.next(); if (scratch == null) { close(); } - success = true; return scratch; - } finally { - if (!success) { - IOUtils.closeWhileHandlingException(reader); - } + } catch (Throwable t) { + IOUtils.closeWhileSuppressingExceptions(t, reader); + throw t; } } From 8c6be013f6d1874d894ae8d19ec1256f8e8adb3e Mon Sep 17 00:00:00 2001 From: Simon Cooper Date: Thu, 28 Aug 2025 10:42:20 +0100 Subject: [PATCH 2/2] finally on the right branch --- .../lucene/search/suggest/analyzing/FreeTextSuggester.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lucene/suggest/src/java/org/apache/lucene/search/suggest/analyzing/FreeTextSuggester.java b/lucene/suggest/src/java/org/apache/lucene/search/suggest/analyzing/FreeTextSuggester.java index 9e94a685d461..619cfa1ce59b 100644 --- a/lucene/suggest/src/java/org/apache/lucene/search/suggest/analyzing/FreeTextSuggester.java +++ b/lucene/suggest/src/java/org/apache/lucene/search/suggest/analyzing/FreeTextSuggester.java @@ -340,9 +340,9 @@ public void build(InputIterator iterator, double ramBufferSizeMB) throws IOExcep // which we transferred to the FST, so now we // rollback: writer.rollback(); - } finally { - IOUtils.rm(tempIndexPath); } + } finally { + IOUtils.rm(tempIndexPath); } }