Fix maxThreads option to be optional instead of required

ypeckstadt · ypeckstadt · commit 0956db644da3 · 2025-11-07T16:17:45.000+09:00
diff --git a/data-loader/cli/src/main/java/com/scalar/db/dataloader/cli/command/dataexport/ExportCommand.java b/data-loader/cli/src/main/java/com/scalar/db/dataloader/cli/command/dataexport/ExportCommand.java
@@ -61,7 +61,11 @@ public Integer call() throws Exception {
       FileUtils.validateFilePath(scalarDbPropertiesFilePath);
       validatePositiveValue(
           spec.commandLine(), dataChunkSize, DataLoaderError.INVALID_DATA_CHUNK_SIZE);
-      validatePositiveValue(spec.commandLine(), maxThreads, DataLoaderError.INVALID_MAX_THREADS);
+      if (maxThreads != null) {
+        validatePositiveValue(spec.commandLine(), maxThreads, DataLoaderError.INVALID_MAX_THREADS);
+      } else {
+        maxThreads = Runtime.getRuntime().availableProcessors();
+      }
 
       StorageFactory storageFactory = StorageFactory.create(scalarDbPropertiesFilePath);
       TableMetadataService metaDataService =
diff --git a/data-loader/cli/src/main/java/com/scalar/db/dataloader/cli/command/dataexport/ExportCommandOptions.java b/data-loader/cli/src/main/java/com/scalar/db/dataloader/cli/command/dataexport/ExportCommandOptions.java
@@ -77,7 +77,7 @@ public class ExportCommandOptions {
       paramLabel = "<MAX_THREADS>",
       description =
           "Maximum number of threads to use for parallel processing (default: number of available processors)")
-  protected int maxThreads;
+  protected Integer maxThreads;
 
   @CommandLine.Option(
       names = {"--start-key", "-sk"},
diff --git a/data-loader/cli/src/main/java/com/scalar/db/dataloader/cli/command/dataimport/ImportCommand.java b/data-loader/cli/src/main/java/com/scalar/db/dataloader/cli/command/dataimport/ImportCommand.java
@@ -61,7 +61,11 @@ public Integer call() throws Exception {
         spec.commandLine(), dataChunkSize, DataLoaderError.INVALID_DATA_CHUNK_SIZE);
     validatePositiveValue(
         spec.commandLine(), transactionSize, DataLoaderError.INVALID_TRANSACTION_SIZE);
-    validatePositiveValue(spec.commandLine(), maxThreads, DataLoaderError.INVALID_MAX_THREADS);
+    if (maxThreads != null) {
+      validatePositiveValue(spec.commandLine(), maxThreads, DataLoaderError.INVALID_MAX_THREADS);
+    } else {
+      maxThreads = Runtime.getRuntime().availableProcessors();
+    }
     validatePositiveValue(
         spec.commandLine(), dataChunkQueueSize, DataLoaderError.INVALID_DATA_CHUNK_QUEUE_SIZE);
     ControlFile controlFile = parseControlFileFromPath(controlFilePath).orElse(null);
diff --git a/data-loader/cli/src/main/java/com/scalar/db/dataloader/cli/command/dataimport/ImportCommandOptions.java b/data-loader/cli/src/main/java/com/scalar/db/dataloader/cli/command/dataimport/ImportCommandOptions.java
@@ -38,9 +38,8 @@ public class ImportCommandOptions {
       names = {"--max-threads", "-mt"},
       paramLabel = "<MAX_THREADS>",
       description =
-          "Maximum number of threads to use for parallel processing (default: number of available processors)",
-      defaultValue = "16")
-  protected int maxThreads;
+          "Maximum number of threads to use for parallel processing (default: number of available processors)")
+  protected Integer maxThreads;
 
   // Deprecated option - kept for backward compatibility
   @CommandLine.Option(
diff --git a/data-loader/cli/src/test/java/com/scalar/db/dataloader/cli/command/dataexport/ExportCommandTest.java b/data-loader/cli/src/test/java/com/scalar/db/dataloader/cli/command/dataexport/ExportCommandTest.java
@@ -190,4 +190,58 @@ void call_withOnlyDeprecatedEndExclusive_shouldApplyInvertedValue() {
     // end-exclusive=false should become end-inclusive=true
     assertEquals(true, command.scanEndInclusive);
   }
+
+  @Test
+  void call_withMaxThreadsSpecified_shouldUseSpecifiedValue() {
+    // Simulate command line parsing with --max-threads
+    String[] args = {
+      "--config",
+      "scalardb.properties",
+      "--namespace",
+      "scalar",
+      "--table",
+      "asset",
+      "--format",
+      "JSON",
+      "--max-threads",
+      "8"
+    };
+    ExportCommand command = new ExportCommand();
+    CommandLine cmd = new CommandLine(command);
+    cmd.parseArgs(args);
+
+    // Verify the value was parsed
+    assertEquals(8, command.maxThreads);
+  }
+
+  @Test
+  void call_withoutMaxThreads_shouldDefaultToAvailableProcessors() {
+    // Simulate command line parsing without --max-threads
+    String[] args = {
+      "--config",
+      "scalardb.properties",
+      "--namespace",
+      "scalar",
+      "--table",
+      "asset",
+      "--format",
+      "JSON"
+    };
+    ExportCommand command = new ExportCommand();
+    CommandLine cmd = new CommandLine(command);
+    cmd.parseArgs(args);
+
+    // Verify maxThreads is null before validation
+    assertEquals(null, command.maxThreads);
+
+    // Simulate what happens in call() after validation
+    command.spec = cmd.getCommandSpec();
+    command.applyDeprecatedOptions();
+    if (command.maxThreads == null) {
+      command.maxThreads = Runtime.getRuntime().availableProcessors();
+    }
+
+    // Verify it was set to available processors
+    assertEquals(Runtime.getRuntime().availableProcessors(), command.maxThreads);
+  }
 }
diff --git a/data-loader/cli/src/test/java/com/scalar/db/dataloader/cli/command/dataimport/ImportCommandTest.java b/data-loader/cli/src/test/java/com/scalar/db/dataloader/cli/command/dataimport/ImportCommandTest.java
@@ -149,4 +149,54 @@ void call_withOnlyDeprecatedThreads_shouldApplyValue() throws Exception {
     // Verify the value was applied to maxThreads
     assertEquals(12, command.maxThreads);
   }
+
+  @Test
+  void call_withMaxThreadsSpecified_shouldUseSpecifiedValue() {
+    // Simulate command line parsing with --max-threads
+    String[] args = {
+      "--config",
+      "scalardb.properties",
+      "--file",
+      "import.json",
+      "--namespace",
+      "scalar",
+      "--table",
+      "asset",
+      "--max-threads",
+      "8"
+    };
+    ImportCommand command = new ImportCommand();
+    CommandLine cmd = new CommandLine(command);
+    cmd.parseArgs(args);
+
+    // Verify the value was parsed
+    assertEquals(8, command.maxThreads);
+  }
+
+  @Test
+  void call_withoutMaxThreads_shouldDefaultToAvailableProcessors() {
+    // Simulate command line parsing without --max-threads
+    String[] args = {
+      "--config", "scalardb.properties",
+      "--file", "import.json",
+      "--namespace", "scalar",
+      "--table", "asset"
+    };
+    ImportCommand command = new ImportCommand();
+    CommandLine cmd = new CommandLine(command);
+    cmd.parseArgs(args);
+
+    // Verify maxThreads is null before validation
+    assertEquals(null, command.maxThreads);
+
+    // Simulate what happens in call() after validation
+    command.spec = cmd.getCommandSpec();
+    command.applyDeprecatedOptions();
+    if (command.maxThreads == null) {
+      command.maxThreads = Runtime.getRuntime().availableProcessors();
+    }
+
+    // Verify it was set to available processors
+    assertEquals(Runtime.getRuntime().availableProcessors(), command.maxThreads);
+  }
 }
