Repository: hadoop Updated Branches: refs/heads/branch-2.7 64cf07985 -> 2cb9dac9a
HDFS-8071. Redundant checkFileProgress() in PART II of getAdditionalBlock(). Contributed by Konstantin Shvachko. Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/2cb9dac9 Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/2cb9dac9 Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/2cb9dac9 Branch: refs/heads/branch-2.7 Commit: 2cb9dac9a30f67bc8d7749dc05bdd58ce3dff6a4 Parents: 64cf079 Author: Konstantin V Shvachko <s...@apache.org> Authored: Mon Apr 6 16:52:52 2015 -0700 Committer: Konstantin V Shvachko <s...@apache.org> Committed: Mon Apr 6 23:35:01 2015 -0700 ---------------------------------------------------------------------- hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt | 3 ++ .../hdfs/server/namenode/FSNamesystem.java | 40 +++++++++----------- .../hdfs/server/namenode/TestAddBlockRetry.java | 17 ++++++++- 3 files changed, 36 insertions(+), 24 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/2cb9dac9/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index 0d48959..657de32 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -465,6 +465,9 @@ Release 2.7.0 - UNRELEASED HDFS-7811. Avoid recursive call getStoragePolicyID in INodeFile#computeQuotaUsage. (Xiaoyu Yao and jing9) + HDFS-8071. Redundant checkFileProgress() in PART II of getAdditionalBlock(). + (shv) + OPTIMIZATIONS HDFS-7454. Reduce memory footprint for AclEntries in NameNode. http://git-wip-us.apache.org/repos/asf/hadoop/blob/2cb9dac9/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java index d1954f8..f3ae849 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java @@ -3032,6 +3032,10 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, FileState fileState = analyzeFileState( src, fileId, clientName, previous, onRetryBlock); final INodeFile pendingFile = fileState.inode; + // Check if the penultimate block is minimally replicated + if (!checkFileProgress(src, pendingFile, false)) { + throw new NotReplicatedYetException("Not replicated yet: " + src); + } src = fileState.path; if (onRetryBlock[0] != null && onRetryBlock[0].getLocations().length > 0) { @@ -3244,11 +3248,6 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, "last block in file " + lastBlockInFile); } } - - // Check if the penultimate block is minimally replicated - if (!checkFileProgress(src, pendingFile, false)) { - throw new NotReplicatedYetException("Not replicated yet: " + src); - } return new FileState(pendingFile, src, iip); } @@ -3551,28 +3550,23 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, * replicated. If not, return false. If checkall is true, then check * all blocks, otherwise check only penultimate block. */ - private boolean checkFileProgress(String src, INodeFile v, boolean checkall) { - readLock(); - try { - if (checkall) { - // check all blocks of the file. - for (BlockInfoContiguous block: v.getBlocks()) { - if (!isCompleteBlock(src, block, blockManager.minReplication)) { - return false; - } - } - } else { - // check the penultimate block of this file - BlockInfoContiguous b = v.getPenultimateBlock(); - if (b != null - && !isCompleteBlock(src, b, blockManager.minReplication)) { + boolean checkFileProgress(String src, INodeFile v, boolean checkall) { + if (checkall) { + // check all blocks of the file. + for (BlockInfoContiguous block: v.getBlocks()) { + if (!isCompleteBlock(src, block, blockManager.minReplication)) { return false; } } - return true; - } finally { - readUnlock(); + } else { + // check the penultimate block of this file + BlockInfoContiguous b = v.getPenultimateBlock(); + if (b != null + && !isCompleteBlock(src, b, blockManager.minReplication)) { + return false; + } } + return true; } private static boolean isCompleteBlock(String src, BlockInfoContiguous b, int minRepl) { http://git-wip-us.apache.org/repos/asf/hadoop/blob/2cb9dac9/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAddBlockRetry.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAddBlockRetry.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAddBlockRetry.java index cf37a54..671f61d 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAddBlockRetry.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAddBlockRetry.java @@ -24,6 +24,7 @@ import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.spy; +import java.io.IOException; import java.lang.reflect.Field; import java.util.EnumSet; import java.util.HashSet; @@ -90,7 +91,7 @@ public class TestAddBlockRetry { public void testRetryAddBlockWhileInChooseTarget() throws Exception { final String src = "/testRetryAddBlockWhileInChooseTarget"; - FSNamesystem ns = cluster.getNamesystem(); + final FSNamesystem ns = cluster.getNamesystem(); BlockManager spyBM = spy(ns.getBlockManager()); final NamenodeProtocols nn = cluster.getNameNodeRpc(); @@ -107,11 +108,15 @@ public class TestAddBlockRetry { LOG.info("chooseTarget for " + src); DatanodeStorageInfo[] ret = (DatanodeStorageInfo[]) invocation.callRealMethod(); + assertTrue("Penultimate block must be complete", + checkFileProgress(src, false)); count++; if(count == 1) { // run second addBlock() LOG.info("Starting second addBlock for " + src); nn.addBlock(src, "clientName", null, null, INodeId.GRANDFATHER_INODE_ID, null); + assertTrue("Penultimate block must be complete", + checkFileProgress(src, false)); LocatedBlocks lbs = nn.getBlockLocations(src, 0, Long.MAX_VALUE); assertEquals("Must be one block", 1, lbs.getLocatedBlocks().size()); lb2 = lbs.get(0); @@ -142,6 +147,16 @@ public class TestAddBlockRetry { assertEquals("Blocks are not equal", lb1.getBlock(), lb2.getBlock()); } + boolean checkFileProgress(String src, boolean checkall) throws IOException { + final FSNamesystem ns = cluster.getNamesystem(); + ns.readLock(); + try { + return ns.checkFileProgress(src, ns.dir.getINode(src).asFile(), checkall); + } finally { + ns.readUnlock(); + } + } + /* * Since NameNode will not persist any locations of the block, addBlock() * retry call after restart NN should re-select the locations and return to
