This is an automated email from the ASF dual-hosted git repository.

zhangduo pushed a commit to branch branch-2
in repository https://gitbox.apache.org/repos/asf/hbase.git


The following commit(s) were added to refs/heads/branch-2 by this push:
     new 12c6a539cfc HBASE-29365 Improve option parser of PerformanceEvaluation 
(#7056)
12c6a539cfc is described below

commit 12c6a539cfcef64bbd0952460b35c81e0fc51f81
Author: Junegunn Choi <[email protected]>
AuthorDate: Tue Jun 3 16:28:21 2025 +0900

    HBASE-29365 Improve option parser of PerformanceEvaluation (#7056)
    
    Signed-off-by: Duo Zhang <[email protected]>
---
 .../apache/hadoop/hbase/PerformanceEvaluation.java | 397 ++++++---------------
 .../hadoop/hbase/TestPerformanceEvaluation.java    |  27 ++
 2 files changed, 138 insertions(+), 286 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 10abf2daf95..a40af66fba2 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
@@ -22,6 +22,7 @@ import com.codahale.metrics.UniformReservoir;
 import io.opentelemetry.api.trace.Span;
 import io.opentelemetry.context.Scope;
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.PrintStream;
 import java.lang.reflect.Constructor;
 import java.math.BigDecimal;
@@ -31,6 +32,7 @@ import java.text.SimpleDateFormat;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Date;
+import java.util.HashMap;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Locale;
@@ -46,6 +48,7 @@ import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
 import java.util.concurrent.ThreadLocalRandom;
+import java.util.function.Consumer;
 import java.util.stream.Collectors;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.hadoop.conf.Configuration;
@@ -110,6 +113,7 @@ import org.apache.yetus.audience.InterfaceAudience;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import org.apache.hbase.thirdparty.com.google.common.base.Splitter;
 import 
org.apache.hbase.thirdparty.com.google.common.util.concurrent.ThreadFactoryBuilder;
 import org.apache.hbase.thirdparty.com.google.gson.Gson;
 
@@ -2819,9 +2823,79 @@ public class PerformanceEvaluation extends Configured 
implements Tool {
    * arguments.
    */
   static TestOptions parseOpts(Queue<String> args) {
-    TestOptions opts = new TestOptions();
+    final TestOptions opts = new TestOptions();
+
+    final Map<String, Consumer<Boolean>> flagHandlers = new HashMap<>();
+    flagHandlers.put("nomapred", v -> opts.nomapred = v);
+    flagHandlers.put("flushCommits", v -> opts.flushCommits = v);
+    flagHandlers.put("writeToWAL", v -> opts.writeToWAL = v);
+    flagHandlers.put("inmemory", v -> opts.inMemoryCF = v);
+    flagHandlers.put("autoFlush", v -> opts.autoFlush = v);
+    flagHandlers.put("oneCon", v -> opts.oneCon = v);
+    flagHandlers.put("latency", v -> opts.reportLatency = v);
+    flagHandlers.put("usetags", v -> opts.useTags = v);
+    flagHandlers.put("filterAll", v -> opts.filterAll = v);
+    flagHandlers.put("valueRandom", v -> opts.valueRandom = v);
+    flagHandlers.put("valueZipf", v -> opts.valueZipf = v);
+    flagHandlers.put("addColumns", v -> opts.addColumns = v);
+    flagHandlers.put("asyncPrefetch", v -> opts.asyncPrefetch = v);
+    flagHandlers.put("cacheBlocks", v -> opts.cacheBlocks = v);
+
+    final Map<String, Consumer<String>> handlers = new HashMap<>();
+    handlers.put("rows", v -> opts.perClientRunRows = Long.parseLong(v));
+    handlers.put("cycles", v -> opts.cycles = Integer.parseInt(v));
+    handlers.put("sampleRate", v -> opts.sampleRate = Float.parseFloat(v));
+    handlers.put("table", v -> opts.tableName = v);
+    handlers.put("startRow", v -> opts.startRow = Long.parseLong(v));
+    handlers.put("compress", v -> opts.compression = 
Compression.Algorithm.valueOf(v));
+    handlers.put("encryption", v -> opts.encryption = v);
+    handlers.put("traceRate", v -> opts.traceRate = Double.parseDouble(v));
+    handlers.put("blockEncoding", v -> opts.blockEncoding = 
DataBlockEncoding.valueOf(v));
+    handlers.put("presplit", v -> opts.presplitRegions = Integer.parseInt(v));
+    handlers.put("connCount", v -> opts.connCount = Integer.parseInt(v));
+    handlers.put("latencyThreshold", v -> opts.latencyThreshold = 
Integer.parseInt(v));
+    handlers.put("multiGet", v -> opts.multiGet = Integer.parseInt(v));
+    handlers.put("multiPut", v -> opts.multiPut = Integer.parseInt(v));
+    handlers.put("numoftags", v -> opts.noOfTags = Integer.parseInt(v));
+    handlers.put("replicas", v -> opts.replicas = Integer.parseInt(v));
+    handlers.put("size", v -> {
+      opts.size = Float.parseFloat(v);
+      if (opts.size <= 1.0f) {
+        throw new IllegalStateException("Size must be > 1; i.e. 1GB");
+      }
+    });
+    handlers.put("splitPolicy", v -> opts.splitPolicy = v);
+    handlers.put("randomSleep", v -> opts.randomSleep = Integer.parseInt(v));
+    handlers.put("measureAfter", v -> opts.measureAfter = Integer.parseInt(v));
+    handlers.put("bloomFilter", v -> opts.bloomType = BloomType.valueOf(v));
+    handlers.put("blockSize", v -> opts.blockSize = Integer.parseInt(v));
+    handlers.put("valueSize", v -> opts.valueSize = Integer.parseInt(v));
+    handlers.put("period", v -> opts.period = Integer.parseInt(v));
+    handlers.put("inmemoryCompaction",
+      v -> opts.inMemoryCompaction = MemoryCompactionPolicy.valueOf(v));
+    handlers.put("columns", v -> opts.columns = Integer.parseInt(v));
+    handlers.put("families", v -> opts.families = Integer.parseInt(v));
+    handlers.put("caching", v -> opts.caching = Integer.parseInt(v));
+    handlers.put("scanReadType", v -> opts.scanReadType = 
Scan.ReadType.valueOf(v.toUpperCase()));
+    handlers.put("bufferSize", v -> opts.bufferSize = Long.parseLong(v));
+    handlers.put("commandPropertiesFile", fileName -> {
+      try {
+        final Properties properties = new Properties();
+        final InputStream resourceStream =
+          
PerformanceEvaluation.class.getClassLoader().getResourceAsStream(fileName);
+        if (resourceStream == null) {
+          throw new IllegalArgumentException("Resource file not found: " + 
fileName);
+        }
+        properties.load(resourceStream);
+        opts.commandProperties = properties;
+      } catch (IOException e) {
+        throw new IllegalStateException("Failed to load command properties 
from file: " + fileName,
+          e);
+      }
+    });
 
     String cmd = null;
+    final Splitter splitter = Splitter.on("=").limit(2).trimResults();
     while ((cmd = args.poll()) != null) {
       if (cmd.equals("-h") || cmd.startsWith("--h")) {
         // place item back onto queue so that caller knows parsing was 
incomplete
@@ -2829,289 +2903,32 @@ public class PerformanceEvaluation extends Configured 
implements Tool {
         break;
       }
 
-      final String nmr = "--nomapred";
-      if (cmd.startsWith(nmr)) {
-        opts.nomapred = true;
-        continue;
-      }
-
-      final String rows = "--rows=";
-      if (cmd.startsWith(rows)) {
-        opts.perClientRunRows = Long.parseLong(cmd.substring(rows.length()));
-        continue;
-      }
-
-      final String cycles = "--cycles=";
-      if (cmd.startsWith(cycles)) {
-        opts.cycles = Integer.parseInt(cmd.substring(cycles.length()));
-        continue;
-      }
-
-      final String sampleRate = "--sampleRate=";
-      if (cmd.startsWith(sampleRate)) {
-        opts.sampleRate = Float.parseFloat(cmd.substring(sampleRate.length()));
-        continue;
-      }
-
-      final String table = "--table=";
-      if (cmd.startsWith(table)) {
-        opts.tableName = cmd.substring(table.length());
-        continue;
-      }
-
-      final String startRow = "--startRow=";
-      if (cmd.startsWith(startRow)) {
-        opts.startRow = Long.parseLong(cmd.substring(startRow.length()));
-        continue;
-      }
-
-      final String compress = "--compress=";
-      if (cmd.startsWith(compress)) {
-        opts.compression = 
Compression.Algorithm.valueOf(cmd.substring(compress.length()));
-        continue;
-      }
-
-      final String encryption = "--encryption=";
-      if (cmd.startsWith(encryption)) {
-        opts.encryption = cmd.substring(encryption.length());
-        continue;
-      }
-
-      final String traceRate = "--traceRate=";
-      if (cmd.startsWith(traceRate)) {
-        opts.traceRate = Double.parseDouble(cmd.substring(traceRate.length()));
-        continue;
-      }
-
-      final String blockEncoding = "--blockEncoding=";
-      if (cmd.startsWith(blockEncoding)) {
-        opts.blockEncoding = 
DataBlockEncoding.valueOf(cmd.substring(blockEncoding.length()));
-        continue;
-      }
-
-      final String flushCommits = "--flushCommits=";
-      if (cmd.startsWith(flushCommits)) {
-        opts.flushCommits = 
Boolean.parseBoolean(cmd.substring(flushCommits.length()));
-        continue;
-      }
-
-      final String writeToWAL = "--writeToWAL=";
-      if (cmd.startsWith(writeToWAL)) {
-        opts.writeToWAL = 
Boolean.parseBoolean(cmd.substring(writeToWAL.length()));
-        continue;
-      }
-
-      final String presplit = "--presplit=";
-      if (cmd.startsWith(presplit)) {
-        opts.presplitRegions = 
Integer.parseInt(cmd.substring(presplit.length()));
-        continue;
-      }
-
-      final String inMemory = "--inmemory=";
-      if (cmd.startsWith(inMemory)) {
-        opts.inMemoryCF = 
Boolean.parseBoolean(cmd.substring(inMemory.length()));
-        continue;
-      }
-
-      final String autoFlush = "--autoFlush=";
-      if (cmd.startsWith(autoFlush)) {
-        opts.autoFlush = 
Boolean.parseBoolean(cmd.substring(autoFlush.length()));
-        continue;
-      }
-
-      final String onceCon = "--oneCon=";
-      if (cmd.startsWith(onceCon)) {
-        opts.oneCon = Boolean.parseBoolean(cmd.substring(onceCon.length()));
-        continue;
-      }
-
-      final String connCount = "--connCount=";
-      if (cmd.startsWith(connCount)) {
-        opts.connCount = Integer.parseInt(cmd.substring(connCount.length()));
-        continue;
-      }
-
-      final String latencyThreshold = "--latencyThreshold=";
-      if (cmd.startsWith(latencyThreshold)) {
-        opts.latencyThreshold = 
Integer.parseInt(cmd.substring(latencyThreshold.length()));
-        continue;
-      }
-
-      final String latency = "--latency";
-      if (cmd.startsWith(latency)) {
-        opts.reportLatency = true;
-        continue;
-      }
-
-      final String multiGet = "--multiGet=";
-      if (cmd.startsWith(multiGet)) {
-        opts.multiGet = Integer.parseInt(cmd.substring(multiGet.length()));
-        continue;
-      }
-
-      final String multiPut = "--multiPut=";
-      if (cmd.startsWith(multiPut)) {
-        opts.multiPut = Integer.parseInt(cmd.substring(multiPut.length()));
-        continue;
-      }
-
-      final String useTags = "--usetags=";
-      if (cmd.startsWith(useTags)) {
-        opts.useTags = Boolean.parseBoolean(cmd.substring(useTags.length()));
-        continue;
-      }
-
-      final String noOfTags = "--numoftags=";
-      if (cmd.startsWith(noOfTags)) {
-        opts.noOfTags = Integer.parseInt(cmd.substring(noOfTags.length()));
-        continue;
-      }
-
-      final String replicas = "--replicas=";
-      if (cmd.startsWith(replicas)) {
-        opts.replicas = Integer.parseInt(cmd.substring(replicas.length()));
-        continue;
-      }
-
-      final String filterOutAll = "--filterAll";
-      if (cmd.startsWith(filterOutAll)) {
-        opts.filterAll = true;
-        continue;
-      }
+      if (cmd.startsWith("--")) {
+        final List<String> parts = splitter.splitToList(cmd.substring(2));
+        final String key = parts.get(0);
 
-      final String size = "--size=";
-      if (cmd.startsWith(size)) {
-        opts.size = Float.parseFloat(cmd.substring(size.length()));
-        if (opts.size <= 1.0f) throw new IllegalStateException("Size must be > 
1; i.e. 1GB");
-        continue;
-      }
-
-      final String splitPolicy = "--splitPolicy=";
-      if (cmd.startsWith(splitPolicy)) {
-        opts.splitPolicy = cmd.substring(splitPolicy.length());
-        continue;
-      }
-
-      final String randomSleep = "--randomSleep=";
-      if (cmd.startsWith(randomSleep)) {
-        opts.randomSleep = 
Integer.parseInt(cmd.substring(randomSleep.length()));
-        continue;
-      }
-
-      final String measureAfter = "--measureAfter=";
-      if (cmd.startsWith(measureAfter)) {
-        opts.measureAfter = 
Integer.parseInt(cmd.substring(measureAfter.length()));
-        continue;
-      }
-
-      final String bloomFilter = "--bloomFilter=";
-      if (cmd.startsWith(bloomFilter)) {
-        opts.bloomType = 
BloomType.valueOf(cmd.substring(bloomFilter.length()));
-        continue;
-      }
-
-      final String blockSize = "--blockSize=";
-      if (cmd.startsWith(blockSize)) {
-        opts.blockSize = Integer.parseInt(cmd.substring(blockSize.length()));
-        continue;
-      }
-
-      final String valueSize = "--valueSize=";
-      if (cmd.startsWith(valueSize)) {
-        opts.valueSize = Integer.parseInt(cmd.substring(valueSize.length()));
-        continue;
-      }
-
-      final String valueRandom = "--valueRandom";
-      if (cmd.startsWith(valueRandom)) {
-        opts.valueRandom = true;
-        continue;
-      }
-
-      final String valueZipf = "--valueZipf";
-      if (cmd.startsWith(valueZipf)) {
-        opts.valueZipf = true;
-        continue;
-      }
-
-      final String period = "--period=";
-      if (cmd.startsWith(period)) {
-        opts.period = Integer.parseInt(cmd.substring(period.length()));
-        continue;
-      }
-
-      final String addColumns = "--addColumns=";
-      if (cmd.startsWith(addColumns)) {
-        opts.addColumns = 
Boolean.parseBoolean(cmd.substring(addColumns.length()));
-        continue;
-      }
-
-      final String inMemoryCompaction = "--inmemoryCompaction=";
-      if (cmd.startsWith(inMemoryCompaction)) {
-        opts.inMemoryCompaction =
-          
MemoryCompactionPolicy.valueOf(cmd.substring(inMemoryCompaction.length()));
-        continue;
-      }
-
-      final String columns = "--columns=";
-      if (cmd.startsWith(columns)) {
-        opts.columns = Integer.parseInt(cmd.substring(columns.length()));
-        continue;
-      }
-
-      final String families = "--families=";
-      if (cmd.startsWith(families)) {
-        opts.families = Integer.parseInt(cmd.substring(families.length()));
-        continue;
-      }
-
-      final String caching = "--caching=";
-      if (cmd.startsWith(caching)) {
-        opts.caching = Integer.parseInt(cmd.substring(caching.length()));
-        continue;
-      }
-
-      final String asyncPrefetch = "--asyncPrefetch";
-      if (cmd.startsWith(asyncPrefetch)) {
-        opts.asyncPrefetch = true;
-        continue;
-      }
-
-      final String cacheBlocks = "--cacheBlocks=";
-      if (cmd.startsWith(cacheBlocks)) {
-        opts.cacheBlocks = 
Boolean.parseBoolean(cmd.substring(cacheBlocks.length()));
-        continue;
-      }
-
-      final String scanReadType = "--scanReadType=";
-      if (cmd.startsWith(scanReadType)) {
-        opts.scanReadType =
-          
Scan.ReadType.valueOf(cmd.substring(scanReadType.length()).toUpperCase());
-        continue;
-      }
-
-      final String bufferSize = "--bufferSize=";
-      if (cmd.startsWith(bufferSize)) {
-        opts.bufferSize = Long.parseLong(cmd.substring(bufferSize.length()));
-        continue;
-      }
-
-      final String commandPropertiesFile = "--commandPropertiesFile=";
-      if (cmd.startsWith(commandPropertiesFile)) {
-        String fileName = 
String.valueOf(cmd.substring(commandPropertiesFile.length()));
-        Properties properties = new Properties();
         try {
-          properties
-            
.load(PerformanceEvaluation.class.getClassLoader().getResourceAsStream(fileName));
-          opts.commandProperties = properties;
-        } catch (IOException e) {
-          LOG.error("Failed to load metricIds from properties file", e);
+          // Boolean options can be specified as --flag or --flag=true/false
+          final Consumer<Boolean> flagHandler = flagHandlers.get(key);
+          if (flagHandler != null) {
+            flagHandler.accept(parts.size() > 1 ? parseBoolean(parts.get(1)) : 
true);
+            continue;
+          }
+
+          // Options that require a value followed by an equals sign
+          final Consumer<String> handler = handlers.get(key);
+          if (handler != null) {
+            if (parts.size() < 2) {
+              throw new IllegalArgumentException("--" + key + " requires a 
value");
+            }
+            handler.accept(parts.get(1));
+            continue;
+          }
+        } catch (RuntimeException e) {
+          throw new IllegalArgumentException("Invalid option: " + cmd, e);
         }
-        continue;
       }
 
-      validateParsedOpts(opts);
-
       if (isCommandClass(cmd)) {
         opts.cmdName = cmd;
         try {
@@ -3119,20 +2936,28 @@ public class PerformanceEvaluation extends Configured 
implements Tool {
         } catch (NoSuchElementException | NumberFormatException e) {
           throw new IllegalArgumentException("Command " + cmd + " does not 
have threads number", e);
         }
-        opts = calculateRowsAndSize(opts);
+        calculateRowsAndSize(opts);
         break;
-      } else {
-        printUsageAndExit("ERROR: Unrecognized option/command: " + cmd, -1);
       }
 
-      // Not matching any option or command.
-      System.err.println("Error: Wrong option or command: " + cmd);
-      args.add(cmd);
-      break;
+      printUsageAndExit("ERROR: Unrecognized option/command: " + cmd, -1);
     }
+
+    validateParsedOpts(opts);
     return opts;
   }
 
+  // Boolean.parseBoolean is not strict enough for our needs, so we implement 
our own
+  private static boolean parseBoolean(String value) {
+    if ("true".equalsIgnoreCase(value)) {
+      return true;
+    }
+    if ("false".equalsIgnoreCase(value)) {
+      return false;
+    }
+    throw new IllegalArgumentException("Invalid boolean value: " + value);
+  }
+
   /**
    * Validates opts after all the opts are parsed, so that caller need not to 
maintain order of opts
    */
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 1e4493a4700..40bcd045c0f 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
@@ -20,6 +20,7 @@ package org.apache.hadoop.hbase;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
@@ -401,4 +402,30 @@ public class TestPerformanceEvaluation {
       return false;
     }
   }
+
+  @Test
+  public void testParseBooleanFlags() {
+    final Queue<String> opts = new LinkedList<>();
+    opts.offer("--valueRandom");
+    opts.offer("--autoFlush"); // default: false
+    opts.offer("--inmemory=true"); // default: false
+    opts.offer("--writeToWAL=false"); // default: true
+    opts.offer(PerformanceEvaluation.RANDOM_READ);
+    opts.offer("1");
+
+    final PerformanceEvaluation.TestOptions options = 
PerformanceEvaluation.parseOpts(opts);
+    assertTrue(options.valueRandom);
+    assertTrue(options.autoFlush);
+    assertTrue(options.inMemoryCF);
+    assertFalse(options.writeToWAL);
+    assertEquals(PerformanceEvaluation.RANDOM_READ, options.getCmdName());
+    assertEquals(1, options.getNumClientThreads());
+  }
+
+  @Test
+  public void testOptionMissingValue() {
+    final Queue<String> opts = new LinkedList<>();
+    opts.offer("--presplit");
+    assertThrows(IllegalArgumentException.class, () -> 
PerformanceEvaluation.parseOpts(opts));
+  }
 }

Reply via email to