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

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


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

commit 22831aae2015c17aa0b2b1ab9a4f08d5aed3f1da
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 7c350afb95..68e2be016d 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
@@ -1288,24 +1288,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 RuntimeException("Calling closeDeepCopies on a deep copy is 
not supported");
       }
 
       for (Reader deepCopy : deepCopies) {
-        deepCopy.closeLocalityGroupReaders();
+        deepCopy.closeLocalityGroupReaders(ignoreIOExceptions);
       }
 
       deepCopies.clear();
@@ -1317,8 +1325,9 @@ public class RFile {
         throw new RuntimeException("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) {

Reply via email to