HBASE-24340 PerformanceEvaluation options should not mandate any specific order

Signed-off-by Anoop Sam John <anoopsamjohn@apache.org>

hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/PerformanceEvaluation.java

@@ -2606,29 +2606,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;
       }
 
@@ -2647,9 +2636,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;
       }
 
@@ -2723,18 +2709,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;
       }
 
@@ -2800,6 +2780,8 @@ public class PerformanceEvaluation extends Configured implements Tool {
         continue;
       }
 
+      validateParsedOpts(opts);
+
       if (isCommandClass(cmd)) {
         opts.cmdName = cmd;
         try {
@@ -2821,6 +2803,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

hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/TestPerformanceEvaluation.java

@@ -255,8 +255,14 @@ public class TestPerformanceEvaluation {
     } catch (IllegalArgumentException e) {
       System.out.println(e.getMessage());
     }
-    ((LinkedList<String>) opts).offerFirst("--multiPut=10");
+
-    ((LinkedList<String>) 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());
@@ -264,6 +270,36 @@ public class TestPerformanceEvaluation {
     assertEquals(10, options.getMultiPut());
   }
 
+  @Test
+  public void testParseOptsMultiPutsAndAutoFlushOrder() {
+    Queue<String> 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<String> opts = new LinkedList<>();
@@ -279,11 +315,46 @@ public class TestPerformanceEvaluation {
     } catch (IllegalArgumentException e) {
       System.out.println(e.getMessage());
     }
-    ((LinkedList<String>) 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<String> 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);
+  }
+
 }