Repository: hadoop Updated Branches: refs/heads/trunk 3fb5abfc8 -> 75c545486
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/75c54548 Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/75c54548 Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/75c54548 Branch: refs/heads/trunk Commit: 75c545486080042952c775f7964212c15ce65f73 Parents: 3fb5abf Author: Konstantin V Shvachko <[email protected]> Authored: Mon Apr 6 16:52:52 2015 -0700 Committer: Konstantin V Shvachko <[email protected]> Committed: Mon Apr 6 22:20:44 2015 -0700 ---------------------------------------------------------------------- hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt | 3 ++ .../hdfs/server/namenode/FSNamesystem.java | 35 +++++++++----------- .../hdfs/server/namenode/TestAddBlockRetry.java | 17 +++++++++- 3 files changed, 34 insertions(+), 21 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/75c54548/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 52325a2..7d20060 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -880,6 +880,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/75c54548/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 a77c382..62d5f67 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); } @@ -3550,21 +3549,17 @@ 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) { - return blockManager.checkBlocksProperlyReplicated(src, v - .getBlocks()); - } else { - // check the penultimate block of this file - BlockInfoContiguous b = v.getPenultimateBlock(); - return b == null || - blockManager.checkBlocksProperlyReplicated( - src, new BlockInfoContiguous[] { b }); - } - } finally { - readUnlock(); + boolean checkFileProgress(String src, INodeFile v, boolean checkall) { + assert hasReadLock(); + if (checkall) { + return blockManager.checkBlocksProperlyReplicated(src, v + .getBlocks()); + } else { + // check the penultimate block of this file + BlockInfoContiguous b = v.getPenultimateBlock(); + return b == null || + blockManager.checkBlocksProperlyReplicated( + src, new BlockInfoContiguous[] { b }); } } http://git-wip-us.apache.org/repos/asf/hadoop/blob/75c54548/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
