From 83535ef2b2c6a8e5bad541febe9c98eecd2aa633 Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Mon, 18 May 2026 17:56:38 +0000 Subject: [PATCH 1/3] Close DFSInputStream on exception in CachableBlockFile.getBCFile A RateLimitedInputStream is created from the supplied InputStream, which in most cases is a DFSInputStream. However, when BCFile.Reader throws an exception the RateLimitedInputStream is not closed leaving the related DFSInputStream open. --- .../blockfile/impl/CachableBlockFile.java | 25 ++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/file/blockfile/impl/CachableBlockFile.java b/core/src/main/java/org/apache/accumulo/core/file/blockfile/impl/CachableBlockFile.java index f13c155641c..aa6813ff778 100644 --- a/core/src/main/java/org/apache/accumulo/core/file/blockfile/impl/CachableBlockFile.java +++ b/core/src/main/java/org/apache/accumulo/core/file/blockfile/impl/CachableBlockFile.java @@ -183,13 +183,18 @@ private BCFile.Reader getBCFile(Supplier cachedMetadataSupplier) throws BCFile.Reader reader = bcfr.get(); if (reader == null) { + IOException ioe = null; RateLimitedInputStream fsIn = new RateLimitedInputStream((InputStream & Seekable) inputSupplier.get(), readLimiter); BCFile.Reader tmpReader = null; byte[] serializedMetadata = cachedMetadataSupplier.get(); if (serializedMetadata == null) { if (fileLenCache == null) { - tmpReader = new BCFile.Reader(fsIn, lengthSupplier.get(), conf, cryptoService); + try { + tmpReader = new BCFile.Reader(fsIn, lengthSupplier.get(), conf, cryptoService); + } catch (IOException e) { + ioe = e; + } } else { long len = getCachedFileLen(); try { @@ -201,19 +206,33 @@ private BCFile.Reader getBCFile(Supplier cachedMetadataSupplier) throws if (tmpReader == null) { len = getCachedFileLen(); - tmpReader = new BCFile.Reader(fsIn, len, conf, cryptoService); + try { + tmpReader = new BCFile.Reader(fsIn, len, conf, cryptoService); + } catch (IOException e) { + ioe = e; + } } } } else { tmpReader = new BCFile.Reader(serializedMetadata, fsIn, conf, cryptoService); } + if (ioe != null) { + fsIn.close(); + if (fileLenCache != null) { + fileLenCache.invalidate(cacheId); + } + throw new IOException("Error creating BCFile.Reader", ioe); + } + if (bcfr.compareAndSet(null, tmpReader)) { fin = fsIn; return tmpReader; } else { fsIn.close(); - tmpReader.close(); + if (tmpReader != null) { + tmpReader.close(); + } return bcfr.get(); } } From 32a17bedb774702e770354d069ef477cc091d245 Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Tue, 19 May 2026 11:48:16 +0000 Subject: [PATCH 2/3] use single try-catch around all BCFile.Reader constructors --- .../blockfile/impl/CachableBlockFile.java | 47 +++++++------------ 1 file changed, 18 insertions(+), 29 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/file/blockfile/impl/CachableBlockFile.java b/core/src/main/java/org/apache/accumulo/core/file/blockfile/impl/CachableBlockFile.java index aa6813ff778..9f15840d2a3 100644 --- a/core/src/main/java/org/apache/accumulo/core/file/blockfile/impl/CachableBlockFile.java +++ b/core/src/main/java/org/apache/accumulo/core/file/blockfile/impl/CachableBlockFile.java @@ -183,46 +183,37 @@ private BCFile.Reader getBCFile(Supplier cachedMetadataSupplier) throws BCFile.Reader reader = bcfr.get(); if (reader == null) { - IOException ioe = null; RateLimitedInputStream fsIn = new RateLimitedInputStream((InputStream & Seekable) inputSupplier.get(), readLimiter); BCFile.Reader tmpReader = null; byte[] serializedMetadata = cachedMetadataSupplier.get(); - if (serializedMetadata == null) { - if (fileLenCache == null) { - try { + try { + if (serializedMetadata == null) { + if (fileLenCache == null) { tmpReader = new BCFile.Reader(fsIn, lengthSupplier.get(), conf, cryptoService); - } catch (IOException e) { - ioe = e; - } - } else { - long len = getCachedFileLen(); - try { - tmpReader = new BCFile.Reader(fsIn, len, conf, cryptoService); - } catch (Exception e) { - log.debug("Failed to open {}, clearing file length cache and retrying", cacheId, e); - fileLenCache.invalidate(cacheId); - } - - if (tmpReader == null) { - len = getCachedFileLen(); + } else { + long len = getCachedFileLen(); try { tmpReader = new BCFile.Reader(fsIn, len, conf, cryptoService); - } catch (IOException e) { - ioe = e; + } catch (Exception e) { + log.debug("Failed to open {}, clearing file length cache and retrying", cacheId, e); + fileLenCache.invalidate(cacheId); + } + + if (tmpReader == null) { + len = getCachedFileLen(); + tmpReader = new BCFile.Reader(fsIn, len, conf, cryptoService); } } + } else { + tmpReader = new BCFile.Reader(serializedMetadata, fsIn, conf, cryptoService); } - } else { - tmpReader = new BCFile.Reader(serializedMetadata, fsIn, conf, cryptoService); - } - - if (ioe != null) { + } catch (IOException | RuntimeException e) { fsIn.close(); if (fileLenCache != null) { fileLenCache.invalidate(cacheId); } - throw new IOException("Error creating BCFile.Reader", ioe); + throw e; } if (bcfr.compareAndSet(null, tmpReader)) { @@ -230,9 +221,7 @@ private BCFile.Reader getBCFile(Supplier cachedMetadataSupplier) throws return tmpReader; } else { fsIn.close(); - if (tmpReader != null) { - tmpReader.close(); - } + tmpReader.close(); return bcfr.get(); } } From dd8515c8b21f0fb3ef294ce25684d05db7165c96 Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Tue, 19 May 2026 16:04:25 +0000 Subject: [PATCH 3/3] Catch error coming from supplier --- .../accumulo/core/file/blockfile/impl/CachableBlockFile.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/accumulo/core/file/blockfile/impl/CachableBlockFile.java b/core/src/main/java/org/apache/accumulo/core/file/blockfile/impl/CachableBlockFile.java index 9f15840d2a3..19033e4394c 100644 --- a/core/src/main/java/org/apache/accumulo/core/file/blockfile/impl/CachableBlockFile.java +++ b/core/src/main/java/org/apache/accumulo/core/file/blockfile/impl/CachableBlockFile.java @@ -186,8 +186,8 @@ private BCFile.Reader getBCFile(Supplier cachedMetadataSupplier) throws RateLimitedInputStream fsIn = new RateLimitedInputStream((InputStream & Seekable) inputSupplier.get(), readLimiter); BCFile.Reader tmpReader = null; - byte[] serializedMetadata = cachedMetadataSupplier.get(); try { + byte[] serializedMetadata = cachedMetadataSupplier.get(); if (serializedMetadata == null) { if (fileLenCache == null) { tmpReader = new BCFile.Reader(fsIn, lengthSupplier.get(), conf, cryptoService);