diff --git a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataexport/ExportManager.java b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataexport/ExportManager.java
@@ -77,11 +77,8 @@ public ExportReport startExport(
       handleTransactionMetadata(exportOptions, tableMetadata);
       processHeader(exportOptions, tableMetadata, writer);
 
-      int maxThreadCount =
-          exportOptions.getMaxThreadCount() == 0
-              ? Runtime.getRuntime().availableProcessors()
-              : exportOptions.getMaxThreadCount();
-      ExecutorService executorService = Executors.newFixedThreadPool(maxThreadCount);
+      ExecutorService executorService =
+          Executors.newFixedThreadPool(exportOptions.getMaxThreadCount());
 
       BufferedWriter bufferedWriter = new BufferedWriter(writer);
       boolean isJson = exportOptions.getOutputFileFormat() == FileFormat.JSON;
diff --git a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataexport/ExportOptions.java b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataexport/ExportOptions.java
@@ -21,10 +21,10 @@ public class ExportOptions {
   private final FileFormat outputFileFormat;
   private final ScanRange scanRange;
   private final int limit;
-  private final int maxThreadCount;
   private final boolean prettyPrintJson;
 
   @Builder.Default private final int dataChunkSize = 200;
+  @Builder.Default private final int maxThreadCount = Runtime.getRuntime().availableProcessors();
   @Builder.Default private final String delimiter = ";";
   @Builder.Default private final boolean excludeHeaderRow = false;
   @Builder.Default private final boolean includeTransactionMetadata = false;
diff --git a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/ImportOptions.java b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/ImportOptions.java
@@ -34,7 +34,7 @@ public class ImportOptions {
   private final ControlFile controlFile;
   private final String namespace;
   private final String tableName;
-  private final int maxThreads;
+  @Builder.Default private final int maxThreads = Runtime.getRuntime().availableProcessors();
   private final String customHeaderRow;
   private final int dataChunkQueueSize;
 }
diff --git a/data-loader/core/src/test/java/com/scalar/db/dataloader/core/dataexport/JsonExportManagerTest.java b/data-loader/core/src/test/java/com/scalar/db/dataloader/core/dataexport/JsonExportManagerTest.java
@@ -130,4 +130,32 @@ void startExport_givenPartitionKey_shouldGenerateOutputFile()
     Assertions.assertTrue(file.exists());
     Assertions.assertTrue(file.delete());
   }
+
+  @Test
+  void exportOptions_withoutMaxThreadCount_shouldUseDefaultAvailableProcessors() {
+    // Create ExportOptions without explicitly setting maxThreadCount
+    ExportOptions exportOptions =
+        ExportOptions.builder("namespace", "table", null, FileFormat.JSON)
+            .sortOrders(Collections.emptyList())
+            .scanRange(new ScanRange(null, null, false, false))
+            .build();
+
+    // Verify the default was applied
+    Assertions.assertEquals(
+        Runtime.getRuntime().availableProcessors(), exportOptions.getMaxThreadCount());
+  }
+
+  @Test
+  void exportOptions_withExplicitMaxThreadCount_shouldUseProvidedValue() {
+    // Create ExportOptions with explicit maxThreadCount
+    ExportOptions exportOptions =
+        ExportOptions.builder("namespace", "table", null, FileFormat.JSON)
+            .sortOrders(Collections.emptyList())
+            .scanRange(new ScanRange(null, null, false, false))
+            .maxThreadCount(8)
+            .build();
+
+    // Verify the explicit value was used
+    Assertions.assertEquals(8, exportOptions.getMaxThreadCount());
+  }
 }
