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 41662a40ad..a9592bfc29 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 BlocksWithLocations getBlocksWithLocations(final DatanodeID datanode, void removeBlocksAssociatedTo(final DatanodeDescriptor node) { for (DatanodeStorageInfo storage : node.getStorageInfos()) { final Iterator it = storage.getBlockIterator(); + //add the BlockInfos to a new collection as the + //returned iterator is not modifiable. + Collection 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 @@ void removeBlocksAssociatedTo(final DatanodeStorageInfo storageInfo) { assert namesystem.hasWriteLock(); final Iterator 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 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) { 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 ab666b7603..8af86d3920 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 @@ int numBlocks() { return blocks.size(); } + /** + * @return iterator to an unmodifiable set of blocks + * related to this {@link DatanodeStorageInfo} + */ Iterator getBlockIterator() { - return blocks.iterator(); + return Collections.unmodifiableSet(blocks).iterator(); } void updateState(StorageReport r) { 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 32bdd780ac..7397507041 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 @@ void testBlockIterator(MiniDFSCluster cluster) { 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 + } } }