HDFS-6481. DatanodeManager#getDatanodeStorageInfos() should check the length of storageIDs. (Contributed by szetszwo)
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/63ed399e Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/63ed399e Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/63ed399e Branch: refs/heads/branch-2.7 Commit: 63ed399e1cccc4087acb442eaffbee6e32f083c9 Parents: f7deb6b Author: Arpit Agarwal <a...@apache.org> Authored: Fri Nov 6 10:13:22 2015 -0800 Committer: Arpit Agarwal <a...@apache.org> Committed: Fri Nov 6 10:32:34 2015 -0800 ---------------------------------------------------------------------- hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt | 3 ++ .../server/blockmanagement/DatanodeManager.java | 14 +++++++-- .../hdfs/server/namenode/FSNamesystem.java | 33 +++++++++++--------- .../TestCommitBlockSynchronization.java | 4 +-- 4 files changed, 35 insertions(+), 19 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/63ed399e/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index 3e62c5d..65f7b61 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -110,6 +110,9 @@ Release 2.7.2 - UNRELEASED HDFS-9317. Document fsck -blockId and -storagepolicy options in branch-2.7. (aajisaka) + HDFS-6481. DatanodeManager#getDatanodeStorageInfos() should check the + length of storageIDs. (szetszwo via Arpit Agarwal) + Release 2.7.1 - 2015-07-06 INCOMPATIBLE CHANGES http://git-wip-us.apache.org/repos/asf/hadoop/blob/63ed399e/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java index 0cf1eee..c774d0b 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java @@ -502,8 +502,18 @@ public class DatanodeManager { } public DatanodeStorageInfo[] getDatanodeStorageInfos( - DatanodeID[] datanodeID, String[] storageIDs) - throws UnregisteredNodeException { + DatanodeID[] datanodeID, String[] storageIDs, + String format, Object... args) throws UnregisteredNodeException { + if (datanodeID.length != storageIDs.length) { + final String err = (storageIDs.length == 0? + "Missing storageIDs: It is likely that the HDFS client," + + " who made this call, is running in an older version of Hadoop" + + " which does not support storageIDs." + : "Length mismatched: storageIDs.length=" + storageIDs.length + " != " + ) + " datanodeID.length=" + datanodeID.length; + throw new HadoopIllegalArgumentException( + err + ", "+ String.format(format, args)); + } if (datanodeID.length == 0) { return null; } http://git-wip-us.apache.org/repos/asf/hadoop/blob/63ed399e/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 a2e35eb..8cdae05 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 @@ -3350,7 +3350,9 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, //find datanode storages final DatanodeManager dm = blockManager.getDatanodeManager(); - chosen = Arrays.asList(dm.getDatanodeStorageInfos(existings, storageIDs)); + chosen = Arrays.asList(dm.getDatanodeStorageInfos(existings, storageIDs, + "src=%s, fileId=%d, blk=%s, clientName=%s, clientMachine=%s", + src, fileId, blk, clientName, clientMachine)); } finally { readUnlock(); } @@ -4258,7 +4260,7 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, + ", deleteBlock=" + deleteblock + ")"); checkOperation(OperationCategory.WRITE); - String src = ""; + final String src; waitForLoadingFSImage(); writeLock(); try { @@ -4303,10 +4305,10 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, + " deleted and the block removal is delayed"); } INodeFile iFile = ((INode)blockCollection).asFile(); + src = iFile.getFullPathName(); if (isFileDeleted(iFile)) { throw new FileNotFoundException("File not found: " - + iFile.getFullPathName() + ", likely due to delayed block" - + " removal"); + + src + ", likely due to delayed block removal"); } if ((!iFile.isUnderConstruction() || storedBlock.isComplete()) && iFile.getLastBlock().isComplete()) { @@ -4382,7 +4384,10 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, DatanodeStorageInfo[] trimmedStorageInfos = blockManager.getDatanodeManager().getDatanodeStorageInfos( trimmedTargets.toArray(new DatanodeID[trimmedTargets.size()]), - trimmedStorages.toArray(new String[trimmedStorages.size()])); + trimmedStorages.toArray(new String[trimmedStorages.size()]), + "src=%s, oldBlock=%s, newgenerationstamp=%d, newlength=%d", + src, oldBlock, newgenerationstamp, newlength); + if(copyTruncate) { iFile.setLastBlock(truncatedBlock, trimmedStorageInfos); } else { @@ -4396,16 +4401,15 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, if (closeFile) { if(copyTruncate) { - src = closeFileCommitBlocks(iFile, truncatedBlock); + closeFileCommitBlocks(src, iFile, truncatedBlock); if(!iFile.isBlockInLatestSnapshot(storedBlock)) { blockManager.removeBlock(storedBlock); } } else { - src = closeFileCommitBlocks(iFile, storedBlock); + closeFileCommitBlocks(src, iFile, storedBlock); } } else { // If this commit does not want to close the file, persist blocks - src = iFile.getFullPathName(); persistBlocks(src, iFile, false); } } finally { @@ -4430,10 +4434,9 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, * @throws IOException on error */ @VisibleForTesting - String closeFileCommitBlocks(INodeFile pendingFile, BlockInfoContiguous storedBlock) - throws IOException { + void closeFileCommitBlocks(String src, INodeFile pendingFile, + BlockInfoContiguous storedBlock) throws IOException { final INodesInPath iip = INodesInPath.fromINode(pendingFile); - final String src = iip.getPath(); // commit the last block and complete it if it has minimum replicas commitOrCompleteLastBlock(pendingFile, iip, storedBlock); @@ -4441,8 +4444,6 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, //remove lease, close file finalizeINodeFileUnderConstruction(src, pendingFile, Snapshot.findLatestSnapshot(pendingFile, Snapshot.CURRENT_STATE_ID)); - - return src; } /** @@ -6356,6 +6357,7 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, assert hasWriteLock(); // check the vadility of the block and lease holder name final INodeFile pendingFile = checkUCBlock(oldBlock, clientName); + final String src = pendingFile.getFullPathName(); final BlockInfoContiguousUnderConstruction blockinfo = (BlockInfoContiguousUnderConstruction)pendingFile.getLastBlock(); @@ -6375,10 +6377,11 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, // find the DatanodeDescriptor objects final DatanodeStorageInfo[] storages = blockManager.getDatanodeManager() - .getDatanodeStorageInfos(newNodes, newStorageIDs); + .getDatanodeStorageInfos(newNodes, newStorageIDs, + "src=%s, oldBlock=%s, newBlock=%s, clientName=%s", + src, oldBlock, newBlock, clientName); blockinfo.setExpectedLocations(storages); - String src = pendingFile.getFullPathName(); persistBlocks(src, pendingFile, logRetryCache); } http://git-wip-us.apache.org/repos/asf/hadoop/blob/63ed399e/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestCommitBlockSynchronization.java ---------------------------------------------------------------------- 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 8320334..c29383a 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 @@ -75,8 +75,8 @@ public class TestCommitBlockSynchronization { doReturn(blockInfo).when(namesystemSpy).getStoredBlock(any(Block.class)); doReturn(blockInfo).when(file).getLastBlock(); - doReturn("").when(namesystemSpy).closeFileCommitBlocks( - any(INodeFile.class), any(BlockInfoContiguous.class)); + doNothing().when(namesystemSpy).closeFileCommitBlocks( + any(String.class), any(INodeFile.class), any(BlockInfoContiguous.class)); doReturn(mock(FSEditLog.class)).when(namesystemSpy).getEditLog(); return namesystemSpy;
