(accumulo) branch main updated: Propagates errors in Rfile.closeDeepCopies (#5248)

[kturner]

This is an automated email from the ASF dual-hosted git repository.

kturner pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/accumulo.git


The following commit(s) were added to refs/heads/main by this push:
     new 0cc90efeab Propagates errors in Rfile.closeDeepCopies (#5248)
0cc90efeab is described below

commit 0cc90efeabdce7ba9543bbace72115763ed16f6a
Author: Keith Turner <ktur...@apache.org>
AuthorDate: Thu Jan 16 17:58:34 2025 -0500

    Propagates errors in Rfile.closeDeepCopies (#5248)
    
    Rfile.closeLocalityGroupReaders() was suppressing IOExceptions.
    This method was called by Rfile.closeDeepCopies() which was called by
    FileManager.releaseReaders().  Suppressing the exception meant that
    releaseReaders() did not see the exception and would decided to return
    the rfile to the pool when it should not.
    
    The only other code calling Rfile.closeLocalityGroupReaders() was
    Rfile.close(). Refactored the code so that Rfile.close() still suppressed
    the exception and Rfile.closeDeepCopies() does not suppress.  Tried to
    preserve the behavior that Rfile.close() closes as many of its
    underlying resource as possible even if some exceptions occur.
---
 .../org/apache/accumulo/core/file/rfile/RFile.java  | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/core/src/main/java/org/apache/accumulo/core/file/rfile/RFile.java 
b/core/src/main/java/org/apache/accumulo/core/file/rfile/RFile.java
index 04c185e321..3956510e5d 100644
--- a/core/src/main/java/org/apache/accumulo/core/file/rfile/RFile.java
+++ b/core/src/main/java/org/apache/accumulo/core/file/rfile/RFile.java
@@ -1294,24 +1294,32 @@ public class RFile {
       this(new CachableBlockFile.Reader(b));
     }
 
-    private void closeLocalityGroupReaders() {
+    private void closeLocalityGroupReaders(boolean ignoreIOExceptions) throws 
IOException {
       for (LocalityGroupReader lgr : currentReaders) {
         try {
           lgr.close();
         } catch (IOException e) {
-          log.warn("Errored out attempting to close LocalityGroupReader.", e);
+          if (ignoreIOExceptions) {
+            log.warn("Errored out attempting to close LocalityGroupReader.", 
e);
+          } else {
+            throw e;
+          }
         }
       }
     }
 
     @Override
-    public void closeDeepCopies() {
+    public void closeDeepCopies() throws IOException {
+      closeDeepCopies(false);
+    }
+
+    private void closeDeepCopies(boolean ignoreIOExceptions) throws 
IOException {
       if (deepCopy) {
         throw new IllegalStateException("Calling closeDeepCopies on a deep 
copy is not supported");
       }
 
       for (Reader deepCopy : deepCopies) {
-        deepCopy.closeLocalityGroupReaders();
+        deepCopy.closeLocalityGroupReaders(ignoreIOExceptions);
       }
 
       deepCopies.clear();
@@ -1323,8 +1331,9 @@ public class RFile {
         throw new IllegalStateException("Calling close on a deep copy is not 
supported");
       }
 
-      closeDeepCopies();
-      closeLocalityGroupReaders();
+      // Closes as much as possible igoring and logging exceptions along the 
way
+      closeDeepCopies(true);
+      closeLocalityGroupReaders(true);
 
       if (sampleReaders != null) {
         for (LocalityGroupReader lgr : sampleReaders) {