HDFS-11681. DatanodeStorageInfo#getBlockIterator() should return an iterator to an unmodifiable set Contributed by Virajith Jalaparti
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/51b671ef Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/51b671ef Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/51b671ef Branch: refs/heads/YARN-5734 Commit: 51b671ef1844069888f976cd16f66c88f9bbc7de Parents: eed7314 Author: Chris Douglas <cdoug...@apache.org> Authored: Wed May 10 22:20:27 2017 -0700 Committer: Chris Douglas <cdoug...@apache.org> Committed: Wed May 10 22:25:28 2017 -0700 ---------------------------------------------------------------------- .../server/blockmanagement/BlockManager.java | 23 +++++++++++--------- .../blockmanagement/DatanodeStorageInfo.java | 7 +++++- .../org/apache/hadoop/hdfs/TestGetBlocks.java | 7 ++++++ 3 files changed, 26 insertions(+), 11 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/51b671ef/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 41662a4..a9592bf 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 @@ -1409,12 +1409,15 @@ public class BlockManager implements BlockStatsMXBean { void removeBlocksAssociatedTo(final DatanodeDescriptor node) { for (DatanodeStorageInfo storage : node.getStorageInfos()) { final Iterator<BlockInfo> it = storage.getBlockIterator(); + //add the BlockInfos to a new collection as the + //returned iterator is not modifiable. + Collection<BlockInfo> toRemove = new ArrayList<>(); while (it.hasNext()) { - BlockInfo block = it.next(); - // DatanodeStorageInfo must be removed using the iterator to avoid - // ConcurrentModificationException in the underlying storage - it.remove(); - removeStoredBlock(block, node); + toRemove.add(it.next()); + } + + for (BlockInfo b : toRemove) { + removeStoredBlock(b, node); } } // Remove all pending DN messages referencing this DN. @@ -1429,11 +1432,11 @@ public class BlockManager implements BlockStatsMXBean { assert namesystem.hasWriteLock(); final Iterator<BlockInfo> it = storageInfo.getBlockIterator(); DatanodeDescriptor node = storageInfo.getDatanodeDescriptor(); - while(it.hasNext()) { - BlockInfo block = it.next(); - // DatanodeStorageInfo must be removed using the iterator to avoid - // ConcurrentModificationException in the underlying storage - it.remove(); + Collection<BlockInfo> toRemove = new ArrayList<>(); + while (it.hasNext()) { + toRemove.add(it.next()); + } + for (BlockInfo block : toRemove) { removeStoredBlock(block, node); final Block b = getBlockOnStorage(block, storageInfo); if (b != null) { http://git-wip-us.apache.org/repos/asf/hadoop/blob/51b671ef/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeStorageInfo.java ---------------------------------------------------------------------- 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 ab666b7..8af86d3 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 @@ -18,6 +18,7 @@ package org.apache.hadoop.hdfs.server.blockmanagement; import java.util.Arrays; +import java.util.Collections; import java.util.Iterator; import java.util.List; @@ -270,8 +271,12 @@ public class DatanodeStorageInfo { return blocks.size(); } + /** + * @return iterator to an unmodifiable set of blocks + * related to this {@link DatanodeStorageInfo} + */ Iterator<BlockInfo> getBlockIterator() { - return blocks.iterator(); + return Collections.unmodifiableSet(blocks).iterator(); } void updateState(StorageReport r) { http://git-wip-us.apache.org/repos/asf/hadoop/blob/51b671ef/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 32bdd78..7397507 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 @@ -307,6 +307,13 @@ public class TestGetBlocks { BlockManagerTestUtil.getBlockIterator(s); while(storageBlockIt.hasNext()) { allBlocks[idx++] = storageBlockIt.next(); + try { + storageBlockIt.remove(); + assertTrue( + "BlockInfo iterator should have been unmodifiable", false); + } catch (UnsupportedOperationException e) { + //expected exception + } } } --------------------------------------------------------------------- To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-commits-h...@hadoop.apache.org