Skip to content

Commit

Permalink
Propagates errors in Rfile.closeDeepCopies (apache#5248)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
keith-turner committed Jan 16, 2025
1 parent d758759 commit 22831aa
Showing 1 changed file with 15 additions and 6 deletions.
21 changes: 15 additions & 6 deletions core/src/main/java/org/apache/accumulo/core/file/rfile/RFile.java
Original file line number Diff line number Diff line change
Expand Up @@ -1288,24 +1288,32 @@ public Reader(CachableBlockFile.CachableBuilder b) throws IOException {
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();
Expand All @@ -1317,8 +1325,9 @@ public void close() throws IOException {
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) {
Expand Down

0 comments on commit 22831aa

Please sign in to comment.