Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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");
}
}
}