From ee5daee1b75289d007193718b69bdede5f3aa11c Mon Sep 17 00:00:00 2001 From: Todd Lipcon Date: Mon, 9 Apr 2012 03:22:41 +0000 Subject: [PATCH] HDFS-3214. InterDatanodeProtocolServerSideTranslatorPB doesn't handle null response from initReplicaRecovery. Contributed by Todd Lipcon. git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1311125 13f79535-47bb-0310-9956-ffa450edef68 --- hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt | 3 +++ ...nterDatanodeProtocolServerSideTranslatorPB.java | 14 +++++++++++--- .../InterDatanodeProtocolTranslatorPB.java | 11 +++++++++++ .../src/main/proto/InterDatanodeProtocol.proto | 7 +++++-- .../fsdataset/impl/TestInterDatanodeProtocol.java | 10 ++++++++-- 5 files changed, 38 insertions(+), 7 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index 115a855139b..7d3a52465dc 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -453,6 +453,9 @@ Release 2.0.0 - UNRELEASED HDFS-3136. Remove SLF4J dependency as HDFS does not need it to fix unnecessary warnings. (Jason Lowe via suresh) + HDFS-3214. InterDatanodeProtocolServerSideTranslatorPB doesn't handle + null response from initReplicaRecovery (todd) + BREAKDOWN OF HDFS-1623 SUBTASKS HDFS-2179. Add fencing framework and mechanisms for NameNode HA. (todd) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/InterDatanodeProtocolServerSideTranslatorPB.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/InterDatanodeProtocolServerSideTranslatorPB.java index 5c475c8502a..8f3eed96852 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/InterDatanodeProtocolServerSideTranslatorPB.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/InterDatanodeProtocolServerSideTranslatorPB.java @@ -56,9 +56,17 @@ public class InterDatanodeProtocolServerSideTranslatorPB implements } catch (IOException e) { throw new ServiceException(e); } - return InitReplicaRecoveryResponseProto.newBuilder() - .setBlock(PBHelper.convert(r)) - .setState(PBHelper.convert(r.getOriginalReplicaState())).build(); + + if (r == null) { + return InitReplicaRecoveryResponseProto.newBuilder() + .setReplicaFound(false) + .build(); + } else { + return InitReplicaRecoveryResponseProto.newBuilder() + .setReplicaFound(true) + .setBlock(PBHelper.convert(r)) + .setState(PBHelper.convert(r.getOriginalReplicaState())).build(); + } } @Override diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/InterDatanodeProtocolTranslatorPB.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/InterDatanodeProtocolTranslatorPB.java index 9d301916ddd..547ca5c21bd 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/InterDatanodeProtocolTranslatorPB.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/InterDatanodeProtocolTranslatorPB.java @@ -85,6 +85,17 @@ public class InterDatanodeProtocolTranslatorPB implements } catch (ServiceException e) { throw ProtobufHelper.getRemoteException(e); } + if (!resp.getReplicaFound()) { + // No replica found on the remote node. + return null; + } else { + if (!resp.hasBlock() || !resp.hasState()) { + throw new IOException("Replica was found but missing fields. " + + "Req: " + req + "\n" + + "Resp: " + resp); + } + } + BlockProto b = resp.getBlock(); return new ReplicaRecoveryInfo(b.getBlockId(), b.getNumBytes(), b.getGenStamp(), PBHelper.convert(resp.getState())); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/proto/InterDatanodeProtocol.proto b/hadoop-hdfs-project/hadoop-hdfs/src/main/proto/InterDatanodeProtocol.proto index 99c98cc1919..1e7c1e59b31 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/proto/InterDatanodeProtocol.proto +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/proto/InterDatanodeProtocol.proto @@ -38,8 +38,11 @@ message InitReplicaRecoveryRequestProto { * Repica recovery information */ message InitReplicaRecoveryResponseProto { - required ReplicaStateProto state = 1; // State of the replica - required BlockProto block = 2; // block information + required bool replicaFound = 1; + + // The following entries are not set if there was no replica found. + optional ReplicaStateProto state = 2; // State of the replica + optional BlockProto block = 3; // block information } /** diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestInterDatanodeProtocol.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestInterDatanodeProtocol.java index 599521f1793..c1167a4094a 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestInterDatanodeProtocol.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestInterDatanodeProtocol.java @@ -17,8 +17,7 @@ */ package org.apache.hadoop.hdfs.server.datanode.fsdataset.impl; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; +import static org.junit.Assert.*; import java.io.IOException; import java.net.InetSocketAddress; @@ -172,6 +171,13 @@ public class TestInterDatanodeProtocol { b.getBlockId(), b.getNumBytes()/2, b.getGenerationStamp()+1); idp.updateReplicaUnderRecovery(b, recoveryId, newblock.getNumBytes()); checkMetaInfo(newblock, datanode); + + // Verify correct null response trying to init recovery for a missing block + ExtendedBlock badBlock = new ExtendedBlock("fake-pool", + b.getBlockId(), 0, 0); + assertNull(idp.initReplicaRecovery( + new RecoveringBlock(badBlock, + locatedblock.getLocations(), recoveryId))); } finally { if (cluster != null) {cluster.shutdown();}