From 91794728d5462da08a76c3cdf14df6c1238d5009 Mon Sep 17 00:00:00 2001 From: Tsz-wo Sze Date: Fri, 1 Aug 2014 14:15:57 +0000 Subject: [PATCH] svn merge -c 1615107 from trunk for HDFS-6796. Improve the argument check during balancer command line parsing. git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/branch-2@1615108 13f79535-47bb-0310-9956-ffa450edef68 --- hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt | 3 ++ .../hadoop/hdfs/server/balancer/Balancer.java | 33 +++++++++++-------- .../hdfs/server/balancer/TestBalancer.java | 31 ++++++++++++++++- 3 files changed, 53 insertions(+), 14 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index c8f5acceae2..a65a82a9c6a 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -88,6 +88,9 @@ Release 2.6.0 - UNRELEASED HDFS-6798. Add test case for incorrect data node condition during balancing. (Benoy Antony via Arpit Agarwal) + HDFS-6796. Improve the argument check during balancer command line parsing. + (Benoy Antony via szetszwo) + OPTIMIZATIONS HDFS-6690. Deduplicate xattr names in memory. (wang) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/balancer/Balancer.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/balancer/Balancer.java index cfbe6a9c45a..62c5c51ffbf 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/balancer/Balancer.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/balancer/Balancer.java @@ -1762,9 +1762,9 @@ public class Balancer { if (args != null) { try { for(int i = 0; i < args.length; i++) { - checkArgument(args.length >= 2, "args = " + Arrays.toString(args)); if ("-threshold".equalsIgnoreCase(args[i])) { - i++; + checkArgument(++i < args.length, + "Threshold value is missing: args = " + Arrays.toString(args)); try { threshold = Double.parseDouble(args[i]); if (threshold < 1 || threshold > 100) { @@ -1779,7 +1779,8 @@ public class Balancer { throw e; } } else if ("-policy".equalsIgnoreCase(args[i])) { - i++; + checkArgument(++i < args.length, + "Policy value is missing: args = " + Arrays.toString(args)); try { policy = BalancingPolicy.parse(args[i]); } catch(IllegalArgumentException e) { @@ -1787,16 +1788,26 @@ public class Balancer { throw e; } } else if ("-exclude".equalsIgnoreCase(args[i])) { - i++; + checkArgument(++i < args.length, + "List of nodes to exclude | -f is missing: args = " + + Arrays.toString(args)); if ("-f".equalsIgnoreCase(args[i])) { - nodesTobeExcluded = Util.getHostListFromFile(args[++i]); + checkArgument(++i < args.length, + "File containing nodes to exclude is not specified: args = " + + Arrays.toString(args)); + nodesTobeExcluded = Util.getHostListFromFile(args[i]); } else { nodesTobeExcluded = Util.parseHostList(args[i]); } } else if ("-include".equalsIgnoreCase(args[i])) { - i++; + checkArgument(++i < args.length, + "List of nodes to include | -f is missing: args = " + + Arrays.toString(args)); if ("-f".equalsIgnoreCase(args[i])) { - nodesTobeIncluded = Util.getHostListFromFile(args[++i]); + checkArgument(++i < args.length, + "File containing nodes to include is not specified: args = " + + Arrays.toString(args)); + nodesTobeIncluded = Util.getHostListFromFile(args[i]); } else { nodesTobeIncluded = Util.parseHostList(args[i]); } @@ -1805,12 +1816,8 @@ public class Balancer { + Arrays.toString(args)); } } - if (!nodesTobeExcluded.isEmpty() && !nodesTobeIncluded.isEmpty()) { - System.err.println( - "-exclude and -include options cannot be specified together."); - throw new IllegalArgumentException( - "-exclude and -include options cannot be specified together."); - } + checkArgument(nodesTobeExcluded.isEmpty() || nodesTobeIncluded.isEmpty(), + "-exclude and -include options cannot be specified together."); } catch(RuntimeException e) { printUsage(System.err); throw e; diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/balancer/TestBalancer.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/balancer/TestBalancer.java index 1b9b4309710..c8933df26b7 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/balancer/TestBalancer.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/balancer/TestBalancer.java @@ -867,13 +867,42 @@ public class TestBalancer { } catch (IllegalArgumentException e) { } - parameters = new String[] { "-threshold 1 -policy" }; + parameters = new String[] {"-threshold", "1", "-policy"}; try { Balancer.Cli.parse(parameters); fail(reason); } catch (IllegalArgumentException e) { } + parameters = new String[] {"-threshold", "1", "-include"}; + try { + Balancer.Cli.parse(parameters); + fail(reason); + } catch (IllegalArgumentException e) { + + } + parameters = new String[] {"-threshold", "1", "-exclude"}; + try { + Balancer.Cli.parse(parameters); + fail(reason); + } catch (IllegalArgumentException e) { + + } + parameters = new String[] {"-include", "-f"}; + try { + Balancer.Cli.parse(parameters); + fail(reason); + } catch (IllegalArgumentException e) { + + } + parameters = new String[] {"-exclude", "-f"}; + try { + Balancer.Cli.parse(parameters); + fail(reason); + } catch (IllegalArgumentException e) { + + } + parameters = new String[] {"-include", "testnode1", "-exclude", "testnode2"}; try { Balancer.Cli.parse(parameters);