diff --git a/data-loader/core/src/test/java/com/scalar/db/dataloader/core/dataimport/ImportOptionsTest.java b/data-loader/core/src/test/java/com/scalar/db/dataloader/core/dataimport/ImportOptionsTest.java
@@ -0,0 +1,42 @@
+package com.scalar.db.dataloader.core.dataimport;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import org.junit.jupiter.api.Test;
+
+/** Unit tests for ImportOptions builder and default values. */
+public class ImportOptionsTest {
+
+  @Test
+  void importOptions_withoutMaxThreads_shouldUseDefaultAvailableProcessors() {
+    // Create ImportOptions without explicitly setting maxThreads
+    ImportOptions importOptions =
+        ImportOptions.builder()
+            .namespace("test_namespace")
+            .tableName("test_table")
+            .dataChunkSize(100)
+            .transactionBatchSize(10)
+            .dataChunkQueueSize(64)
+            .build();
+
+    // Verify the default was applied
+    assertEquals(Runtime.getRuntime().availableProcessors(), importOptions.getMaxThreads());
+  }
+
+  @Test
+  void importOptions_withExplicitMaxThreads_shouldUseProvidedValue() {
+    // Create ImportOptions with explicit maxThreads
+    ImportOptions importOptions =
+        ImportOptions.builder()
+            .namespace("test_namespace")
+            .tableName("test_table")
+            .dataChunkSize(100)
+            .transactionBatchSize(10)
+            .maxThreads(8)
+            .dataChunkQueueSize(64)
+            .build();
+
+    // Verify the explicit value was used
+    assertEquals(8, importOptions.getMaxThreads());
+  }
+}

Original file line number	Diff line number	Diff line change
`@@ -34,7 +34,7 @@ public class ImportOptions {`
`34`	`34`	`private final ControlFile controlFile;`
`35`	`35`	`private final String namespace;`
`36`	`36`	`private final String tableName;`
`37`		`- private final int maxThreads;`
	`37`	`+ @Builder.Default private final int maxThreads = Runtime.getRuntime().availableProcessors();`
`38`	`38`	`private final String customHeaderRow;`
`39`	`39`	`private final int dataChunkQueueSize;`
`40`	`40`	`}`