From 89a8edc0149e3f31a5ade9a0927c4b6332cf6b1a Mon Sep 17 00:00:00 2001 From: Akira Ajisaka Date: Tue, 16 May 2017 12:41:59 -0400 Subject: [PATCH] HDFS-11696. Fix warnings from Spotbugs in hadoop-hdfs. Contributed by Yiqun Lin. --- .../org/apache/hadoop/hdfs/DFSClient.java | 7 +++-- .../hdfs/server/protocol/SlowDiskReports.java | 5 ++-- .../dev-support/findbugsExcludeFile.xml | 5 ++++ .../hdfs/qjournal/server/JournalNode.java | 16 ++++++----- .../server/common/HdfsServerConstants.java | 7 ++++- .../hdfs/server/datanode/DataStorage.java | 12 ++++++--- .../namenode/NNStorageRetentionManager.java | 27 ++++++++++--------- .../apache/hadoop/hdfs/tools/DFSAdmin.java | 6 ++--- .../ImageLoaderCurrent.java | 10 ++++--- 9 files changed, 62 insertions(+), 33 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSClient.java b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSClient.java index d21b9b435f9..7de8b7121cf 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSClient.java +++ b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSClient.java @@ -2857,9 +2857,12 @@ public class DFSClient implements java.io.Closeable, RemotePeerFactory, } synchronized (DFSClient.class) { if (STRIPED_READ_THREAD_POOL == null) { - STRIPED_READ_THREAD_POOL = DFSUtilClient.getThreadPoolExecutor(1, + // Only after thread pool is fully constructed then save it to + // volatile field. + ThreadPoolExecutor threadPool = DFSUtilClient.getThreadPoolExecutor(1, numThreads, 60, "StripedRead-", true); - STRIPED_READ_THREAD_POOL.allowCoreThreadTimeOut(true); + threadPool.allowCoreThreadTimeOut(true); + STRIPED_READ_THREAD_POOL = threadPool; } } } diff --git a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/server/protocol/SlowDiskReports.java b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/server/protocol/SlowDiskReports.java index 8095c2a690a..496389a1ddc 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/server/protocol/SlowDiskReports.java +++ b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/server/protocol/SlowDiskReports.java @@ -101,8 +101,9 @@ public final class SlowDiskReports { } boolean areEqual; - for (String disk : this.slowDisks.keySet()) { - if (!this.slowDisks.get(disk).equals(that.slowDisks.get(disk))) { + for (Map.Entry> entry : this.slowDisks + .entrySet()) { + if (!entry.getValue().equals(that.slowDisks.get(entry.getKey()))) { return false; } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/dev-support/findbugsExcludeFile.xml b/hadoop-hdfs-project/hadoop-hdfs/dev-support/findbugsExcludeFile.xml index be54efb2422..9270990e0b6 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/dev-support/findbugsExcludeFile.xml +++ b/hadoop-hdfs-project/hadoop-hdfs/dev-support/findbugsExcludeFile.xml @@ -252,4 +252,9 @@ + + + + + diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/qjournal/server/JournalNode.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/qjournal/server/JournalNode.java index 42e9be70ac5..ab724d5266b 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/qjournal/server/JournalNode.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/qjournal/server/JournalNode.java @@ -297,14 +297,18 @@ public class JournalNode implements Tool, Configurable, JournalNodeMXBean { return file.isDirectory(); } }); - for (File journalDir : journalDirs) { - String jid = journalDir.getName(); - if (!status.containsKey(jid)) { - Map jMap = new HashMap(); - jMap.put("Formatted", "true"); - status.put(jid, jMap); + + if (journalDirs != null) { + for (File journalDir : journalDirs) { + String jid = journalDir.getName(); + if (!status.containsKey(jid)) { + Map jMap = new HashMap(); + jMap.put("Formatted", "true"); + status.put(jid, jMap); + } } } + return JSON.toString(status); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/HdfsServerConstants.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/HdfsServerConstants.java index c3098f3cff0..ab50eb4012b 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/HdfsServerConstants.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/HdfsServerConstants.java @@ -188,8 +188,10 @@ public interface HdfsServerConstants { return NamenodeRole.NAMENODE; } } - + public void setClusterId(String cid) { + Preconditions.checkState(this == UPGRADE || this == UPGRADEONLY + || this == FORMAT); clusterId = cid; } @@ -214,6 +216,7 @@ public interface HdfsServerConstants { } public void setForce(int force) { + Preconditions.checkState(this == RECOVER); this.force = force; } @@ -226,6 +229,7 @@ public interface HdfsServerConstants { } public void setForceFormat(boolean force) { + Preconditions.checkState(this == FORMAT); isForceFormat = force; } @@ -234,6 +238,7 @@ public interface HdfsServerConstants { } public void setInteractiveFormat(boolean interactive) { + Preconditions.checkState(this == FORMAT); isInteractiveFormat = interactive; } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataStorage.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataStorage.java index 9a7108124f9..6d6e96a6995 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataStorage.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataStorage.java @@ -1336,10 +1336,14 @@ public class DataStorage extends Storage { return name.startsWith(BLOCK_SUBDIR_PREFIX); } }); - for(int i = 0; i < otherNames.length; i++) - linkBlocksHelper(new File(from, otherNames[i]), - new File(to, otherNames[i]), oldLV, hl, upgradeToIdBasedLayout, - blockRoot, idBasedLayoutSingleLinks); + + if (otherNames != null) { + for (int i = 0; i < otherNames.length; i++) { + linkBlocksHelper(new File(from, otherNames[i]), + new File(to, otherNames[i]), oldLV, hl, upgradeToIdBasedLayout, + blockRoot, idBasedLayoutSingleLinks); + } + } } /** diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NNStorageRetentionManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NNStorageRetentionManager.java index 98b7e9ac52c..2a83541d38a 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NNStorageRetentionManager.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NNStorageRetentionManager.java @@ -255,24 +255,27 @@ public class NNStorageRetentionManager { }); // Check whether there is any work to do. - if (filesInStorage.length <= numCheckpointsToRetain) { + if (filesInStorage != null + && filesInStorage.length <= numCheckpointsToRetain) { return; } // Create a sorted list of txids from the file names. TreeSet sortedTxIds = new TreeSet(); - for (String fName : filesInStorage) { - // Extract the transaction id from the file name. - long fTxId; - try { - fTxId = Long.parseLong(fName.substring(oivImagePrefix.length() + 1)); - } catch (NumberFormatException nfe) { - // This should not happen since we have already filtered it. - // Log and continue. - LOG.warn("Invalid file name. Skipping " + fName); - continue; + if (filesInStorage != null) { + for (String fName : filesInStorage) { + // Extract the transaction id from the file name. + long fTxId; + try { + fTxId = Long.parseLong(fName.substring(oivImagePrefix.length() + 1)); + } catch (NumberFormatException nfe) { + // This should not happen since we have already filtered it. + // Log and continue. + LOG.warn("Invalid file name. Skipping " + fName); + continue; + } + sortedTxIds.add(Long.valueOf(fTxId)); } - sortedTxIds.add(Long.valueOf(fTxId)); } int numFilesToDelete = sortedTxIds.size() - numCheckpointsToRetain; diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/DFSAdmin.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/DFSAdmin.java index 6d670893690..5375cd972eb 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/DFSAdmin.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/DFSAdmin.java @@ -1917,7 +1917,7 @@ public class DFSAdmin extends FsShell { return exitCode; } } else if ("-report".equals(cmd)) { - if (argv.length < 1) { + if (argv.length > 4) { printUsage(cmd); return exitCode; } @@ -1947,7 +1947,7 @@ public class DFSAdmin extends FsShell { return exitCode; } } else if (RollingUpgradeCommand.matches(cmd)) { - if (argv.length < 1 || argv.length > 2) { + if (argv.length > 2) { printUsage(cmd); return exitCode; } @@ -2022,7 +2022,7 @@ public class DFSAdmin extends FsShell { return exitCode; } } else if ("-triggerBlockReport".equals(cmd)) { - if (argv.length < 1) { + if ((argv.length != 2) && (argv.length != 3)) { printUsage(cmd); return exitCode; } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/ImageLoaderCurrent.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/ImageLoaderCurrent.java index f2c7427b51f..2e2eaf4e4d4 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/ImageLoaderCurrent.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/ImageLoaderCurrent.java @@ -722,9 +722,13 @@ class ImageLoaderCurrent implements ImageLoader { if (supportSnapshot && supportInodeId) { dirNodeMap.put(inodeId, pathName); } - v.visit(ImageElement.NS_QUOTA, numBlocks == -1 ? in.readLong() : -1); - if (NameNodeLayoutVersion.supports(Feature.DISKSPACE_QUOTA, imageVersion)) - v.visit(ImageElement.DS_QUOTA, numBlocks == -1 ? in.readLong() : -1); + + v.visit(ImageElement.NS_QUOTA, in.readLong()); + if (NameNodeLayoutVersion.supports(Feature.DISKSPACE_QUOTA, + imageVersion)) { + v.visit(ImageElement.DS_QUOTA, in.readLong()); + } + if (supportSnapshot) { boolean snapshottable = in.readBoolean(); if (!snapshottable) {