From 2f3faf39efa88d0023e1f470b14817549ca94944 Mon Sep 17 00:00:00 2001 From: Haohui Mai Date: Mon, 24 Aug 2015 14:44:08 -0700 Subject: [PATCH] HDFS-8248. Store INodeId instead of the INodeFile object in BlockInfoContiguous. Contributed by Haohui Mai. --- hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt | 3 ++ .../blockmanagement/BlockCollection.java | 5 ++ .../server/blockmanagement/BlockInfo.java | 25 +++++---- .../server/blockmanagement/BlockManager.java | 19 +++---- .../server/blockmanagement/BlocksMap.java | 10 ++-- .../blockmanagement/DecommissionManager.java | 7 ++- .../SequentialBlockIdGenerator.java | 5 +- .../hdfs/server/namenode/FSNamesystem.java | 24 ++++++--- .../hdfs/server/namenode/INodeFile.java | 8 +-- .../hadoop/hdfs/server/namenode/INodeId.java | 1 + .../hdfs/server/namenode/Namesystem.java | 3 ++ .../server/blockmanagement/TestBlockInfo.java | 11 ++-- .../blockmanagement/TestBlockManager.java | 15 ++++++ .../TestReplicationPolicy.java | 10 +++- .../TestCommitBlockSynchronization.java | 9 ++-- .../hadoop/hdfs/server/namenode/TestFsck.java | 54 +++++++++++++------ .../snapshot/TestSnapshotBlocksMap.java | 30 +++-------- .../snapshot/TestSnapshotDeletion.java | 9 ++-- 18 files changed, 154 insertions(+), 94 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index 001ca187835..08678069f78 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -499,6 +499,9 @@ Release 2.8.0 - UNRELEASED HDFS-8896. DataNode object isn't GCed when shutdown, because it has GC root in ShutdownHookManager. (Walter Su via jing9) + HDFS-8248. Store INodeId instead of the INodeFile object in + BlockInfoContiguous. (wheat9) + OPTIMIZATIONS HDFS-8026. Trace FSOutputSummer#writeChecksumChunks rather than diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockCollection.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockCollection.java index 3952cc6a981..95d99838a2f 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockCollection.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockCollection.java @@ -79,4 +79,9 @@ public interface BlockCollection { * @return whether the block collection is under construction. */ public boolean isUnderConstruction(); + + /** + * @return the id for the block collection + */ + long getId(); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfo.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfo.java index ad1813d9f90..17e115669a1 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfo.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfo.java @@ -28,6 +28,8 @@ import org.apache.hadoop.hdfs.server.common.HdfsServerConstants.BlockUCState; import org.apache.hadoop.hdfs.server.namenode.NameNode; import org.apache.hadoop.util.LightWeightGSet; +import static org.apache.hadoop.hdfs.server.namenode.INodeId.INVALID_INODE_ID; + /** * BlockInfo class maintains for a given block * the {@link INodeFile} it is part of and datanodes where the replicas of @@ -43,11 +45,14 @@ public abstract class BlockInfo extends Block public static final BlockInfo[] EMPTY_ARRAY = {}; /** - * Replication factor + * Replication factor. */ private short replication; - private BlockCollection bc; + /** + * Block collection ID. + */ + private long bcId; /** For implementing {@link LightWeightGSet.LinkedElement} interface. */ private LightWeightGSet.LinkedElement nextLinkedElement; @@ -74,14 +79,14 @@ public abstract class BlockInfo extends Block */ public BlockInfo(short replication) { this.triplets = new Object[3*replication]; - this.bc = null; + this.bcId = INVALID_INODE_ID; this.replication = replication; } public BlockInfo(Block blk, short replication) { super(blk); this.triplets = new Object[3*replication]; - this.bc = null; + this.bcId = INVALID_INODE_ID; this.replication = replication; } @@ -91,7 +96,7 @@ public abstract class BlockInfo extends Block */ protected BlockInfo(BlockInfo from) { this(from, from.getReplication()); - this.bc = from.bc; + this.bcId = from.bcId; } public short getReplication() { @@ -102,16 +107,16 @@ public abstract class BlockInfo extends Block this.replication = repl; } - public BlockCollection getBlockCollection() { - return bc; + public long getBlockCollectionId() { + return bcId; } - public void setBlockCollection(BlockCollection bc) { - this.bc = bc; + public void setBlockCollectionId(long id) { + this.bcId = id; } public boolean isDeleted() { - return (bc == null); + return bcId == INVALID_INODE_ID; } public DatanodeDescriptor getDatanode(int index) { diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java index c45ecc12970..e60ffde70bd 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java @@ -554,7 +554,7 @@ public class BlockManager implements BlockStatsMXBean { numReplicas.decommissionedAndDecommissioning(); if (block instanceof BlockInfo) { - BlockCollection bc = ((BlockInfo) block).getBlockCollection(); + BlockCollection bc = getBlockCollection((BlockInfo)block); String fileName = (bc == null) ? "[orphaned]" : bc.getName(); out.print(fileName + ": "); } @@ -1344,7 +1344,7 @@ public class BlockManager implements BlockStatsMXBean { for (int priority = 0; priority < blocksToReplicate.size(); priority++) { for (BlockInfo block : blocksToReplicate.get(priority)) { // block should belong to a file - bc = blocksMap.getBlockCollection(block); + bc = getBlockCollection(block); // abandoned block or block reopened for append if(bc == null || (bc.isUnderConstruction() && block.equals(bc.getLastBlock()))) { neededReplications.remove(block, priority); // remove from neededReplications @@ -1428,7 +1428,7 @@ public class BlockManager implements BlockStatsMXBean { int priority = rw.priority; // Recheck since global lock was released // block should belong to a file - bc = blocksMap.getBlockCollection(block); + bc = getBlockCollection(block); // abandoned block or block reopened for append if(bc == null || (bc.isUnderConstruction() && block.equals(bc.getLastBlock()))) { neededReplications.remove(block, priority); // remove from neededReplications @@ -2531,7 +2531,7 @@ public class BlockManager implements BlockStatsMXBean { int numCurrentReplica = countLiveNodes(storedBlock); if (storedBlock.getBlockUCState() == BlockUCState.COMMITTED && numCurrentReplica >= minReplication) { - completeBlock(storedBlock.getBlockCollection(), storedBlock, false); + completeBlock(getBlockCollection(storedBlock), storedBlock, false); } else if (storedBlock.isComplete() && result == AddBlockResult.ADDED) { // check whether safe replication is reached for the block // only complete blocks are counted towards that. @@ -2569,7 +2569,7 @@ public class BlockManager implements BlockStatsMXBean { // it will happen in next block report otherwise. return block; } - BlockCollection bc = storedBlock.getBlockCollection(); + BlockCollection bc = getBlockCollection(storedBlock); assert bc != null : "Block must belong to a file"; // add block to the datanode @@ -2964,7 +2964,8 @@ public class BlockManager implements BlockStatsMXBean { BlockPlacementPolicy replicator) { assert namesystem.hasWriteLock(); // first form a rack to datanodes map and - BlockCollection bc = getBlockCollection(b); + BlockInfo bi = getStoredBlock(b); + BlockCollection bc = getBlockCollection(bi); final BlockStoragePolicy storagePolicy = storagePolicySuite.getPolicy(bc.getStoragePolicyID()); final List excessTypes = storagePolicy.chooseExcess( replication, DatanodeStorageInfo.toStorageTypes(nonExcess)); @@ -3101,7 +3102,7 @@ public class BlockManager implements BlockStatsMXBean { // necessary. In that case, put block on a possibly-will- // be-replicated list. // - BlockCollection bc = blocksMap.getBlockCollection(block); + BlockCollection bc = getBlockCollection(storedBlock); if (bc != null) { namesystem.decrementSafeBlockCount(storedBlock); updateNeededReplications(storedBlock, -1, 0); @@ -3617,8 +3618,8 @@ public class BlockManager implements BlockStatsMXBean { return blocksMap.addBlockCollection(block, bc); } - public BlockCollection getBlockCollection(Block b) { - return blocksMap.getBlockCollection(b); + public BlockCollection getBlockCollection(BlockInfo b) { + return namesystem.getBlockCollection(b.getBlockCollectionId()); } /** @return an iterator of the datanodes. */ diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlocksMap.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlocksMap.java index 0dbf4858b7c..33c68f39461 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlocksMap.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlocksMap.java @@ -20,6 +20,7 @@ package org.apache.hadoop.hdfs.server.blockmanagement; import java.util.Iterator; import org.apache.hadoop.hdfs.protocol.Block; +import org.apache.hadoop.hdfs.server.namenode.INodeId; import org.apache.hadoop.hdfs.server.protocol.DatanodeStorage; import org.apache.hadoop.util.GSet; import org.apache.hadoop.util.LightWeightGSet; @@ -94,11 +95,6 @@ class BlocksMap { } } - BlockCollection getBlockCollection(Block b) { - BlockInfo info = blocks.get(b); - return (info != null) ? info.getBlockCollection() : null; - } - /** * Add block b belonging to the specified block collection to the map. */ @@ -108,7 +104,7 @@ class BlocksMap { info = b; blocks.put(info); } - info.setBlockCollection(bc); + info.setBlockCollectionId(bc.getId()); return info; } @@ -122,7 +118,7 @@ class BlocksMap { if (blockInfo == null) return; - blockInfo.setBlockCollection(null); + blockInfo.setBlockCollectionId(INodeId.INVALID_INODE_ID); for(int idx = blockInfo.numNodes()-1; idx >= 0; idx--) { DatanodeDescriptor dn = blockInfo.getDatanode(idx); dn.removeBlock(blockInfo); // remove from the list and wipe the location diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DecommissionManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DecommissionManager.java index 755be79d215..f75f4b12726 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DecommissionManager.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DecommissionManager.java @@ -37,6 +37,7 @@ import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hdfs.DFSConfigKeys; import org.apache.hadoop.hdfs.protocol.DatanodeID; +import org.apache.hadoop.hdfs.server.namenode.INodeId; import org.apache.hadoop.hdfs.server.namenode.Namesystem; import org.apache.hadoop.hdfs.util.CyclicIteration; import org.apache.hadoop.util.ChunkedArrayList; @@ -552,12 +553,14 @@ public class DecommissionManager { it.remove(); continue; } - BlockCollection bc = blockManager.blocksMap.getBlockCollection(block); - if (bc == null) { + + long bcId = block.getBlockCollectionId(); + if (bcId == INodeId.INVALID_INODE_ID) { // Orphan block, will be invalidated eventually. Skip. continue; } + BlockCollection bc = namesystem.getBlockCollection(bcId); final NumberReplicas num = blockManager.countNodes(block); final int liveReplicas = num.liveReplicas(); final int curReplicas = liveReplicas; diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/SequentialBlockIdGenerator.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/SequentialBlockIdGenerator.java index eef8857b042..f053b7b5418 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/SequentialBlockIdGenerator.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/SequentialBlockIdGenerator.java @@ -20,6 +20,7 @@ package org.apache.hadoop.hdfs.server.blockmanagement; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.hdfs.protocol.Block; import org.apache.hadoop.hdfs.server.blockmanagement.BlockManager; +import org.apache.hadoop.hdfs.server.namenode.INodeId; import org.apache.hadoop.util.SequentialNumber; /** @@ -61,6 +62,8 @@ public class SequentialBlockIdGenerator extends SequentialNumber { * Returns whether the given block is one pointed-to by a file. */ private boolean isValidBlock(Block b) { - return (blockManager.getBlockCollection(b) != null); + BlockInfo bi = blockManager.getStoredBlock(b); + return bi != null && bi.getBlockCollectionId() != + INodeId.INVALID_INODE_ID; } } 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 883ed0801ce..5ae5e8a46aa 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 @@ -3206,7 +3206,7 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, @Override public boolean isInSnapshot(BlockInfo blockUC) { assert hasReadLock(); - final BlockCollection bc = blockUC.getBlockCollection(); + final BlockCollection bc = blockManager.getBlockCollection(blockUC); if (bc == null || !(bc instanceof INodeFile) || !bc.isUnderConstruction()) { return false; @@ -3235,6 +3235,12 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, return true; } + @Override + public BlockCollection getBlockCollection(long id) { + INode inode = getFSDirectory().getInode(id); + return inode == null ? null : inode.asFile(); + } + void commitBlockSynchronization(ExtendedBlock oldBlock, long newgenerationstamp, long newlength, boolean closeFile, boolean deleteblock, DatanodeID[] newtargets, @@ -3292,7 +3298,8 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, + " is null, likely because the file owning this block was" + " deleted and the block removal is delayed"); } - INodeFile iFile = ((INode)storedBlock.getBlockCollection()).asFile(); + long bcId = storedBlock.getBlockCollectionId(); + INodeFile iFile = ((INode)getBlockCollection(bcId)).asFile(); if (isFileDeleted(iFile)) { throw new FileNotFoundException("File not found: " + iFile.getFullPathName() + ", likely due to delayed block" @@ -3714,9 +3721,10 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, while (it.hasNext()) { Block b = it.next(); BlockInfo blockInfo = blockManager.getStoredBlock(b); - if (blockInfo.getBlockCollection().getStoragePolicyID() - == lpPolicy.getId()) { - filesToDelete.add(blockInfo.getBlockCollection()); + BlockCollection bc = getBlockCollection( + blockInfo.getBlockCollectionId()); + if (bc.getStoragePolicyID() == lpPolicy.getId()) { + filesToDelete.add(bc); } } @@ -5229,7 +5237,8 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, } // check file inode - final INodeFile file = ((INode)storedBlock.getBlockCollection()).asFile(); + long bcId = storedBlock.getBlockCollectionId(); + final INodeFile file = ((INode)getBlockCollection(bcId)).asFile(); if (file == null || !file.isUnderConstruction() || isFileDeleted(file)) { throw new IOException("The file " + storedBlock + " belonged to does not exist or it is not under construction."); @@ -5492,7 +5501,8 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, while (blkIterator.hasNext()) { BlockInfo blk = blkIterator.next(); - final INode inode = (INode)blockManager.getBlockCollection(blk); + BlockCollection bc = getBlockCollection(blk.getBlockCollectionId()); + final INode inode = (INode)bc; skip++; if (inode != null && blockManager.countNodes(blk).liveReplicas() == 0) { String src = FSDirectory.getFullPathName(inode); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java index cd80b763c55..4e75a3d6eb5 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java @@ -89,7 +89,7 @@ public class INodeFile extends INodeWithAdditionalFields private final LongBitFormat BITS; - private HeaderFormat(LongBitFormat previous, int length, long min) { + HeaderFormat(LongBitFormat previous, int length, long min) { BITS = new LongBitFormat(name(), previous, length, min); } @@ -244,7 +244,7 @@ public class INodeFile extends INodeWithAdditionalFields } void setLastBlock(BlockInfo blk) { - blk.setBlockCollection(this); + blk.setBlockCollectionId(this.getId()); setBlock(numBlocks() - 1, blk); } @@ -460,7 +460,7 @@ public class INodeFile extends INodeWithAdditionalFields setBlocks(newlist); for(BlockInfo b : blocks) { - b.setBlockCollection(this); + b.setBlockCollectionId(getId()); short oldRepl = b.getReplication(); short repl = getPreferredBlockReplication(); if (oldRepl != repl) { @@ -544,7 +544,7 @@ public class INodeFile extends INodeWithAdditionalFields if (blocks != null && reclaimContext.collectedBlocks != null) { for (BlockInfo blk : blocks) { reclaimContext.collectedBlocks.addDeleteBlock(blk); - blk.setBlockCollection(null); + blk.setBlockCollectionId(INodeId.INVALID_INODE_ID); } } clearBlocks(); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeId.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeId.java index 00b33cd06a3..10139bf6cbf 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeId.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeId.java @@ -37,6 +37,7 @@ public class INodeId extends SequentialNumber { */ public static final long LAST_RESERVED_ID = 2 << 14 - 1; public static final long ROOT_INODE_ID = LAST_RESERVED_ID + 1; + public static final long INVALID_INODE_ID = -1; /** * To check if the request id is the same as saved id. Don't check fileId diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/Namesystem.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/Namesystem.java index 679d4ab6cb3..4a208d89a88 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/Namesystem.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/Namesystem.java @@ -20,6 +20,7 @@ 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.BlockInfo; +import org.apache.hadoop.hdfs.server.blockmanagement.BlockCollection; import org.apache.hadoop.hdfs.server.namenode.NameNode.OperationCategory; import org.apache.hadoop.hdfs.util.RwLock; import org.apache.hadoop.ipc.StandbyException; @@ -41,6 +42,8 @@ public interface Namesystem extends RwLock, SafeMode { boolean isGenStampInFuture(Block block); + BlockCollection getBlockCollection(long id); + void adjustSafeModeBlockTotals(int deltaSafe, int deltaTotal); void checkOperation(OperationCategory read) throws StandbyException; diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockInfo.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockInfo.java index 5126aa78dfb..ceef9f29645 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockInfo.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockInfo.java @@ -17,6 +17,7 @@ */ package org.apache.hadoop.hdfs.server.blockmanagement; +import static org.apache.hadoop.hdfs.server.namenode.INodeId.INVALID_INODE_ID; import static org.hamcrest.core.Is.is; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; @@ -51,9 +52,9 @@ public class TestBlockInfo { public void testIsDeleted() { BlockInfo blockInfo = new BlockInfoContiguous((short) 3); BlockCollection bc = Mockito.mock(BlockCollection.class); - blockInfo.setBlockCollection(bc); + blockInfo.setBlockCollectionId(1000); Assert.assertFalse(blockInfo.isDeleted()); - blockInfo.setBlockCollection(null); + blockInfo.setBlockCollectionId(INVALID_INODE_ID); Assert.assertTrue(blockInfo.isDeleted()); } @@ -71,10 +72,10 @@ public class TestBlockInfo { @Test public void testCopyConstructor() { - BlockInfo old = new BlockInfoContiguous((short) 3); + BlockInfoContiguous old = new BlockInfoContiguous((short) 3); try { - BlockInfo copy = new BlockInfoContiguous((BlockInfoContiguous)old); - assertEquals(old.getBlockCollection(), copy.getBlockCollection()); + BlockInfoContiguous copy = new BlockInfoContiguous(old); + assertEquals(old.getBlockCollectionId(), copy.getBlockCollectionId()); assertEquals(old.getCapacity(), copy.getCapacity()); } catch (Exception e) { Assert.fail("Copy constructor throws exception: " + e); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockManager.java index 0c0d7153cee..d40b1c7e2b8 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockManager.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockManager.java @@ -51,6 +51,7 @@ import org.apache.hadoop.hdfs.server.datanode.DataNodeTestUtils; import org.apache.hadoop.hdfs.server.datanode.FinalizedReplica; import org.apache.hadoop.hdfs.server.datanode.ReplicaBeingWritten; import org.apache.hadoop.hdfs.server.namenode.FSNamesystem; +import org.apache.hadoop.hdfs.server.namenode.INodeId; import org.apache.hadoop.hdfs.server.namenode.NameNodeAdapter; import org.apache.hadoop.hdfs.server.protocol.DatanodeRegistration; import org.apache.hadoop.hdfs.server.protocol.DatanodeStorage; @@ -89,6 +90,7 @@ public class TestBlockManager { private FSNamesystem fsn; private BlockManager bm; + private long mockINodeId; @Before public void setupMockCluster() throws IOException { @@ -97,6 +99,7 @@ public class TestBlockManager { "need to set a dummy value here so it assumes a multi-rack cluster"); fsn = Mockito.mock(FSNamesystem.class); Mockito.doReturn(true).when(fsn).hasWriteLock(); + Mockito.doReturn(true).when(fsn).hasReadLock(); bm = new BlockManager(fsn, conf); final String[] racks = { "/rackA", @@ -109,6 +112,7 @@ public class TestBlockManager { nodes = Arrays.asList(DFSTestUtil.toDatanodeDescriptor(storages)); rackA = nodes.subList(0, 3); rackB = nodes.subList(3, 6); + mockINodeId = INodeId.ROOT_INODE_ID + 1; } private void addNodes(Iterable nodesToAdd) { @@ -433,9 +437,14 @@ public class TestBlockManager { } private BlockInfo addBlockOnNodes(long blockId, List nodes) { + long inodeId = ++mockINodeId; BlockCollection bc = Mockito.mock(BlockCollection.class); + Mockito.doReturn(inodeId).when(bc).getId(); + Mockito.doReturn(bc).when(fsn).getBlockCollection(inodeId); BlockInfo blockInfo = blockOnNodes(blockId, nodes); + blockInfo.setReplication((short) 3); + blockInfo.setBlockCollectionId(inodeId); bm.blocksMap.addBlockCollection(blockInfo, bc); return blockInfo; } @@ -740,7 +749,10 @@ public class TestBlockManager { BlockInfo blockInfo = new BlockInfoContiguous(block, (short) 3); BlockCollection bc = Mockito.mock(BlockCollection.class); + long inodeId = ++mockINodeId; + doReturn(inodeId).when(bc).getId(); bm.blocksMap.addBlockCollection(blockInfo, bc); + doReturn(bc).when(fsn).getBlockCollection(inodeId); return blockInfo; } @@ -749,7 +761,10 @@ public class TestBlockManager { BlockInfo blockInfo = new BlockInfoContiguous(block, (short) 3); blockInfo.convertToBlockUnderConstruction(UNDER_CONSTRUCTION, null); BlockCollection bc = Mockito.mock(BlockCollection.class); + long inodeId = ++mockINodeId; + doReturn(inodeId).when(bc).getId(); bm.blocksMap.addBlockCollection(blockInfo, bc); + doReturn(bc).when(fsn).getBlockCollection(inodeId); return blockInfo; } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicy.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicy.java index b1ec4cbaa9e..cec33fef7c4 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicy.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicy.java @@ -1199,6 +1199,7 @@ public class TestReplicationPolicy { Namesystem mockNS = mock(Namesystem.class); when(mockNS.isPopulatingReplQueues()).thenReturn(true); when(mockNS.hasWriteLock()).thenReturn(true); + when(mockNS.hasReadLock()).thenReturn(true); BlockManager bm = new BlockManager(mockNS, new HdfsConfiguration()); UnderReplicatedBlocks underReplicatedBlocks = bm.neededReplications; @@ -1225,6 +1226,8 @@ public class TestReplicationPolicy { BlockInfoContiguous info = new BlockInfoContiguous(block1, (short) 1); info.convertToBlockUnderConstruction(BlockUCState.UNDER_CONSTRUCTION, null); BlockCollection bc = mock(BlockCollection.class); + when(bc.getId()).thenReturn(1000L); + when(mockNS.getBlockCollection(1000L)).thenReturn(bc); bm.addBlockCollection(info, bc); // Adding this block will increase its current replication, and that will @@ -1245,6 +1248,8 @@ public class TestReplicationPolicy { throws IOException { Namesystem mockNS = mock(Namesystem.class); when(mockNS.isPopulatingReplQueues()).thenReturn(true); + when(mockNS.hasReadLock()).thenReturn(true); + BlockManager bm = new BlockManager(mockNS, new HdfsConfiguration()); UnderReplicatedBlocks underReplicatedBlocks = bm.neededReplications; @@ -1266,13 +1271,14 @@ public class TestReplicationPolicy { final BlockInfo info = new BlockInfoContiguous(block1, (short) 1); final BlockCollection mbc = mock(BlockCollection.class); + when(mbc.getId()).thenReturn(1000L); when(mbc.getLastBlock()).thenReturn(info); when(mbc.getPreferredBlockSize()).thenReturn(block1.getNumBytes() + 1); when(mbc.isUnderConstruction()).thenReturn(true); ContentSummary cs = mock(ContentSummary.class); when(cs.getLength()).thenReturn((long)1); when(mbc.computeContentSummary(bm.getStoragePolicySuite())).thenReturn(cs); - info.setBlockCollection(mbc); + info.setBlockCollectionId(1000); bm.addBlockCollection(info, mbc); DatanodeStorageInfo[] storageAry = {new DatanodeStorageInfo( @@ -1305,6 +1311,8 @@ public class TestReplicationPolicy { throws IOException { Namesystem mockNS = mock(Namesystem.class); when(mockNS.isPopulatingReplQueues()).thenReturn(true); + when(mockNS.hasReadLock()).thenReturn(true); + BlockManager bm = new BlockManager(mockNS, new HdfsConfiguration()); UnderReplicatedBlocks underReplicatedBlocks = bm.neededReplications; diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestCommitBlockSynchronization.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestCommitBlockSynchronization.java index 35a098a38aa..6512afd16e8 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestCommitBlockSynchronization.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestCommitBlockSynchronization.java @@ -70,7 +70,7 @@ public class TestCommitBlockSynchronization { BlockInfo blockInfo = new BlockInfoContiguous(block, (short) 1); blockInfo.convertToBlockUnderConstruction( HdfsServerConstants.BlockUCState.UNDER_CONSTRUCTION, targets); - blockInfo.setBlockCollection(file); + blockInfo.setBlockCollectionId(file.getId()); blockInfo.setGenerationStamp(genStamp); blockInfo.getUnderConstructionFeature().initializeBlockRecovery(blockInfo, genStamp); @@ -88,8 +88,7 @@ public class TestCommitBlockSynchronization { } private INodeFile mockFileUnderConstruction() { - INodeFile file = mock(INodeFile.class); - return file; + return mock(INodeFile.class); } @Test @@ -110,7 +109,7 @@ public class TestCommitBlockSynchronization { // Simulate 'completing' the block. BlockInfo completedBlockInfo = new BlockInfoContiguous(block, (short) 1); - completedBlockInfo.setBlockCollection(file); + completedBlockInfo.setBlockCollectionId(file.getId()); completedBlockInfo.setGenerationStamp(genStamp); doReturn(completedBlockInfo).when(namesystemSpy) .getStoredBlock(any(Block.class)); @@ -182,7 +181,7 @@ public class TestCommitBlockSynchronization { lastBlock, genStamp, length, true, false, newTargets, null); BlockInfo completedBlockInfo = new BlockInfoContiguous(block, (short) 1); - completedBlockInfo.setBlockCollection(file); + completedBlockInfo.setBlockCollectionId(file.getId()); completedBlockInfo.setGenerationStamp(genStamp); doReturn(completedBlockInfo).when(namesystemSpy) .getStoredBlock(any(Block.class)); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFsck.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFsck.java index 3495898ba0e..a925da7f010 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFsck.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFsck.java @@ -80,6 +80,7 @@ import org.apache.hadoop.hdfs.protocol.ExtendedBlock; import org.apache.hadoop.hdfs.protocol.HdfsFileStatus; import org.apache.hadoop.hdfs.protocol.LocatedBlock; import org.apache.hadoop.hdfs.protocol.LocatedBlocks; +import org.apache.hadoop.hdfs.server.blockmanagement.BlockCollection; import org.apache.hadoop.hdfs.server.blockmanagement.BlockInfo; import org.apache.hadoop.hdfs.server.blockmanagement.BlockManager; import org.apache.hadoop.hdfs.server.blockmanagement.DatanodeDescriptor; @@ -817,11 +818,19 @@ public class TestFsck { // decommission datanode ExtendedBlock eb = DFSTestUtil.getFirstBlock(dfs, path); - DatanodeDescriptor dn = - cluster.getNameNode().getNamesystem().getBlockManager() - .getBlockCollection(eb.getLocalBlock()).getBlocks()[0].getDatanode(0); - cluster.getNameNode().getNamesystem().getBlockManager().getDatanodeManager() - .getDecomManager().startDecommission(dn); + FSNamesystem fsn = cluster.getNameNode().getNamesystem(); + BlockManager bm = fsn.getBlockManager(); + BlockCollection bc = null; + try { + fsn.writeLock(); + BlockInfo bi = bm.getStoredBlock(eb.getLocalBlock()); + bc = bm.getBlockCollection(bi); + } finally { + fsn.writeUnlock(); + } + DatanodeDescriptor dn = bc.getBlocks()[0] + .getDatanode(0); + bm.getDatanodeManager().getDecomManager().startDecommission(dn); String dnName = dn.getXferAddr(); // check the replica status while decommissioning @@ -1376,12 +1385,19 @@ public class TestFsck { assertTrue(outStr.contains(NamenodeFsck.HEALTHY_STATUS)); //decommission datanode + FSNamesystem fsn = cluster.getNameNode().getNamesystem(); + BlockManager bm = fsn.getBlockManager(); ExtendedBlock eb = util.getFirstBlock(dfs, path); - DatanodeDescriptor dn = cluster.getNameNode().getNamesystem() - .getBlockManager().getBlockCollection(eb.getLocalBlock()) - .getBlocks()[0].getDatanode(0); - cluster.getNameNode().getNamesystem().getBlockManager() - .getDatanodeManager().getDecomManager().startDecommission(dn); + BlockCollection bc = null; + try { + fsn.writeLock(); + BlockInfo bi = bm.getStoredBlock(eb.getLocalBlock()); + bc = bm.getBlockCollection(bi); + } finally { + fsn.writeUnlock(); + } + DatanodeDescriptor dn = bc.getBlocks()[0].getDatanode(0); + bm.getDatanodeManager().getDecomManager().startDecommission(dn); String dnName = dn.getXferAddr(); //wait for decommission start @@ -1584,12 +1600,20 @@ public class TestFsck { assertTrue(outStr.contains(NamenodeFsck.HEALTHY_STATUS)); // decommission datanode + FSNamesystem fsn = cluster.getNameNode().getNamesystem(); + BlockManager bm = fsn.getBlockManager(); ExtendedBlock eb = util.getFirstBlock(dfs, path); - DatanodeDescriptor dn = cluster.getNameNode().getNamesystem() - .getBlockManager().getBlockCollection(eb.getLocalBlock()) - .getBlocks()[0].getDatanode(0); - cluster.getNameNode().getNamesystem().getBlockManager() - .getDatanodeManager().getDecomManager().startDecommission(dn); + BlockCollection bc = null; + try { + fsn.writeLock(); + BlockInfo bi = bm.getStoredBlock(eb.getLocalBlock()); + bc = bm.getBlockCollection(bi); + } finally { + fsn.writeUnlock(); + } + DatanodeDescriptor dn = bc.getBlocks()[0] + .getDatanode(0); + bm.getDatanodeManager().getDecomManager().startDecommission(dn); String dnName = dn.getXferAddr(); // wait for decommission start diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotBlocksMap.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotBlocksMap.java index 43f43f7b544..dad1eeb3457 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotBlocksMap.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotBlocksMap.java @@ -17,11 +17,11 @@ */ package org.apache.hadoop.hdfs.server.namenode.snapshot; +import static org.apache.hadoop.hdfs.server.namenode.INodeId.INVALID_INODE_ID; import static org.apache.hadoop.test.GenericTestUtils.assertExceptionContains; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -87,23 +87,6 @@ public class TestSnapshotBlocksMap { } } - void assertAllNull(INodeFile inode, Path path, String[] snapshots) throws Exception { - Assert.assertNull(inode.getBlocks()); - assertINodeNull(path.toString()); - assertINodeNullInSnapshots(path, snapshots); - } - - void assertINodeNull(String path) throws Exception { - Assert.assertNull(fsdir.getINode(path)); - } - - void assertINodeNullInSnapshots(Path path, String... snapshots) throws Exception { - for(String s : snapshots) { - assertINodeNull(SnapshotTestHelper.getSnapshotPath( - path.getParent(), s, path.getName()).toString()); - } - } - static INodeFile assertBlockCollection(String path, int numBlocks, final FSDirectory dir, final BlockManager blkManager) throws Exception { final INodeFile file = INodeFile.valueOf(dir.getINode(path), path); @@ -117,8 +100,7 @@ public class TestSnapshotBlocksMap { static void assertBlockCollection(final BlockManager blkManager, final INodeFile file, final BlockInfo b) { Assert.assertSame(b, blkManager.getStoredBlock(b)); - Assert.assertSame(file, blkManager.getBlockCollection(b)); - Assert.assertSame(file, b.getBlockCollection()); + Assert.assertEquals(file.getId(), b.getBlockCollectionId()); } /** @@ -150,7 +132,7 @@ public class TestSnapshotBlocksMap { hdfs.delete(sub2, true); // The INode should have been removed from the blocksMap for(BlockInfo b : blocks) { - assertNull(blockmanager.getBlockCollection(b)); + assertEquals(INVALID_INODE_ID, b.getBlockCollectionId()); } } @@ -188,7 +170,7 @@ public class TestSnapshotBlocksMap { hdfs.delete(file0, true); // Make sure the blocks of file0 is still in blocksMap for(BlockInfo b : blocks0) { - assertNotNull(blockmanager.getBlockCollection(b)); + assertNotEquals(INVALID_INODE_ID, b.getBlockCollectionId()); } assertBlockCollection(snapshotFile0.toString(), 4, fsdir, blockmanager); @@ -202,7 +184,7 @@ public class TestSnapshotBlocksMap { // Make sure the first block of file0 is still in blocksMap for(BlockInfo b : blocks0) { - assertNotNull(blockmanager.getBlockCollection(b)); + assertNotEquals(INVALID_INODE_ID, b.getBlockCollectionId()); } assertBlockCollection(snapshotFile0.toString(), 4, fsdir, blockmanager); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotDeletion.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotDeletion.java index 4e074382c03..6f70b82af29 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotDeletion.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotDeletion.java @@ -17,6 +17,7 @@ */ package org.apache.hadoop.hdfs.server.namenode.snapshot; +import static org.apache.hadoop.hdfs.server.namenode.INodeId.INVALID_INODE_ID; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -268,7 +269,7 @@ public class TestSnapshotDeletion { checkQuotaUsageComputation(dir, 8, BLOCKSIZE * REPLICATION * 3); // check blocks of tempFile for (BlockInfo b : blocks) { - assertNull(blockmanager.getBlockCollection(b)); + assertEquals(INVALID_INODE_ID, b.getBlockCollectionId()); } // make a change: create a new file under subsub @@ -345,7 +346,7 @@ public class TestSnapshotDeletion { // newFile checkQuotaUsageComputation(dir, 9L, BLOCKSIZE * REPLICATION * 4); for (BlockInfo b : blocks) { - assertNull(blockmanager.getBlockCollection(b)); + assertEquals(INVALID_INODE_ID, b.getBlockCollectionId()); } // make sure the whole subtree of sub is stored correctly in snapshot @@ -508,7 +509,7 @@ public class TestSnapshotDeletion { // metaChangeFile's replication factor decreases checkQuotaUsageComputation(dir, 6, 2 * BLOCKSIZE * REPLICATION - BLOCKSIZE); for (BlockInfo b : blocks) { - assertNull(blockmanager.getBlockCollection(b)); + assertEquals(INVALID_INODE_ID, b.getBlockCollectionId()); } // check 1. there is no snapshot s0 @@ -839,7 +840,7 @@ public class TestSnapshotDeletion { assertFalse(hdfs.exists(file14_s1)); assertFalse(hdfs.exists(file15_s1)); for (BlockInfo b : blocks_14) { - assertNull(blockmanager.getBlockCollection(b)); + assertEquals(INVALID_INODE_ID, b.getBlockCollectionId()); } INodeFile nodeFile13 = (INodeFile) fsdir.getINode(file13.toString());