HDFS-13222. Update getBlocks method to take minBlockSize in RPC calls. Contributed by Bharat Viswanadham
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/88fba00c Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/88fba00c Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/88fba00c Branch: refs/heads/HDFS-7240 Commit: 88fba00caa8c8e26f70deb9be5b534e7482620a1 Parents: e0307e5 Author: Tsz-Wo Nicholas Sze <[email protected]> Authored: Wed Mar 7 11:27:53 2018 -0800 Committer: Tsz-Wo Nicholas Sze <[email protected]> Committed: Wed Mar 7 11:27:53 2018 -0800 ---------------------------------------------------------------------- .../NamenodeProtocolServerSideTranslatorPB.java | 3 +- .../NamenodeProtocolTranslatorPB.java | 5 +- .../hadoop/hdfs/server/balancer/Dispatcher.java | 2 +- .../hdfs/server/balancer/NameNodeConnector.java | 5 +- .../server/blockmanagement/BlockManager.java | 17 ++----- .../hdfs/server/namenode/FSNamesystem.java | 7 +-- .../hdfs/server/namenode/NameNodeRpcServer.java | 13 +++-- .../hdfs/server/protocol/NamenodeProtocol.java | 9 ++-- .../src/main/proto/NamenodeProtocol.proto | 1 + .../org/apache/hadoop/hdfs/TestGetBlocks.java | 50 ++++++++++++++++---- .../hdfs/server/balancer/TestBalancer.java | 2 +- 11 files changed, 73 insertions(+), 41 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/88fba00c/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/NamenodeProtocolServerSideTranslatorPB.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/NamenodeProtocolServerSideTranslatorPB.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/NamenodeProtocolServerSideTranslatorPB.java index 6a10fe4..90c2c49 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/NamenodeProtocolServerSideTranslatorPB.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/NamenodeProtocolServerSideTranslatorPB.java @@ -86,7 +86,8 @@ public class NamenodeProtocolServerSideTranslatorPB implements .build(); BlocksWithLocations blocks; try { - blocks = impl.getBlocks(dnInfo, request.getSize()); + blocks = impl.getBlocks(dnInfo, request.getSize(), + request.getMinBlockSize()); } catch (IOException e) { throw new ServiceException(e); } http://git-wip-us.apache.org/repos/asf/hadoop/blob/88fba00c/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/NamenodeProtocolTranslatorPB.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/NamenodeProtocolTranslatorPB.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/NamenodeProtocolTranslatorPB.java index 02074f3..632f8b7 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/NamenodeProtocolTranslatorPB.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/NamenodeProtocolTranslatorPB.java @@ -99,11 +99,12 @@ public class NamenodeProtocolTranslatorPB implements NamenodeProtocol, } @Override - public BlocksWithLocations getBlocks(DatanodeInfo datanode, long size) + public BlocksWithLocations getBlocks(DatanodeInfo datanode, long size, long + minBlockSize) throws IOException { GetBlocksRequestProto req = GetBlocksRequestProto.newBuilder() .setDatanode(PBHelperClient.convert((DatanodeID)datanode)).setSize(size) - .build(); + .setMinBlockSize(minBlockSize).build(); try { return PBHelper.convert(rpcProxy.getBlocks(NULL_CONTROLLER, req) .getBlocks()); http://git-wip-us.apache.org/repos/asf/hadoop/blob/88fba00c/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/balancer/Dispatcher.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/balancer/Dispatcher.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/balancer/Dispatcher.java index 9270fde..349ced1 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/balancer/Dispatcher.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/balancer/Dispatcher.java @@ -785,7 +785,7 @@ public class Dispatcher { private long getBlockList() throws IOException { final long size = Math.min(getBlocksSize, blocksToReceive); final BlocksWithLocations newBlksLocs = - nnc.getBlocks(getDatanodeInfo(), size); + nnc.getBlocks(getDatanodeInfo(), size, getBlocksMinBlockSize); if (LOG.isTraceEnabled()) { LOG.trace("getBlocks(" + getDatanodeInfo() + ", " http://git-wip-us.apache.org/repos/asf/hadoop/blob/88fba00c/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/balancer/NameNodeConnector.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/balancer/NameNodeConnector.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/balancer/NameNodeConnector.java index be59cce..b0dd779 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/balancer/NameNodeConnector.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/balancer/NameNodeConnector.java @@ -162,9 +162,10 @@ public class NameNodeConnector implements Closeable { } /** @return blocks with locations. */ - public BlocksWithLocations getBlocks(DatanodeInfo datanode, long size) + public BlocksWithLocations getBlocks(DatanodeInfo datanode, long size, long + minBlockSize) throws IOException { - return namenode.getBlocks(datanode, size); + return namenode.getBlocks(datanode, size, minBlockSize); } /** http://git-wip-us.apache.org/repos/asf/hadoop/blob/88fba00c/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java ---------------------------------------------------------------------- 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 6b7175d..3f07a12 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 @@ -409,13 +409,6 @@ public class BlockManager implements BlockStatsMXBean { private int numBlocksPerIteration; /** - * Minimum size that a block can be sent to Balancer through getBlocks. - * And after HDFS-8824, the small blocks are unused anyway, so there's no - * point to send them to balancer. - */ - private long getBlocksMinBlockSize = -1; - - /** * Progress of the Reconstruction queues initialisation. */ private double reconstructionQueuesInitProgress = 0.0; @@ -539,9 +532,6 @@ public class BlockManager implements BlockStatsMXBean { this.numBlocksPerIteration = conf.getInt( DFSConfigKeys.DFS_BLOCK_MISREPLICATION_PROCESSING_LIMIT, DFSConfigKeys.DFS_BLOCK_MISREPLICATION_PROCESSING_LIMIT_DEFAULT); - this.getBlocksMinBlockSize = conf.getLongBytes( - DFSConfigKeys.DFS_BALANCER_GETBLOCKS_MIN_BLOCK_SIZE_KEY, - DFSConfigKeys.DFS_BALANCER_GETBLOCKS_MIN_BLOCK_SIZE_DEFAULT); final int minMaintenanceR = conf.getInt( DFSConfigKeys.DFS_NAMENODE_MAINTENANCE_REPLICATION_MIN_KEY, @@ -1469,7 +1459,8 @@ public class BlockManager implements BlockStatsMXBean { /** Get all blocks with location information from a datanode. */ public BlocksWithLocations getBlocksWithLocations(final DatanodeID datanode, - final long size) throws UnregisteredNodeException { + final long size, final long minBlockSize) throws + UnregisteredNodeException { final DatanodeDescriptor node = getDatanodeManager().getDatanode(datanode); if (node == null) { blockLog.warn("BLOCK* getBlocks: Asking for blocks from an" + @@ -1491,7 +1482,7 @@ public class BlockManager implements BlockStatsMXBean { while(totalSize<size && iter.hasNext()) { curBlock = iter.next(); if(!curBlock.isComplete()) continue; - if (curBlock.getNumBytes() < getBlocksMinBlockSize) { + if (curBlock.getNumBytes() < minBlockSize) { continue; } totalSize += addBlock(curBlock, results); @@ -1501,7 +1492,7 @@ public class BlockManager implements BlockStatsMXBean { for(int i=0; i<startBlock&&totalSize<size; i++) { curBlock = iter.next(); if(!curBlock.isComplete()) continue; - if (curBlock.getNumBytes() < getBlocksMinBlockSize) { + if (curBlock.getNumBytes() < minBlockSize) { continue; } totalSize += addBlock(curBlock, results); http://git-wip-us.apache.org/repos/asf/hadoop/blob/88fba00c/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java ---------------------------------------------------------------------- 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 d36b122..e0ece35 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 @@ -1718,13 +1718,14 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, * @param datanode on which blocks are located * @param size total size of blocks */ - public BlocksWithLocations getBlocks(DatanodeID datanode, long size) - throws IOException { + public BlocksWithLocations getBlocks(DatanodeID datanode, long size, long + minimumBlockSize) throws IOException { checkOperation(OperationCategory.READ); readLock(); try { checkOperation(OperationCategory.READ); - return getBlockManager().getBlocksWithLocations(datanode, size); + return getBlockManager().getBlocksWithLocations(datanode, size, + minimumBlockSize); } finally { readUnlock("getBlocks"); } http://git-wip-us.apache.org/repos/asf/hadoop/blob/88fba00c/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeRpcServer.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeRpcServer.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeRpcServer.java index 9494263..1b7a636 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeRpcServer.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeRpcServer.java @@ -618,15 +618,20 @@ public class NameNodeRpcServer implements NamenodeProtocols { // NamenodeProtocol ///////////////////////////////////////////////////// @Override // NamenodeProtocol - public BlocksWithLocations getBlocks(DatanodeInfo datanode, long size) - throws IOException { + public BlocksWithLocations getBlocks(DatanodeInfo datanode, long size, long + minBlockSize) + throws IOException { if(size <= 0) { throw new IllegalArgumentException( - "Unexpected not positive size: "+size); + "Unexpected not positive size: "+size); + } + if(minBlockSize < 0) { + throw new IllegalArgumentException( + "Unexpected not positive size: "+size); } checkNNStartup(); namesystem.checkSuperuserPrivilege(); - return namesystem.getBlocks(datanode, size); + return namesystem.getBlocks(datanode, size, minBlockSize); } @Override // NamenodeProtocol http://git-wip-us.apache.org/repos/asf/hadoop/blob/88fba00c/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/NamenodeProtocol.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/NamenodeProtocol.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/NamenodeProtocol.java index ccdf516c..0c8adc6 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/NamenodeProtocol.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/NamenodeProtocol.java @@ -67,17 +67,18 @@ public interface NamenodeProtocol { /** * Get a list of blocks belonging to <code>datanode</code> * whose total size equals <code>size</code>. - * + * * @see org.apache.hadoop.hdfs.server.balancer.Balancer * @param datanode a data node * @param size requested size + * @param minBlockSize each block should be of this minimum Block Size * @return a list of blocks & their locations * @throws IOException if size is less than or equal to 0 or - datanode does not exist + datanode does not exist */ @Idempotent - public BlocksWithLocations getBlocks(DatanodeInfo datanode, long size) - throws IOException; + BlocksWithLocations getBlocks(DatanodeInfo datanode, long size, long + minBlockSize) throws IOException; /** * Get the current block keys http://git-wip-us.apache.org/repos/asf/hadoop/blob/88fba00c/hadoop-hdfs-project/hadoop-hdfs/src/main/proto/NamenodeProtocol.proto ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/proto/NamenodeProtocol.proto b/hadoop-hdfs-project/hadoop-hdfs/src/main/proto/NamenodeProtocol.proto index 8aa09d3..29ae9d6 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/proto/NamenodeProtocol.proto +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/proto/NamenodeProtocol.proto @@ -43,6 +43,7 @@ import "HdfsServer.proto"; message GetBlocksRequestProto { required DatanodeIDProto datanode = 1; // Datanode ID required uint64 size = 2; // Size in bytes + optional uint64 minBlockSize = 3 [default = 0]; // Minimum Block Size in bytes } http://git-wip-us.apache.org/repos/asf/hadoop/blob/88fba00c/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestGetBlocks.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestGetBlocks.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestGetBlocks.java index 7397507..e3a1763 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestGetBlocks.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestGetBlocks.java @@ -229,32 +229,48 @@ public class TestGetBlocks { NamenodeProtocol namenode = NameNodeProxies.createProxy(CONF, DFSUtilClient.getNNUri(addr), NamenodeProtocol.class).getProxy(); - // get blocks of size fileLen from dataNodes[0] + // get blocks of size fileLen from dataNodes[0], with minBlockSize as + // fileLen BlockWithLocations[] locs; - locs = namenode.getBlocks(dataNodes[0], fileLen).getBlocks(); - assertEquals(locs.length, 12); + + // Should return all 13 blocks, as minBlockSize is not passed + locs = namenode.getBlocks(dataNodes[0], fileLen, 0) + .getBlocks(); + assertEquals(13, locs.length); + assertEquals(locs[0].getStorageIDs().length, 2); + assertEquals(locs[1].getStorageIDs().length, 2); + + // Should return 12 blocks, as minBlockSize is DEFAULT_BLOCK_SIZE + locs = namenode.getBlocks(dataNodes[0], fileLen, DEFAULT_BLOCK_SIZE) + .getBlocks(); + assertEquals(12, locs.length); assertEquals(locs[0].getStorageIDs().length, 2); assertEquals(locs[1].getStorageIDs().length, 2); // get blocks of size BlockSize from dataNodes[0] - locs = namenode.getBlocks(dataNodes[0], DEFAULT_BLOCK_SIZE).getBlocks(); + locs = namenode.getBlocks(dataNodes[0], DEFAULT_BLOCK_SIZE, + DEFAULT_BLOCK_SIZE).getBlocks(); assertEquals(locs.length, 1); assertEquals(locs[0].getStorageIDs().length, 2); // get blocks of size 1 from dataNodes[0] - locs = namenode.getBlocks(dataNodes[0], 1).getBlocks(); + locs = namenode.getBlocks(dataNodes[0], 1, 1).getBlocks(); assertEquals(locs.length, 1); assertEquals(locs[0].getStorageIDs().length, 2); // get blocks of size 0 from dataNodes[0] - getBlocksWithException(namenode, dataNodes[0], 0); + getBlocksWithException(namenode, dataNodes[0], 0, 0); // get blocks of size -1 from dataNodes[0] - getBlocksWithException(namenode, dataNodes[0], -1); + getBlocksWithException(namenode, dataNodes[0], -1, 0); + + // minBlockSize is -1 + getBlocksWithException(namenode, dataNodes[0], DEFAULT_BLOCK_SIZE, -1); // get blocks of size BlockSize from a non-existent datanode DatanodeInfo info = DFSTestUtil.getDatanodeInfo("1.2.3.4"); - getBlocksWithException(namenode, info, 2); + getBlocksWithIncorrectDatanodeException(namenode, info, 2, 0); + testBlockIterator(cluster); } finally { @@ -263,10 +279,24 @@ public class TestGetBlocks { } private void getBlocksWithException(NamenodeProtocol namenode, - DatanodeInfo datanode, long size) throws IOException { + DatanodeInfo datanode, long size, long minBlockSize) throws IOException { + boolean getException = false; + try { + namenode.getBlocks(datanode, size, minBlockSize); + } catch (RemoteException e) { + getException = true; + assertTrue(e.getClassName().contains("IllegalArgumentException")); + } + assertTrue(getException); + } + + private void getBlocksWithIncorrectDatanodeException( + NamenodeProtocol namenode, DatanodeInfo datanode, long size, + long minBlockSize) + throws IOException { boolean getException = false; try { - namenode.getBlocks(DFSTestUtil.getLocalDatanodeInfo(), 2); + namenode.getBlocks(datanode, size, minBlockSize); } catch (RemoteException e) { getException = true; assertTrue(e.getClassName().contains("HadoopIllegalArgumentException")); http://git-wip-us.apache.org/repos/asf/hadoop/blob/88fba00c/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/balancer/TestBalancer.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/balancer/TestBalancer.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/balancer/TestBalancer.java index 9452b8b..9579b82 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/balancer/TestBalancer.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/balancer/TestBalancer.java @@ -2072,7 +2072,7 @@ public class TestBalancer { endGetBlocksTime = Time.monotonicNow(); numGetBlocksCalls++; return blk; - }}).when(fsnSpy).getBlocks(any(DatanodeID.class), anyLong()); + }}).when(fsnSpy).getBlocks(any(DatanodeID.class), anyLong(), anyLong()); } /** --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
