Repository: hadoop Updated Branches: refs/heads/branch-2 7dc558b6a -> a72ef921f
HDFS-9445. Datanode may deadlock while handling a bad volume. Contributed by Walter Su. (cherry picked from commit a48301791e9564363bc2abad4e89e344b0d7a5ff) Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/a72ef921 Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/a72ef921 Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/a72ef921 Branch: refs/heads/branch-2 Commit: a72ef921f00f6536bcb3e09ef5bd147cd8890065 Parents: 7dc558b Author: Kihwal Lee <kih...@apache.org> Authored: Fri Dec 11 08:46:03 2015 -0600 Committer: Kihwal Lee <kih...@apache.org> Committed: Fri Dec 11 08:46:03 2015 -0600 ---------------------------------------------------------------------- hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt | 3 + .../datanode/fsdataset/impl/FsDatasetImpl.java | 89 ++++++++++++-------- .../fsdataset/impl/TestFsDatasetImpl.java | 4 + 3 files changed, 59 insertions(+), 37 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/a72ef921/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 3fd5231..d2eed68 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -1709,6 +1709,9 @@ Release 2.7.2 - UNRELEASED HDFS-9294. DFSClient deadlock when close file and failed to renew lease. (Brahma Reddy Battula via szetszwo) + HDFS-9445. Datanode may deadlock while handling a bad volume. + (Wlater Su via Kihwal) + Release 2.7.1 - 2015-07-06 INCOMPATIBLE CHANGES http://git-wip-us.apache.org/repos/asf/hadoop/blob/a72ef921/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java index a896287..cb5af93 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java @@ -475,48 +475,67 @@ class FsDatasetImpl implements FsDatasetSpi<FsVolumeImpl> { * Removes a set of volumes from FsDataset. * @param volumesToRemove a set of absolute root path of each volume. * @param clearFailure set true to clear failure information. - * - * DataNode should call this function before calling - * {@link DataStorage#removeVolumes(java.util.Collection)}. */ @Override - public synchronized void removeVolumes( - Set<File> volumesToRemove, boolean clearFailure) { + public void removeVolumes(Set<File> volumesToRemove, boolean clearFailure) { // Make sure that all volumes are absolute path. for (File vol : volumesToRemove) { Preconditions.checkArgument(vol.isAbsolute(), String.format("%s is not absolute path.", vol.getPath())); } - for (int idx = 0; idx < dataStorage.getNumStorageDirs(); idx++) { - Storage.StorageDirectory sd = dataStorage.getStorageDir(idx); - final File absRoot = sd.getRoot().getAbsoluteFile(); - if (volumesToRemove.contains(absRoot)) { - LOG.info("Removing " + absRoot + " from FsDataset."); - - // Disable the volume from the service. - asyncDiskService.removeVolume(sd.getCurrentDir()); - volumes.removeVolume(absRoot, clearFailure); - - // Removed all replica information for the blocks on the volume. Unlike - // updating the volumeMap in addVolume(), this operation does not scan - // disks. - for (String bpid : volumeMap.getBlockPoolList()) { - for (Iterator<ReplicaInfo> it = volumeMap.replicas(bpid).iterator(); - it.hasNext(); ) { - ReplicaInfo block = it.next(); - final File absBasePath = - new File(block.getVolume().getBasePath()).getAbsoluteFile(); - if (absBasePath.equals(absRoot)) { - invalidate(bpid, block); - it.remove(); + + Map<String, List<ReplicaInfo>> blkToInvalidate = new HashMap<>(); + List<String> storageToRemove = new ArrayList<>(); + synchronized (this) { + for (int idx = 0; idx < dataStorage.getNumStorageDirs(); idx++) { + Storage.StorageDirectory sd = dataStorage.getStorageDir(idx); + final File absRoot = sd.getRoot().getAbsoluteFile(); + if (volumesToRemove.contains(absRoot)) { + LOG.info("Removing " + absRoot + " from FsDataset."); + + // Disable the volume from the service. + asyncDiskService.removeVolume(sd.getCurrentDir()); + volumes.removeVolume(absRoot, clearFailure); + + // Removed all replica information for the blocks on the volume. + // Unlike updating the volumeMap in addVolume(), this operation does + // not scan disks. + for (String bpid : volumeMap.getBlockPoolList()) { + List<ReplicaInfo> blocks = new ArrayList<>(); + for (Iterator<ReplicaInfo> it = volumeMap.replicas(bpid).iterator(); + it.hasNext(); ) { + ReplicaInfo block = it.next(); + final File absBasePath = + new File(block.getVolume().getBasePath()).getAbsoluteFile(); + if (absBasePath.equals(absRoot)) { + blocks.add(block); + it.remove(); + } } + blkToInvalidate.put(bpid, blocks); } + + storageToRemove.add(sd.getStorageUuid()); } + } + setupAsyncLazyPersistThreads(); + } - storageMap.remove(sd.getStorageUuid()); + // Call this outside the lock. + for (Map.Entry<String, List<ReplicaInfo>> entry : + blkToInvalidate.entrySet()) { + String bpid = entry.getKey(); + List<ReplicaInfo> blocks = entry.getValue(); + for (ReplicaInfo block : blocks) { + invalidate(bpid, block); + } + } + + synchronized (this) { + for(String storageUuid : storageToRemove) { + storageMap.remove(storageUuid); } } - setupAsyncLazyPersistThreads(); } private StorageType getStorageTypeFromLocations( @@ -1936,15 +1955,11 @@ class FsDatasetImpl implements FsDatasetSpi<FsVolumeImpl> { public void invalidate(String bpid, ReplicaInfo block) { // If a DFSClient has the replica in its cache of short-circuit file // descriptors (and the client is using ShortCircuitShm), invalidate it. - // The short-circuit registry is null in the unit tests, because the - // datanode is mock object. - if (datanode.getShortCircuitRegistry() != null) { - datanode.getShortCircuitRegistry().processBlockInvalidation( - new ExtendedBlockId(block.getBlockId(), bpid)); + datanode.getShortCircuitRegistry().processBlockInvalidation( + new ExtendedBlockId(block.getBlockId(), bpid)); - // If the block is cached, start uncaching it. - cacheManager.uncacheBlock(bpid, block.getBlockId()); - } + // If the block is cached, start uncaching it. + cacheManager.uncacheBlock(bpid, block.getBlockId()); datanode.notifyNamenodeDeletedBlock(new ExtendedBlock(bpid, block), block.getStorageUuid()); http://git-wip-us.apache.org/repos/asf/hadoop/blob/a72ef921/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java index 62907ec..a3d5769 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java @@ -39,6 +39,7 @@ import org.apache.hadoop.hdfs.server.datanode.DataStorage; import org.apache.hadoop.hdfs.server.datanode.FinalizedReplica; import org.apache.hadoop.hdfs.server.datanode.ReplicaHandler; import org.apache.hadoop.hdfs.server.datanode.ReplicaInfo; +import org.apache.hadoop.hdfs.server.datanode.ShortCircuitRegistry; import org.apache.hadoop.hdfs.server.datanode.StorageLocation; import org.apache.hadoop.hdfs.server.datanode.fsdataset.FsDatasetSpi; import org.apache.hadoop.hdfs.server.datanode.fsdataset.FsVolumeReference; @@ -147,6 +148,9 @@ public class TestFsDatasetImpl { when(datanode.getDnConf()).thenReturn(dnConf); final BlockScanner disabledBlockScanner = new BlockScanner(datanode, conf); when(datanode.getBlockScanner()).thenReturn(disabledBlockScanner); + final ShortCircuitRegistry shortCircuitRegistry = + new ShortCircuitRegistry(conf); + when(datanode.getShortCircuitRegistry()).thenReturn(shortCircuitRegistry); createStorageDirs(storage, conf, NUM_INIT_VOLUMES); dataset = new FsDatasetImpl(datanode, storage, conf);
