diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/FileIOEngine.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/FileIOEngine.java index ba431c4c6dcb..e7e6fd9a7278 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/FileIOEngine.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/FileIOEngine.java @@ -17,6 +17,7 @@ */ package org.apache.hadoop.hbase.io.hfile.bucket; +import java.io.Closeable; import java.io.File; import java.io.IOException; import java.io.RandomAccessFile; @@ -225,15 +226,23 @@ public void sync() throws IOException { @Override public void shutdown() { for (int i = 0; i < filePaths.length; i++) { + closeAll(filePaths[i], fileChannels[i], rafs[i]); + } + } + + /** + * Close each non-null closeable, logging failures without throwing, so one failure cannot skip + * the rest. + */ + private static void closeAll(String filePath, Closeable... closeables) { + for (Closeable c : closeables) { + if (c == null) { + continue; + } try { - if (fileChannels[i] != null) { - fileChannels[i].close(); - } - if (rafs[i] != null) { - rafs[i].close(); - } - } catch (IOException ex) { - LOG.error("Failed closing " + filePaths[i] + " when shudown the IOEngine", ex); + c.close(); + } catch (IOException e) { + LOG.error("Failed closing {} when shutting down the IOEngine", filePath, e); } } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestFileIOEngine.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestFileIOEngine.java index f19d13d8490e..66d3d80cd58d 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestFileIOEngine.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestFileIOEngine.java @@ -27,6 +27,8 @@ import java.io.File; import java.io.IOException; +import java.io.RandomAccessFile; +import java.lang.reflect.Field; import java.nio.ByteBuffer; import java.nio.channels.FileChannel; import java.util.ArrayList; @@ -192,4 +194,24 @@ public void testRefreshFileConnection() throws IOException { assertEquals(fileChannels[i], reopenedFileChannels[i]); } } + + @Test + public void testShutdownClosesRandomAccessFileWhenChannelCloseFails() throws Exception { + FileChannel[] channels = fileIOEngine.getFileChannels(); + + // rafs is private + Field rafsField = FileIOEngine.class.getDeclaredField("rafs"); + rafsField.setAccessible(true); + RandomAccessFile[] rafs = (RandomAccessFile[]) rafsField.get(fileIOEngine); + + FileChannel failingChannel = Mockito.mock(FileChannel.class); + Mockito.doThrow(new IOException("channel close failed")).when(failingChannel).close(); + RandomAccessFile raf = Mockito.mock(RandomAccessFile.class); + channels[0] = failingChannel; + rafs[0] = raf; + + fileIOEngine.shutdown(); + + Mockito.verify(raf).close(); + } }