From c718fa236502cdd0a5c347d654c4b0f908c07cc9 Mon Sep 17 00:00:00 2001 From: Sambit Mohapatra Date: Mon, 8 Jun 2020 23:02:39 -0700 Subject: [PATCH] HBASE-24340 PerformanceEvaluation options should not mandate any specific order Signed-off-by Anoop Sam John --- .../hadoop/hbase/PerformanceEvaluation.java | 41 +++++----- .../hbase/TestPerformanceEvaluation.java | 77 ++++++++++++++++++- 2 files changed, 95 insertions(+), 23 deletions(-) diff --git a/hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/PerformanceEvaluation.java b/hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/PerformanceEvaluation.java index cb6dc1b354c..b7f3a3e7e4e 100644 --- a/hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/PerformanceEvaluation.java +++ b/hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/PerformanceEvaluation.java @@ -2616,29 +2616,18 @@ public class PerformanceEvaluation extends Configured implements Tool { final String autoFlush = "--autoFlush="; if (cmd.startsWith(autoFlush)) { opts.autoFlush = Boolean.parseBoolean(cmd.substring(autoFlush.length())); - if (!opts.autoFlush && opts.multiPut > 0) { - throw new IllegalArgumentException("autoFlush must be true when multiPut is more than 0"); - } continue; } final String onceCon = "--oneCon="; if (cmd.startsWith(onceCon)) { opts.oneCon = Boolean.parseBoolean(cmd.substring(onceCon.length())); - if (opts.oneCon && opts.connCount > 1) { - throw new IllegalArgumentException("oneCon is set to true, " - + "connCount should not bigger than 1"); - } continue; } final String connCount = "--connCount="; if (cmd.startsWith(connCount)) { opts.connCount = Integer.parseInt(cmd.substring(connCount.length())); - if (opts.oneCon && opts.connCount > 1) { - throw new IllegalArgumentException("oneCon is set to true, " - + "connCount should not bigger than 1"); - } continue; } @@ -2657,9 +2646,6 @@ public class PerformanceEvaluation extends Configured implements Tool { final String multiPut = "--multiPut="; if (cmd.startsWith(multiPut)) { opts.multiPut = Integer.parseInt(cmd.substring(multiPut.length())); - if (!opts.autoFlush && opts.multiPut > 0) { - throw new IllegalArgumentException("autoFlush must be true when multiPut is more than 0"); - } continue; } @@ -2733,18 +2719,12 @@ public class PerformanceEvaluation extends Configured implements Tool { final String valueRandom = "--valueRandom"; if (cmd.startsWith(valueRandom)) { opts.valueRandom = true; - if (opts.valueZipf) { - throw new IllegalStateException("Either valueZipf or valueRandom but not both"); - } continue; } final String valueZipf = "--valueZipf"; if (cmd.startsWith(valueZipf)) { opts.valueZipf = true; - if (opts.valueRandom) { - throw new IllegalStateException("Either valueZipf or valueRandom but not both"); - } continue; } @@ -2810,6 +2790,8 @@ public class PerformanceEvaluation extends Configured implements Tool { continue; } + validateParsedOpts(opts); + if (isCommandClass(cmd)) { opts.cmdName = cmd; try { @@ -2831,6 +2813,25 @@ public class PerformanceEvaluation extends Configured implements Tool { return opts; } + /** + * Validates opts after all the opts are parsed, so that caller need not to maintain order of opts + */ + private static void validateParsedOpts(TestOptions opts) { + + if (!opts.autoFlush && opts.multiPut > 0) { + throw new IllegalArgumentException("autoFlush must be true when multiPut is more than 0"); + } + + if (opts.oneCon && opts.connCount > 1) { + throw new IllegalArgumentException("oneCon is set to true, " + + "connCount should not bigger than 1"); + } + + if (opts.valueZipf && opts.valueRandom) { + throw new IllegalStateException("Either valueZipf or valueRandom but not both"); + } + } + static TestOptions calculateRowsAndSize(final TestOptions opts) { int rowsPerGB = getRowsPerGB(opts); if ((opts.getCmdName() != null diff --git a/hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/TestPerformanceEvaluation.java b/hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/TestPerformanceEvaluation.java index 140bd4bf2bc..79fefbf4daa 100644 --- a/hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/TestPerformanceEvaluation.java +++ b/hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/TestPerformanceEvaluation.java @@ -257,8 +257,14 @@ public class TestPerformanceEvaluation { } catch (IllegalArgumentException e) { System.out.println(e.getMessage()); } - ((LinkedList) opts).offerFirst("--multiPut=10"); - ((LinkedList) opts).offerFirst("--autoFlush=true"); + + //Re-create options + opts = new LinkedList<>(); + opts.offer("--autoFlush=true"); + opts.offer("--multiPut=10"); + opts.offer(cmdName); + opts.offer("64"); + options = PerformanceEvaluation.parseOpts(opts); assertNotNull(options); assertNotNull(options.getCmdName()); @@ -266,6 +272,36 @@ public class TestPerformanceEvaluation { assertEquals(10, options.getMultiPut()); } + @Test + public void testParseOptsMultiPutsAndAutoFlushOrder() { + Queue opts = new LinkedList<>(); + String cmdName = "sequentialWrite"; + String cmdMultiPut = "--multiPut=10"; + String cmdAutoFlush = "--autoFlush=true"; + opts.offer(cmdAutoFlush); + opts.offer(cmdMultiPut); + opts.offer(cmdName); + opts.offer("64"); + PerformanceEvaluation.TestOptions options = null; + options = PerformanceEvaluation.parseOpts(opts); + assertNotNull(options); + assertEquals(true, options.autoFlush); + assertEquals(10, options.getMultiPut()); + + // Change the order of AutoFlush and Multiput + opts = new LinkedList<>(); + opts.offer(cmdMultiPut); + opts.offer(cmdAutoFlush); + opts.offer(cmdName); + opts.offer("64"); + + options = null; + options = PerformanceEvaluation.parseOpts(opts); + assertNotNull(options); + assertEquals(10, options.getMultiPut()); + assertEquals(true, options.autoFlush); + } + @Test public void testParseOptsConnCount() { Queue opts = new LinkedList<>(); @@ -281,11 +317,46 @@ public class TestPerformanceEvaluation { } catch (IllegalArgumentException e) { System.out.println(e.getMessage()); } - ((LinkedList) opts).offerFirst("--connCount=10"); + + opts = new LinkedList<>(); + opts.offer("--connCount=10"); + opts.offer(cmdName); + opts.offer("64"); + options = PerformanceEvaluation.parseOpts(opts); assertNotNull(options); assertNotNull(options.getCmdName()); assertEquals(cmdName, options.getCmdName()); assertEquals(10, options.getConnCount()); } + + @Test + public void testParseOptsValueRandom() { + Queue opts = new LinkedList<>(); + String cmdName = "sequentialWrite"; + opts.offer("--valueRandom"); + opts.offer("--valueZipf"); + opts.offer(cmdName); + opts.offer("64"); + PerformanceEvaluation.TestOptions options = null; + try { + options = PerformanceEvaluation.parseOpts(opts); + fail("should fail"); + } catch (IllegalStateException e) { + System.out.println(e.getMessage()); + } + + opts = new LinkedList<>(); + opts.offer("--valueRandom"); + opts.offer(cmdName); + opts.offer("64"); + + options = PerformanceEvaluation.parseOpts(opts); + + assertNotNull(options); + assertNotNull(options.getCmdName()); + assertEquals(cmdName, options.getCmdName()); + assertEquals(true, options.valueRandom); + } + }