HDFS-9754. Avoid unnecessary getBlockCollection calls in BlockManager. Contributed by Jing Zhao.

(cherry picked from commit 972782d956)
This commit is contained in:
Jing Zhao 2016-02-12 11:07:52 -08:00
parent f501c6dc49
commit f8c9c0ff0e
11 changed files with 65 additions and 76 deletions

View File

@ -67,6 +67,9 @@ Release 2.9.0 - UNRELEASED
HDFS-9780. RollingFileSystemSink doesn't work on secure clusters.
(Daniel Templeton via kasha)
HDFS-9754. Avoid unnecessary getBlockCollection calls in BlockManager.
(jing9)
OPTIMIZATIONS
BUG FIXES

View File

@ -115,6 +115,10 @@ public abstract class BlockInfo extends Block
this.bcId = id;
}
public void delete() {
setBlockCollectionId(INVALID_INODE_ID);
}
public boolean isDeleted() {
return bcId == INVALID_INODE_ID;
}
@ -354,6 +358,12 @@ public abstract class BlockInfo extends Block
return getBlockUCState().equals(BlockUCState.COMPLETE);
}
public final boolean isCompleteOrCommitted() {
final BlockUCState state = getBlockUCState();
return state.equals(BlockUCState.COMPLETE) ||
state.equals(BlockUCState.COMMITTED);
}
/**
* Add/Update the under construction feature.
*/

View File

