HDFS-8071. Redundant checkFileProgress() in PART II of getAdditionalBlock(). Contributed by Konstantin Shvachko.

This commit is contained in:
Konstantin V Shvachko 2015-04-06 16:52:52 -07:00
parent 3fb5abfc87
commit 75c5454860
3 changed files with 34 additions and 21 deletions

View File

@ -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.

View File

@ -3032,6 +3032,10 @@ LocatedBlock getAdditionalBlock(String src, long fileId, String clientName,
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 @@ FileState analyzeFileState(String src,
"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 @@ private Block createNewBlock() throws IOException {
* 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 });
}
}

View File

@ -24,6 +24,7 @@
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 void tearDown() throws Exception {
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 DatanodeStorageInfo[] answer(InvocationOnMock invocation)
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 DatanodeStorageInfo[] answer(InvocationOnMock invocation)
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