Skip to content

Commit 22831aa

Browse files
committed
Propagates errors in Rfile.closeDeepCopies (apache#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.
1 parent d758759 commit 22831aa

File tree

1 file changed

+15
-6
lines changed
  • core/src/main/java/org/apache/accumulo/core/file/rfile

1 file changed

+15
-6
lines changed

core/src/main/java/org/apache/accumulo/core/file/rfile/RFile.java

+15-6
Original file line numberDiff line numberDiff line change
@@ -1288,24 +1288,32 @@ public Reader(CachableBlockFile.CachableBuilder b) throws IOException {
12881288
this(new CachableBlockFile.Reader(b));
12891289
}
12901290

1291-
private void closeLocalityGroupReaders() {
1291+
private void closeLocalityGroupReaders(boolean ignoreIOExceptions) throws IOException {
12921292
for (LocalityGroupReader lgr : currentReaders) {
12931293
try {
12941294
lgr.close();
12951295
} catch (IOException e) {
1296-
log.warn("Errored out attempting to close LocalityGroupReader.", e);
1296+
if (ignoreIOExceptions) {
1297+
log.warn("Errored out attempting to close LocalityGroupReader.", e);
1298+
} else {
1299+
throw e;
1300+
}
12971301
}
12981302
}
12991303
}
13001304

13011305
@Override
1302-
public void closeDeepCopies() {
1306+
public void closeDeepCopies() throws IOException {
1307+
closeDeepCopies(false);
1308+
}
1309+
1310+
private void closeDeepCopies(boolean ignoreIOExceptions) throws IOException {
13031311
if (deepCopy) {
13041312
throw new RuntimeException("Calling closeDeepCopies on a deep copy is not supported");
13051313
}
13061314

13071315
for (Reader deepCopy : deepCopies) {
1308-
deepCopy.closeLocalityGroupReaders();
1316+
deepCopy.closeLocalityGroupReaders(ignoreIOExceptions);
13091317
}
13101318

13111319
deepCopies.clear();
@@ -1317,8 +1325,9 @@ public void close() throws IOException {
13171325
throw new RuntimeException("Calling close on a deep copy is not supported");
13181326
}
13191327

1320-
closeDeepCopies();
1321-
closeLocalityGroupReaders();
1328+
// Closes as much as possible igoring and logging exceptions along the way
1329+
closeDeepCopies(true);
1330+
closeLocalityGroupReaders(true);
13221331

13231332
if (sampleReaders != null) {
13241333
for (LocalityGroupReader lgr : sampleReaders) {

0 commit comments

Comments
 (0)