diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index 5b934360c16..04834af39a5 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -479,9 +479,6 @@ Release 2.6.0 - UNRELEASED HDFS-6791. A block could remain under replicated if all of its replicas are on decommissioned nodes. (Ming Ma via jing9) - HDFS-6830. BlockInfo.addStorage fails when DN changes the storage for a - block replica. (Arpit Agarwal) - Release 2.5.0 - UNRELEASED INCOMPATIBLE CHANGES 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 0ce112118c1..272cb1d48a1 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 @@ -194,12 +194,24 @@ public int numNodes() { * Add a {@link DatanodeStorageInfo} location for a block */ boolean addStorage(DatanodeStorageInfo storage) { + boolean added = true; + int idx = findDatanode(storage.getDatanodeDescriptor()); + if(idx >= 0) { + if (getStorageInfo(idx) == storage) { // the storage is already there + return false; + } else { + // The block is on the DN but belongs to a different storage. + // Update our state. + removeStorage(getStorageInfo(idx)); + added = false; // Just updating storage. Return false. + } + } // find the last null node int lastNode = ensureCapacity(1); setStorageInfo(lastNode, storage); setNext(lastNode, null); setPrevious(lastNode, null); - return true; + return added; } /** @@ -228,18 +240,16 @@ assert getPrevious(dnIndex) == null && getNext(dnIndex) == null : * Find specified DatanodeDescriptor. * @return index or -1 if not found. */ - boolean findDatanode(DatanodeDescriptor dn) { + int findDatanode(DatanodeDescriptor dn) { int len = getCapacity(); for(int idx = 0; idx < len; idx++) { DatanodeDescriptor cur = getDatanode(idx); - if(cur == dn) { - return true; - } - if(cur == null) { + if(cur == dn) + return idx; + if(cur == null) break; - } } - return false; + return -1; } /** * Find specified DatanodeStorageInfo. 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 6fdec77be69..8046d8ad0cd 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 @@ -2065,7 +2065,7 @@ private BlockInfo processReportedBlock( // Add replica if appropriate. If the replica was previously corrupt // but now okay, it might need to be updated. if (reportedState == ReplicaState.FINALIZED - && (!storedBlock.findDatanode(dn) + && (storedBlock.findDatanode(dn) < 0 || corruptReplicas.isReplicaCorrupt(storedBlock, dn))) { toAdd.add(storedBlock); } @@ -2246,7 +2246,7 @@ void addStoredBlockUnderConstruction(StatefulBlockInfo ucBlock, storageInfo, ucBlock.reportedBlock, ucBlock.reportedState); if (ucBlock.reportedState == ReplicaState.FINALIZED && - !block.findDatanode(storageInfo.getDatanodeDescriptor())) { + block.findDatanode(storageInfo.getDatanodeDescriptor()) < 0) { addStoredBlock(block, storageInfo, null, true); } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeStorageInfo.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeStorageInfo.java index 9e442c7e1f7..791fc3157d9 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeStorageInfo.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeStorageInfo.java @@ -208,28 +208,12 @@ long getBlockPoolUsed() { } public boolean addBlock(BlockInfo b) { - // First check whether the block belongs to a different storage - // on the same DN. - boolean replaced = false; - DatanodeStorageInfo otherStorage = - b.findStorageInfo(getDatanodeDescriptor()); - - if (otherStorage != null) { - if (otherStorage != this) { - // The block belongs to a different storage. Remove it first. - otherStorage.removeBlock(b); - replaced = true; - } else { - // The block is already associated with this storage. - return false; - } - } - + if(!b.addStorage(this)) + return false; // add to the head of the data-node list - b.addStorage(this); blockList = b.listInsert(blockList, this); numBlocks++; - return !replaced; + return true; } boolean removeBlock(BlockInfo b) { 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 61094dfc8ff..7cfe423c6e4 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,7 +17,6 @@ */ package org.apache.hadoop.hdfs.server.blockmanagement; -import static org.hamcrest.core.Is.is; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; @@ -60,24 +59,17 @@ public void testAddStorage() throws Exception { @Test - public void testReplaceStorage() throws Exception { + public void testReplaceStorageIfDifferetnOneAlreadyExistedFromSameDataNode() throws Exception { + BlockInfo blockInfo = new BlockInfo(3); - // Create two dummy storages. final DatanodeStorageInfo storage1 = DFSTestUtil.createDatanodeStorageInfo("storageID1", "127.0.0.1"); final DatanodeStorageInfo storage2 = new DatanodeStorageInfo(storage1.getDatanodeDescriptor(), new DatanodeStorage("storageID2")); - final int NUM_BLOCKS = 10; - BlockInfo[] blockInfos = new BlockInfo[NUM_BLOCKS]; - // Create a few dummy blocks and add them to the first storage. - for (int i = 0; i < NUM_BLOCKS; ++i) { - blockInfos[i] = new BlockInfo(3); - storage1.addBlock(blockInfos[i]); - } + blockInfo.addStorage(storage1); + boolean added = blockInfo.addStorage(storage2); - // Try to move one of the blocks to a different storage. - boolean added = storage2.addBlock(blockInfos[NUM_BLOCKS/2]); - Assert.assertThat(added, is(false)); - Assert.assertThat(blockInfos[NUM_BLOCKS/2].getStorageInfo(0), is(storage2)); + Assert.assertFalse(added); + Assert.assertEquals(storage2, blockInfo.getStorageInfo(0)); } @Test