@ -674,14 +674,14 @@ public class BlockManager implements BlockStatsMXBean {
if(lastBlock.isComplete())
return false; // already completed (e.g. by syncBlock)
final boolean b = commitBlock(lastBlock, commitBlock);
final boolean committed = commitBlock(lastBlock, commitBlock);
if (countNodes(lastBlock).liveReplicas() >= minReplication) {
if (b) {
addExpectedReplicasToPending(lastBlock, bc);
if (committed) {
addExpectedReplicasToPending(lastBlock);
}
completeBlock(lastBlock, false);
}
return b;
return committed;
}
/**
@ -689,24 +689,20 @@ public class BlockManager implements BlockStatsMXBean {
* pendingReplications in order to keep ReplicationMonitor from scheduling
* the block.
*/
public void addExpectedReplicasToPending(BlockInfo blk, BlockCollection bc) {
addExpectedReplicasToPending(blk);
}
private void addExpectedReplicasToPending(BlockInfo lastBlock) {
public void addExpectedReplicasToPending(BlockInfo blk) {
DatanodeStorageInfo[] expectedStorages =
lastBlock.getUnderConstructionFeature().getExpectedStorageLocations();
if (expectedStorages.length - lastBlock.numNodes() > 0) {
blk.getUnderConstructionFeature().getExpectedStorageLocations();
if (expectedStorages.length - blk.numNodes() > 0) {
ArrayList<DatanodeDescriptor> pendingNodes =
new ArrayList<DatanodeDescriptor>();
for (DatanodeStorageInfo storage : expectedStorages) {
DatanodeDescriptor dnd = storage.getDatanodeDescriptor();
if (lastBlock.findStorageInfo(dnd) == null) {
if (blk.findStorageInfo(dnd) == null) {
pendingNodes.add(dnd);
}
pendingReplications.increment(blk,
pendingNodes.toArray(new DatanodeDescriptor[pendingNodes.size()]));
}
pendingReplications.increment(lastBlock,
pendingNodes.toArray(new DatanodeDescriptor[pendingNodes.size()]));
}
}
@ -886,7 +882,7 @@ public class BlockManager implements BlockStatsMXBean {
if (!blk.isComplete()) {
final DatanodeStorageInfo[] storages = blk.getUnderConstructionFeature()
.getExpectedStorageLocations();
final ExtendedBlock eb = new ExtendedBlock(namesystem.getBlockPoolId(), blk);
final ExtendedBlock eb = new ExtendedBlock(getBlockPoolId(), blk);
return newLocatedBlock(eb, storages, pos, false);
}
@ -918,7 +914,7 @@ public class BlockManager implements BlockStatsMXBean {
" numNodes: " + numNodes +
" numCorrupt: " + numCorruptNodes +
" numCorruptRepls: " + numCorruptReplicas;
final ExtendedBlock eb = new ExtendedBlock(namesystem.getBlockPoolId(), blk);
final ExtendedBlock eb = new ExtendedBlock(getBlockPoolId(), blk);
return newLocatedBlock(eb, machines, pos, isCorrupt);
}
@ -1430,11 +1426,8 @@ public class BlockManager implements BlockStatsMXBean {
}
private ReplicationWork scheduleReplication(BlockInfo block, int priority) {
// block should belong to a file
BlockCollection bc = getBlockCollection(block);
// abandoned block or block reopened for append
if (bc == null
|| (bc.isUnderConstruction() && block.equals(bc.getLastBlock()))) {
// skip abandoned block or block reopened for append
if (block.isDeleted() || !block.isCompleteOrCommitted()) {
// remove from neededReplications
neededReplications.remove(block, priority);
return null;
@ -1473,6 +1466,7 @@ public class BlockManager implements BlockStatsMXBean {
} else {
additionalReplRequired = 1; // Needed on a new rack
}
final BlockCollection bc = getBlockCollection(block);
return new ReplicationWork(block, bc, srcNode, containingNodes,
liveReplicaNodes, additionalReplRequired, priority);
}
@ -1481,11 +1475,8 @@ public class BlockManager implements BlockStatsMXBean {
BlockInfo block = rw.getBlock();
int priority = rw.getPriority();
// Recheck since global lock was released
// block should belong to a file
BlockCollection bc = getBlockCollection(block);
// abandoned block or block reopened for append
if (bc == null
|| (bc.isUnderConstruction() && block.equals(bc.getLastBlock()))) {
// skip abandoned block or block reopened for append
if (block.isDeleted() || !block.isCompleteOrCommitted()) {
neededReplications.remove(block, priority);
rw.resetTargets();
return false;
@ -2630,8 +2621,6 @@ public class BlockManager implements BlockStatsMXBean {
// it will happen in next block report otherwise.
return block;
}
BlockCollection bc = getBlockCollection(storedBlock);
assert bc != null : "Block must belong to a file";
// add block to the datanode
AddBlockResult result = storageInfo.addBlock(storedBlock);
@ -2667,7 +2656,7 @@ public class BlockManager implements BlockStatsMXBean {
if(storedBlock.getBlockUCState() == BlockUCState.COMMITTED &&
numLiveReplicas >= minReplication) {
addExpectedReplicasToPending(storedBlock, bc);
addExpectedReplicasToPending(storedBlock);
completeBlock(storedBlock, false);
} else if (storedBlock.isComplete() && result == AddBlockResult.ADDED) {
// check whether safe replication is reached for the block
@ -2678,8 +2667,8 @@ public class BlockManager implements BlockStatsMXBean {
bmSafeMode.incrementSafeBlockCount(numCurrentReplica);
}
// if file is under construction, then done for now
if (bc.isUnderConstruction()) {
// if block is still under construction, then done for now
if (!storedBlock.isCompleteOrCommitted()) {
return storedBlock;
}
@ -3090,8 +3079,7 @@ public class BlockManager implements BlockStatsMXBean {
// necessary. In that case, put block on a possibly-will-
// be-replicated list.
//
BlockCollection bc = getBlockCollection(storedBlock);
if (bc != null) {
if (!storedBlock.isDeleted()) {
bmSafeMode.decrementSafeBlockCount(storedBlock);
updateNeededReplications(storedBlock, -1, 0);
}

View File

@ -114,7 +114,7 @@ class BlocksMap {
if (blockInfo == null)
return;
blockInfo.setBlockCollectionId(INodeId.INVALID_INODE_ID);
assert blockInfo.getBlockCollectionId() == INodeId.INVALID_INODE_ID;
for(int idx = blockInfo.numNodes()-1; idx >= 0; idx--) {
DatanodeDescriptor dn = blockInfo.getDatanode(idx);
if (dn != null) {

View File

@ -18,6 +18,7 @@
package org.apache.hadoop.hdfs.server.namenode;
import java.io.IOException;
import java.util.Set;
import org.apache.hadoop.HadoopIllegalArgumentException;
import org.apache.hadoop.fs.UnresolvedLinkException;
@ -179,6 +180,7 @@ final class FSDirTruncateOp {
"Should be the same block.";
if (oldBlock.getBlockId() != tBlk.getBlockId()
&& !file.isBlockInLatestSnapshot(oldBlock)) {
oldBlock.delete();
fsd.getBlockManager().removeBlockFromMap(oldBlock);
}
}
@ -289,9 +291,9 @@ final class FSDirTruncateOp {
verifyQuotaForTruncate(fsn, iip, file, newLength, delta);
long remainingLength =
file.collectBlocksBeyondMax(newLength, collectedBlocks);
file.excludeSnapshotBlocks(latestSnapshot, collectedBlocks);
Set<BlockInfo> toRetain = file.getSnapshotBlocksToRetain(latestSnapshot);
long remainingLength = file.collectBlocksBeyondMax(newLength,
collectedBlocks, toRetain);
file.setModificationTime(mtime);
// return whether on a block boundary
return (remainingLength - newLength) == 0;

View File

@ -3147,7 +3147,7 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
final BlockInfo b = blocks[i];
if (b != null && b.getBlockUCState() == BlockUCState.COMMITTED) {
// b is COMMITTED but not yet COMPLETE, add it to pending replication.
blockManager.addExpectedReplicasToPending(b, pendingFile);
blockManager.addExpectedReplicasToPending(b);
}
}
}
@ -4269,9 +4269,7 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
return new PermissionStatus(fsOwner.getShortUserName(), supergroup, permission);
}
@Override
public void checkSuperuserPrivilege()
throws AccessControlException {
void checkSuperuserPrivilege() throws AccessControlException {
if (isPermissionEnabled) {
FSPermissionChecker pc = getPermissionChecker();
pc.checkSuperuserPrivilege();

View File

@ -1011,14 +1011,10 @@ public abstract class INode implements INodeAttributes, Diff.Element<byte[]> {
*/
public void addDeleteBlock(BlockInfo toDelete) {
assert toDelete != null : "toDelete is null";
toDelete.delete();
toDeleteList.add(toDelete);
}
public void removeDeleteBlock(BlockInfo block) {
assert block != null : "block is null";
toDeleteList.remove(block);
}
public void addUpdateReplicationFactor(BlockInfo block, short targetRepl) {
toUpdateReplicationInfo.add(
new UpdatedReplicationInfo(targetRepl, block));

View File

@ -25,6 +25,7 @@ import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.PrintWriter;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
@ -291,12 +292,13 @@ public class INodeFile extends INodeWithAdditionalFields
return null;
}
BlockInfo ucBlock = blocks[size_1];
BlockInfo lastBlock = blocks[size_1];
//copy to a new list
BlockInfo[] newlist = new BlockInfo[size_1];
System.arraycopy(blocks, 0, newlist, 0, size_1);
setBlocks(newlist);
return ucBlock;
lastBlock.delete();
return lastBlock;
}
/* End of Under-Construction Feature */
@ -572,7 +574,6 @@ public class INodeFile extends INodeWithAdditionalFields
if (blocks != null && reclaimContext.collectedBlocks != null) {
for (BlockInfo blk : blocks) {
reclaimContext.collectedBlocks.addDeleteBlock(blk);
blk.setBlockCollectionId(INodeId.INVALID_INODE_ID);
}
}
clearBlocks();
@ -802,7 +803,7 @@ public class INodeFile extends INodeWithAdditionalFields
* @return sum of sizes of the remained blocks
*/
public long collectBlocksBeyondMax(final long max,
final BlocksMapUpdateInfo collectedBlocks) {
final BlocksMapUpdateInfo collectedBlocks, Set<BlockInfo> toRetain) {
final BlockInfo[] oldBlocks = getBlocks();
if (oldBlocks == null) {
return 0;
@ -824,7 +825,10 @@ public class INodeFile extends INodeWithAdditionalFields
// collect the blocks beyond max
if (collectedBlocks != null) {
for(; n < oldBlocks.length; n++) {
collectedBlocks.addDeleteBlock(oldBlocks[n]);
final BlockInfo del = oldBlocks[n];
if (toRetain == null || !toRetain.contains(del)) {
collectedBlocks.addDeleteBlock(del);
}
}
}
return size;
@ -915,22 +919,18 @@ public class INodeFile extends INodeWithAdditionalFields
}
/** Exclude blocks collected for deletion that belong to a snapshot. */
void excludeSnapshotBlocks(int snapshotId,
BlocksMapUpdateInfo collectedBlocks) {
if(collectedBlocks == null || collectedBlocks.getToDeleteList().isEmpty())
return;
Set<BlockInfo> getSnapshotBlocksToRetain(int snapshotId) {
FileWithSnapshotFeature sf = getFileWithSnapshotFeature();
if(sf == null)
return;
BlockInfo[] snapshotBlocks =
getDiffs().findEarlierSnapshotBlocks(snapshotId);
if(snapshotBlocks == null)
return;
List<BlockInfo> toDelete = collectedBlocks.getToDeleteList();
for(BlockInfo blk : snapshotBlocks) {
if(toDelete.contains(blk))
collectedBlocks.removeDeleteBlock(blk);
if(sf == null) {
return null;
}
BlockInfo[] snapshotBlocks = getDiffs().findEarlierSnapshotBlocks(snapshotId);
if(snapshotBlocks == null) {
return null;
}
Set<BlockInfo> toRetain = new HashSet<>(snapshotBlocks.length);
Collections.addAll(toRetain, snapshotBlocks);
return toRetain;
}
/**

View File

@ -18,13 +18,9 @@
package org.apache.hadoop.hdfs.server.namenode;
import org.apache.hadoop.classification.InterfaceAudience;
import org.apache.hadoop.hdfs.protocol.Block;
import org.apache.hadoop.hdfs.server.blockmanagement.BlockCollection;
import org.apache.hadoop.hdfs.server.namenode.NameNode.OperationCategory;
import org.apache.hadoop.hdfs.server.namenode.ha.HAContext;
import org.apache.hadoop.hdfs.util.RwLock;
import org.apache.hadoop.ipc.StandbyException;
import org.apache.hadoop.security.AccessControlException;
/** Namesystem operations. */
@InterfaceAudience.Private
@ -32,12 +28,6 @@ public interface Namesystem extends RwLock, SafeMode {
/** Is this name system running? */
boolean isRunning();
/** Check if the user has superuser privilege. */
void checkSuperuserPrivilege() throws AccessControlException;
/** @return the block pool ID */
String getBlockPoolId();
BlockCollection getBlockCollection(long id);
void startSecretManagerIfNecessary();

View File

@ -220,7 +220,7 @@ public class FileWithSnapshotFeature implements INode.Feature {
FileDiff last = diffs.getLast();
BlockInfo[] snapshotBlocks = last == null ? null : last.getBlocks();
if(snapshotBlocks == null)
file.collectBlocksBeyondMax(max, reclaimContext.collectedBlocks());
file.collectBlocksBeyondMax(max, reclaimContext.collectedBlocks(), null);
else
file.collectBlocksBeyondSnapshot(snapshotBlocks,
reclaimContext.collectedBlocks());

View File

@ -95,6 +95,7 @@ public class TestNameNodeMetadataConsistency {
cluster.getNameNode().getNamesystem().writeLock();
BlockInfo bInfo = cluster.getNameNode().getNamesystem().getBlockManager()
.getStoredBlock(block.getLocalBlock());
bInfo.delete();
cluster.getNameNode().getNamesystem().getBlockManager()
.removeBlock(bInfo);
cluster.getNameNode().getNamesystem().writeUnlock();
@ -146,6 +147,7 @@ public class TestNameNodeMetadataConsistency {
BlockInfo bInfo = cluster.getNameNode().getNamesystem().getBlockManager
().getStoredBlock(block.getLocalBlock());
cluster.getNameNode().getNamesystem().writeLock();
bInfo.delete();
cluster.getNameNode().getNamesystem().getBlockManager()
.removeBlock(bInfo);
cluster.getNameNode().getNamesystem().writeUnlock();