diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/FSHLog.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/FSHLog.java index 131f284557af..f502e66b44e9 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/FSHLog.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/FSHLog.java @@ -591,13 +591,21 @@ boolean isLowReplicationRollEnabled() { + (3 * Bytes.SIZEOF_INT) + (4 * Bytes.SIZEOF_LONG)); /** - * This method gets the pipeline for the current WAL. + * This method gets the pipeline for the current WAL. Note that DFSOutputStream#getPipeline() can + * legitimately return null (e.g. the underlying streamer is closed, or no block pipeline is + * currently established, such as between blocks) -- see HDFS-826. We normalize that to an empty + * array here so that callers (in particular debug-log formatting in AbstractFSWAL) never have to + * null-check, matching the contract already honored by AsyncFSWAL#getPipeline(). */ @Override DatanodeInfo[] getPipeline() { if (this.hdfs_out != null) { if (this.hdfs_out.getWrappedStream() instanceof DFSOutputStream) { - return ((DFSOutputStream) this.hdfs_out.getWrappedStream()).getPipeline(); + DatanodeInfo[] pipeline = + ((DFSOutputStream) this.hdfs_out.getWrappedStream()).getPipeline(); + if (pipeline != null) { + return pipeline; + } } } return new DatanodeInfo[0]; diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestFSHLog.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestFSHLog.java index d6310da12320..20e3cfa4c04d 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestFSHLog.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestFSHLog.java @@ -18,6 +18,9 @@ package org.apache.hadoop.hbase.regionserver.wal; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import java.io.IOException; import java.lang.reflect.Field; @@ -29,6 +32,7 @@ import java.util.concurrent.Executors; import java.util.concurrent.atomic.AtomicBoolean; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.FSDataOutputStream; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.HConstants; @@ -50,6 +54,9 @@ import org.apache.hadoop.hbase.util.Threads; import org.apache.hadoop.hbase.wal.WALEdit; import org.apache.hadoop.hbase.wal.WALKey; +import org.apache.hadoop.hdfs.DFSOutputStream; +import org.apache.hadoop.hdfs.client.HdfsDataOutputStream; +import org.apache.hadoop.hdfs.protocol.DatanodeInfo; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; @@ -215,4 +222,37 @@ public void run() { region.close(); } } + + /** + * Regression test for HBASE-30346: FSHLog#getPipeline() must never return null, even when the + * underlying DFSOutputStream#getPipeline() legitimately returns null (e.g. the DFS streamer is + * closed, or no block pipeline is currently established -- see HDFS-826 and the + * DFSOutputStream#getPipeline() javadoc: "returns the list of targets, if any"). Prior to this + * fix, AbstractFSWAL#rollWriterInternal's debug-log statement called + * Arrays.stream(getPipeline()), which threw a NullPointerException whenever this happened during + * a WAL roll, aborting the RegionServer on what was otherwise a successful roll. + */ + @Test + public void testGetPipelineDoesNotReturnNullWhenUnderlyingStreamerHasNone() throws Exception { + FS.mkdirs(new Path(CommonFSUtils.getRootDir(CONF), this.name)); + try (FSHLog log = new FSHLog(FS, CommonFSUtils.getRootDir(CONF), this.name, + HConstants.HREGION_OLDLOGDIR_NAME, CONF, null, true, null, null)) { + log.init(); + + // Simulate the legitimate HDFS contract: the wrapped DFSOutputStream currently has no + // established pipeline (e.g. streamer closed, or between blocks) and returns null. + DFSOutputStream mockDfsOut = mock(DFSOutputStream.class); + when(mockDfsOut.getPipeline()).thenReturn(null); + FSDataOutputStream wrappedOut = new HdfsDataOutputStream(mockDfsOut, null); + + Field hdfsOutField = FSHLog.class.getDeclaredField("hdfs_out"); + hdfsOutField.setAccessible(true); + hdfsOutField.set(log, wrappedOut); + + DatanodeInfo[] pipeline = log.getPipeline(); + assertNotNull(pipeline, "getPipeline() must never return null"); + assertEquals(0, pipeline.length, + "Should normalize a null underlying pipeline to an empty array"); + } + } }