From 57f887a529f276b80022dbb87b7a17298fde8b20 Mon Sep 17 00:00:00 2001 From: Surendra Singh Lilhore Date: Tue, 10 Mar 2020 23:56:42 +0530 Subject: [PATCH] HDFS-15135. EC : ArrayIndexOutOfBoundsException in BlockRecoveryWorker#RecoveryTaskStriped. Contributed by Ravuri Sushma sree. --- ...atanodeProtocolClientSideTranslatorPB.java | 3 + .../server/datanode/BlockRecoveryWorker.java | 2 +- .../server/datanode/TestBlockRecovery.java | 67 ++++++++++++++++++- 3 files changed, 70 insertions(+), 2 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/DatanodeProtocolClientSideTranslatorPB.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/DatanodeProtocolClientSideTranslatorPB.java index e4125dc9e41..e95b4c7aa84 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/DatanodeProtocolClientSideTranslatorPB.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/DatanodeProtocolClientSideTranslatorPB.java @@ -319,6 +319,9 @@ public class DatanodeProtocolClientSideTranslatorPB implements .setNewLength(newlength).setCloseFile(closeFile) .setDeleteBlock(deleteblock); for (int i = 0; i < newtargets.length; i++) { + if (newtargets[i] == null) { + continue; + } builder.addNewTaragets(PBHelperClient.convert(newtargets[i])); builder.addNewTargetStorages(newtargetstorages[i]); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/BlockRecoveryWorker.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/BlockRecoveryWorker.java index 34f6c33003f..1e9cf2adfdc 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/BlockRecoveryWorker.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/BlockRecoveryWorker.java @@ -462,7 +462,7 @@ public class BlockRecoveryWorker { // notify Namenode the new size and locations final DatanodeID[] newLocs = new DatanodeID[totalBlkNum]; final String[] newStorages = new String[totalBlkNum]; - for (int i = 0; i < totalBlkNum; i++) { + for (int i = 0; i < blockIndices.length; i++) { newLocs[blockIndices[i]] = DatanodeID.EMPTY_DATANODE_ID; newStorages[blockIndices[i]] = ""; } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockRecovery.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockRecovery.java index 80f9fb6b002..6e0490e9b28 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockRecovery.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockRecovery.java @@ -33,6 +33,8 @@ import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyBoolean; import static org.mockito.Matchers.anyListOf; import static org.mockito.Matchers.anyLong; +import static org.mockito.Matchers.anyString; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Matchers.anyObject; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; @@ -44,6 +46,7 @@ import static org.mockito.Mockito.when; import java.io.File; import java.io.IOException; +import java.io.OutputStream; import java.net.InetSocketAddress; import java.net.URISyntaxException; import java.util.ArrayList; @@ -61,6 +64,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import com.google.common.collect.Iterators; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.apache.hadoop.conf.Configuration; @@ -99,6 +103,7 @@ import org.apache.hadoop.hdfs.server.protocol.DatanodeRegistration; import org.apache.hadoop.hdfs.server.protocol.HeartbeatResponse; import org.apache.hadoop.hdfs.server.protocol.InterDatanodeProtocol; import org.apache.hadoop.hdfs.server.protocol.NNHAStatusHeartbeat; +import org.apache.hadoop.hdfs.server.protocol.NamenodeProtocols; import org.apache.hadoop.hdfs.server.protocol.NamespaceInfo; import org.apache.hadoop.hdfs.server.protocol.ReplicaRecoveryInfo; import org.apache.hadoop.hdfs.server.protocol.StorageReport; @@ -708,7 +713,67 @@ public class TestBlockRecovery { streams.close(); } } - + + @Test(timeout = 60000) + public void testEcRecoverBlocks() throws Throwable { + // Stop the Mocked DN started in startup() + tearDown(); + ErasureCodingPolicy ecPolicy = StripedFileTestUtil.getDefaultECPolicy(); + MiniDFSCluster cluster = new MiniDFSCluster.Builder(conf).numDataNodes(8) + .build(); + + try { + cluster.waitActive(); + NamenodeProtocols preSpyNN = cluster.getNameNodeRpc(); + NamenodeProtocols spyNN = spy(preSpyNN); + + // Delay completeFile + GenericTestUtils.DelayAnswer delayer = new GenericTestUtils.DelayAnswer( + LOG); + doAnswer(delayer).when(spyNN).complete(anyString(), anyString(), any(), + anyLong()); + String topDir = "/myDir"; + DFSClient client = new DFSClient(null, spyNN, conf, null); + Path file = new Path(topDir + "/testECLeaseRecover"); + client.mkdirs(topDir, null, false); + client.enableErasureCodingPolicy(ecPolicy.getName()); + client.setErasureCodingPolicy(topDir, ecPolicy.getName()); + OutputStream stm = client.create(file.toString(), true); + + // write 5MB File + AppendTestUtil.write(stm, 0, 1024 * 1024 * 5); + final AtomicReference err = new AtomicReference(); + Thread t = new Thread() { + @Override + public void run() { + try { + stm.close(); + } catch (Throwable t) { + err.set(t); + } + } + }; + t.start(); + + // Waiting for close to get to latch + delayer.waitForCall(); + GenericTestUtils.waitFor(new Supplier() { + @Override + public Boolean get() { + try { + return client.getNamenode().recoverLease(file.toString(), + client.getClientName()); + } catch (IOException e) { + return false; + } + } + }, 5000, 24000); + delayer.proceed(); + } finally { + cluster.shutdown(); + } + } + /** * Test to verify the race between finalizeBlock and Lease recovery *