From 36df2ebceb81d2c59894d3e98ea42748ffb227cd Mon Sep 17 00:00:00 2001 From: Ashvin Agrawal Date: Tue, 1 Oct 2019 22:33:53 -0700 Subject: [PATCH 1/4] Add method to identify a Provided storage block --- .../server/blockmanagement/BlockInfo.java | 6 ++++ .../blockmanagement/BlockInfoContiguous.java | 30 +++++++++++++++++++ .../blockmanagement/BlockInfoStriped.java | 5 ++++ .../server/blockmanagement/TestBlockInfo.java | 25 ++++++++++++++++ 4 files changed, 66 insertions(+) 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 d160f61fc8f54..dc6cf3266a5fc 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 @@ -181,6 +181,12 @@ public int getCapacity() { /** @return true if there is no datanode storage associated with the block */ abstract boolean hasNoStorage(); + /** + * Checks whether this block has a Provided replica. + * @return true if this block has a replica on Provided storage. + */ + abstract boolean isProvided(); + /** * Find specified DatanodeStorageInfo. * @return DatanodeStorageInfo or null if not found. diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoContiguous.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoContiguous.java index 149efc93091d8..ba8f7952b34ac 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoContiguous.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoContiguous.java @@ -19,6 +19,7 @@ import com.google.common.base.Preconditions; import org.apache.hadoop.classification.InterfaceAudience; +import org.apache.hadoop.fs.StorageType; import org.apache.hadoop.hdfs.protocol.Block; import org.apache.hadoop.hdfs.protocol.BlockType; @@ -28,12 +29,16 @@ @InterfaceAudience.Private public class BlockInfoContiguous extends BlockInfo { + private boolean hasProvidedStorage; + public BlockInfoContiguous(short size) { super(size); + hasProvidedStorage = false; } public BlockInfoContiguous(Block blk, short size) { super(blk, size); + hasProvidedStorage = false; } /** @@ -62,6 +67,9 @@ boolean addStorage(DatanodeStorageInfo storage, Block reportedBlock) { // find the last null node int lastNode = ensureCapacity(1); setStorageInfo(lastNode, storage); + if (storage.getStorageType() == StorageType.PROVIDED) { + hasProvidedStorage = true; + } return true; } @@ -77,9 +85,31 @@ boolean removeStorage(DatanodeStorageInfo storage) { setStorageInfo(dnIndex, getStorageInfo(lastNode)); // set the last entry to null setStorageInfo(lastNode, null); + if (storage.getStorageType() == StorageType.PROVIDED + && !hasProvidedStorages()) { + hasProvidedStorage = false; + } return true; } + @Override + boolean isProvided() { + return hasProvidedStorage; + } + + private boolean hasProvidedStorages() { + int len = getCapacity(); + for(int idx = 0; idx < len; idx++) { + DatanodeStorageInfo cur = getStorageInfo(idx); + if(cur != null) { + if (cur.getStorageType() == StorageType.PROVIDED) { + return true; + } + } + } + return false; + } + @Override public int numNodes() { assert this.storages != null : "BlockInfo is not initialized"; diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoStriped.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoStriped.java index 8bc63c1214d28..241155596342b 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoStriped.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoStriped.java @@ -244,6 +244,11 @@ final boolean hasNoStorage() { return true; } + @Override + boolean isProvided() { + return false; + } + /** * This class contains datanode storage information and block index in the * block group. 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 fa0dd70a7e0d8..6f23b843a28fa 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 @@ -19,7 +19,10 @@ import static org.apache.hadoop.hdfs.server.namenode.INodeId.INVALID_INODE_ID; import static org.hamcrest.core.Is.is; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import org.apache.hadoop.fs.StorageType; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.apache.hadoop.hdfs.DFSTestUtil; @@ -64,6 +67,28 @@ public void testAddStorage() throws Exception { Assert.assertEquals(storage, blockInfo.getStorageInfo(0)); } + @Test + public void testAddProvidedStorage() throws Exception { + BlockInfo blockInfo = new BlockInfoContiguous((short) 3); + + DatanodeStorageInfo storage = mock(DatanodeStorageInfo.class); + when(storage.getStorageType()).thenReturn(StorageType.PROVIDED); + boolean added = blockInfo.addStorage(storage, blockInfo); + + Assert.assertTrue(added); + Assert.assertEquals(storage, blockInfo.getStorageInfo(0)); + Assert.assertTrue(blockInfo.isProvided()); + + blockInfo = new BlockInfoContiguous((short) 3); + storage = mock(DatanodeStorageInfo.class); + when(storage.getStorageType()).thenReturn(StorageType.DISK); + added = blockInfo.addStorage(storage, blockInfo); + + Assert.assertTrue(added); + Assert.assertEquals(storage, blockInfo.getStorageInfo(0)); + Assert.assertFalse(blockInfo.isProvided()); + } + @Test public void testReplaceStorage() throws Exception { From 39e393809263dbd143cd859865581c63904d3303 Mon Sep 17 00:00:00 2001 From: Ashvin Agrawal Date: Wed, 2 Oct 2019 14:00:54 -0700 Subject: [PATCH 2/4] Remove provided computation from the regular pipeline --- .../blockmanagement/BlockInfoContiguous.java | 15 --------------- .../server/blockmanagement/BlockInfoStriped.java | 4 ++++ 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoContiguous.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoContiguous.java index ba8f7952b34ac..36422cffbacc0 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoContiguous.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoContiguous.java @@ -29,16 +29,12 @@ @InterfaceAudience.Private public class BlockInfoContiguous extends BlockInfo { - private boolean hasProvidedStorage; - public BlockInfoContiguous(short size) { super(size); - hasProvidedStorage = false; } public BlockInfoContiguous(Block blk, short size) { super(blk, size); - hasProvidedStorage = false; } /** @@ -67,9 +63,6 @@ boolean addStorage(DatanodeStorageInfo storage, Block reportedBlock) { // find the last null node int lastNode = ensureCapacity(1); setStorageInfo(lastNode, storage); - if (storage.getStorageType() == StorageType.PROVIDED) { - hasProvidedStorage = true; - } return true; } @@ -85,19 +78,11 @@ boolean removeStorage(DatanodeStorageInfo storage) { setStorageInfo(dnIndex, getStorageInfo(lastNode)); // set the last entry to null setStorageInfo(lastNode, null); - if (storage.getStorageType() == StorageType.PROVIDED - && !hasProvidedStorages()) { - hasProvidedStorage = false; - } return true; } @Override boolean isProvided() { - return hasProvidedStorage; - } - - private boolean hasProvidedStorages() { int len = getCapacity(); for(int idx = 0; idx < len; idx++) { DatanodeStorageInfo cur = getStorageInfo(idx); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoStriped.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoStriped.java index 241155596342b..16265ded88bca 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoStriped.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoStriped.java @@ -244,6 +244,10 @@ final boolean hasNoStorage() { return true; } + /** + * Striped blocks on Provided Storage is not supported. All blocks on + * Provided storage are assumed to be "contiguous". + */ @Override boolean isProvided() { return false; From 1b41027e784e2fa15aef00fa60f5e3aaab3c8a31 Mon Sep 17 00:00:00 2001 From: Ashvin Agrawal Date: Wed, 2 Oct 2019 20:48:24 -0700 Subject: [PATCH 3/4] Fix cody style --- .../server/blockmanagement/BlockInfoContiguous.java | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoContiguous.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoContiguous.java index 36422cffbacc0..7378e6f21b765 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoContiguous.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoContiguous.java @@ -84,12 +84,11 @@ boolean removeStorage(DatanodeStorageInfo storage) { @Override boolean isProvided() { int len = getCapacity(); - for(int idx = 0; idx < len; idx++) { - DatanodeStorageInfo cur = getStorageInfo(idx); - if(cur != null) { - if (cur.getStorageType() == StorageType.PROVIDED) { - return true; - } + for (int idx = 0; idx < len; idx++) { + DatanodeStorageInfo storage = getStorageInfo(idx); + if (storage != null + && storage.getStorageType().equals(StorageType.PROVIDED)) { + return true; } } return false; From 42381bed343c4cd11136576de51ba4da4c26a70d Mon Sep 17 00:00:00 2001 From: Ashvin Agrawal Date: Thu, 3 Oct 2019 11:54:22 -0700 Subject: [PATCH 4/4] Improve unit test for block condition check --- .../server/blockmanagement/TestBlockInfo.java | 35 ++++++++++++------- 1 file changed, 23 insertions(+), 12 deletions(-) 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 6f23b843a28fa..3c5c5d9fb2fee 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 @@ -69,24 +69,35 @@ public void testAddStorage() throws Exception { @Test public void testAddProvidedStorage() throws Exception { + // block with only provided storage BlockInfo blockInfo = new BlockInfoContiguous((short) 3); - - DatanodeStorageInfo storage = mock(DatanodeStorageInfo.class); - when(storage.getStorageType()).thenReturn(StorageType.PROVIDED); - boolean added = blockInfo.addStorage(storage, blockInfo); - + DatanodeStorageInfo providedStorage = mock(DatanodeStorageInfo.class); + when(providedStorage.getStorageType()).thenReturn(StorageType.PROVIDED); + boolean added = blockInfo.addStorage(providedStorage, blockInfo); Assert.assertTrue(added); - Assert.assertEquals(storage, blockInfo.getStorageInfo(0)); + Assert.assertEquals(providedStorage, blockInfo.getStorageInfo(0)); Assert.assertTrue(blockInfo.isProvided()); + } - blockInfo = new BlockInfoContiguous((short) 3); - storage = mock(DatanodeStorageInfo.class); - when(storage.getStorageType()).thenReturn(StorageType.DISK); - added = blockInfo.addStorage(storage, blockInfo); - + @Test + public void testAddTwoStorageTypes() throws Exception { + // block with only disk storage + BlockInfo blockInfo = new BlockInfoContiguous((short) 3); + DatanodeStorageInfo diskStorage = mock(DatanodeStorageInfo.class); + DatanodeDescriptor mockDN = mock(DatanodeDescriptor.class); + when(diskStorage.getDatanodeDescriptor()).thenReturn(mockDN); + when(diskStorage.getStorageType()).thenReturn(StorageType.DISK); + boolean added = blockInfo.addStorage(diskStorage, blockInfo); Assert.assertTrue(added); - Assert.assertEquals(storage, blockInfo.getStorageInfo(0)); + Assert.assertEquals(diskStorage, blockInfo.getStorageInfo(0)); Assert.assertFalse(blockInfo.isProvided()); + + // now add provided storage + DatanodeStorageInfo providedStorage = mock(DatanodeStorageInfo.class); + when(providedStorage.getStorageType()).thenReturn(StorageType.PROVIDED); + added = blockInfo.addStorage(providedStorage, blockInfo); + Assert.assertTrue(added); + Assert.assertTrue(blockInfo.isProvided()); } @Test