Skip to content

Commit 03a179d

Browse files
committed
Fix maxThreads option to be optional instead of required
# Conflicts: # data-loader/cli/src/test/java/com/scalar/db/dataloader/cli/command/dataimport/ImportCommandTest.java
1 parent d269d9d commit 03a179d

File tree

11 files changed

+191
-13
lines changed

11 files changed

+191
-13
lines changed

data-loader/cli/src/main/java/com/scalar/db/dataloader/cli/command/dataexport/ExportCommand.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,11 @@ public Integer call() throws Exception {
6161
FileUtils.validateFilePath(scalarDbPropertiesFilePath);
6262
validatePositiveValue(
6363
spec.commandLine(), dataChunkSize, DataLoaderError.INVALID_DATA_CHUNK_SIZE);
64-
validatePositiveValue(spec.commandLine(), maxThreads, DataLoaderError.INVALID_MAX_THREADS);
64+
if (maxThreads != null) {
65+
validatePositiveValue(spec.commandLine(), maxThreads, DataLoaderError.INVALID_MAX_THREADS);
66+
} else {
67+
maxThreads = Runtime.getRuntime().availableProcessors();
68+
}
6569

6670
StorageFactory storageFactory = StorageFactory.create(scalarDbPropertiesFilePath);
6771
TableMetadataService metaDataService =

data-loader/cli/src/main/java/com/scalar/db/dataloader/cli/command/dataexport/ExportCommandOptions.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ public class ExportCommandOptions {
7777
paramLabel = "<MAX_THREADS>",
7878
description =
7979
"Maximum number of threads to use for parallel processing (default: number of available processors)")
80-
protected int maxThreads;
80+
protected Integer maxThreads;
8181

8282
@CommandLine.Option(
8383
names = {"--start-key", "-sk"},

data-loader/cli/src/main/java/com/scalar/db/dataloader/cli/command/dataimport/ImportCommand.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,11 @@ public Integer call() throws Exception {
6161
spec.commandLine(), dataChunkSize, DataLoaderError.INVALID_DATA_CHUNK_SIZE);
6262
validatePositiveValue(
6363
spec.commandLine(), transactionSize, DataLoaderError.INVALID_TRANSACTION_SIZE);
64-
validatePositiveValue(spec.commandLine(), maxThreads, DataLoaderError.INVALID_MAX_THREADS);
64+
if (maxThreads != null) {
65+
validatePositiveValue(spec.commandLine(), maxThreads, DataLoaderError.INVALID_MAX_THREADS);
66+
} else {
67+
maxThreads = Runtime.getRuntime().availableProcessors();
68+
}
6569
validatePositiveValue(
6670
spec.commandLine(), dataChunkQueueSize, DataLoaderError.INVALID_DATA_CHUNK_QUEUE_SIZE);
6771
ControlFile controlFile = parseControlFileFromPath(controlFilePath).orElse(null);

data-loader/cli/src/main/java/com/scalar/db/dataloader/cli/command/dataimport/ImportCommandOptions.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,8 @@ public class ImportCommandOptions {
4242
names = {"--max-threads", "-mt"},
4343
paramLabel = "<MAX_THREADS>",
4444
description =
45-
"Maximum number of threads to use for parallel processing (default: number of available processors)",
46-
defaultValue = "16")
47-
protected int maxThreads;
45+
"Maximum number of threads to use for parallel processing (default: number of available processors)")
46+
protected Integer maxThreads;
4847

4948
// Deprecated option - kept for backward compatibility
5049
@CommandLine.Option(

data-loader/cli/src/test/java/com/scalar/db/dataloader/cli/command/dataexport/ExportCommandTest.java

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,4 +190,58 @@ void call_withOnlyDeprecatedEndExclusive_shouldApplyInvertedValue() {
190190
// end-exclusive=false should become end-inclusive=true
191191
assertEquals(true, command.scanEndInclusive);
192192
}
193+
194+
@Test
195+
void call_withMaxThreadsSpecified_shouldUseSpecifiedValue() {
196+
// Simulate command line parsing with --max-threads
197+
String[] args = {
198+
"--config",
199+
"scalardb.properties",
200+
"--namespace",
201+
"scalar",
202+
"--table",
203+
"asset",
204+
"--format",
205+
"JSON",
206+
"--max-threads",
207+
"8"
208+
};
209+
ExportCommand command = new ExportCommand();
210+
CommandLine cmd = new CommandLine(command);
211+
cmd.parseArgs(args);
212+
213+
// Verify the value was parsed
214+
assertEquals(8, command.maxThreads);
215+
}
216+
217+
@Test
218+
void call_withoutMaxThreads_shouldDefaultToAvailableProcessors() {
219+
// Simulate command line parsing without --max-threads
220+
String[] args = {
221+
"--config",
222+
"scalardb.properties",
223+
"--namespace",
224+
"scalar",
225+
"--table",
226+
"asset",
227+
"--format",
228+
"JSON"
229+
};
230+
ExportCommand command = new ExportCommand();
231+
CommandLine cmd = new CommandLine(command);
232+
cmd.parseArgs(args);
233+
234+
// Verify maxThreads is null before validation
235+
assertEquals(null, command.maxThreads);
236+
237+
// Simulate what happens in call() after validation
238+
command.spec = cmd.getCommandSpec();
239+
command.applyDeprecatedOptions();
240+
if (command.maxThreads == null) {
241+
command.maxThreads = Runtime.getRuntime().availableProcessors();
242+
}
243+
244+
// Verify it was set to available processors
245+
assertEquals(Runtime.getRuntime().availableProcessors(), command.maxThreads);
246+
}
193247
}

data-loader/cli/src/test/java/com/scalar/db/dataloader/cli/command/dataimport/ImportCommandTest.java

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,4 +229,54 @@ void call_withEnableLogSuccessShortForm_shouldSetToTrueWithoutValue() throws Exc
229229
// Verify the short form flag was parsed correctly without requiring a value
230230
assertTrue(command.enableLogSuccessRecords);
231231
}
232+
233+
@Test
234+
void call_withMaxThreadsSpecified_shouldUseSpecifiedValue() {
235+
// Simulate command line parsing with --max-threads
236+
String[] args = {
237+
"--config",
238+
"scalardb.properties",
239+
"--file",
240+
"import.json",
241+
"--namespace",
242+
"scalar",
243+
"--table",
244+
"asset",
245+
"--max-threads",
246+
"8"
247+
};
248+
ImportCommand command = new ImportCommand();
249+
CommandLine cmd = new CommandLine(command);
250+
cmd.parseArgs(args);
251+
252+
// Verify the value was parsed
253+
assertEquals(8, command.maxThreads);
254+
}
255+
256+
@Test
257+
void call_withoutMaxThreads_shouldDefaultToAvailableProcessors() {
258+
// Simulate command line parsing without --max-threads
259+
String[] args = {
260+
"--config", "scalardb.properties",
261+
"--file", "import.json",
262+
"--namespace", "scalar",
263+
"--table", "asset"
264+
};
265+
ImportCommand command = new ImportCommand();
266+
CommandLine cmd = new CommandLine(command);
267+
cmd.parseArgs(args);
268+
269+
// Verify maxThreads is null before validation
270+
assertEquals(null, command.maxThreads);
271+
272+
// Simulate what happens in call() after validation
273+
command.spec = cmd.getCommandSpec();
274+
command.applyDeprecatedOptions();
275+
if (command.maxThreads == null) {
276+
command.maxThreads = Runtime.getRuntime().availableProcessors();
277+
}
278+
279+
// Verify it was set to available processors
280+
assertEquals(Runtime.getRuntime().availableProcessors(), command.maxThreads);
281+
}
232282
}

data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataexport/ExportManager.java

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -77,11 +77,8 @@ public ExportReport startExport(
7777
handleTransactionMetadata(exportOptions, tableMetadata);
7878
processHeader(exportOptions, tableMetadata, writer);
7979

80-
int maxThreadCount =
81-
exportOptions.getMaxThreadCount() == 0
82-
? Runtime.getRuntime().availableProcessors()
83-
: exportOptions.getMaxThreadCount();
84-
ExecutorService executorService = Executors.newFixedThreadPool(maxThreadCount);
80+
ExecutorService executorService =
81+
Executors.newFixedThreadPool(exportOptions.getMaxThreadCount());
8582

8683
BufferedWriter bufferedWriter = new BufferedWriter(writer);
8784
boolean isJson = exportOptions.getOutputFileFormat() == FileFormat.JSON;

data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataexport/ExportOptions.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@ public class ExportOptions {
2121
private final FileFormat outputFileFormat;
2222
private final ScanRange scanRange;
2323
private final int limit;
24-
private final int maxThreadCount;
2524
private final boolean prettyPrintJson;
2625

2726
@Builder.Default private final int dataChunkSize = 200;
27+
@Builder.Default private final int maxThreadCount = Runtime.getRuntime().availableProcessors();
2828
@Builder.Default private final String delimiter = ";";
2929
@Builder.Default private final boolean excludeHeaderRow = false;
3030
@Builder.Default private final boolean includeTransactionMetadata = false;

data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/ImportOptions.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public class ImportOptions {
3434
private final ControlFile controlFile;
3535
private final String namespace;
3636
private final String tableName;
37-
private final int maxThreads;
37+
@Builder.Default private final int maxThreads = Runtime.getRuntime().availableProcessors();
3838
private final String customHeaderRow;
3939
private final int dataChunkQueueSize;
4040
}

data-loader/core/src/test/java/com/scalar/db/dataloader/core/dataexport/JsonExportManagerTest.java

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,4 +130,32 @@ void startExport_givenPartitionKey_shouldGenerateOutputFile()
130130
Assertions.assertTrue(file.exists());
131131
Assertions.assertTrue(file.delete());
132132
}
133+
134+
@Test
135+
void exportOptions_withoutMaxThreadCount_shouldUseDefaultAvailableProcessors() {
136+
// Create ExportOptions without explicitly setting maxThreadCount
137+
ExportOptions exportOptions =
138+
ExportOptions.builder("namespace", "table", null, FileFormat.JSON)
139+
.sortOrders(Collections.emptyList())
140+
.scanRange(new ScanRange(null, null, false, false))
141+
.build();
142+
143+
// Verify the default was applied
144+
Assertions.assertEquals(
145+
Runtime.getRuntime().availableProcessors(), exportOptions.getMaxThreadCount());
146+
}
147+
148+
@Test
149+
void exportOptions_withExplicitMaxThreadCount_shouldUseProvidedValue() {
150+
// Create ExportOptions with explicit maxThreadCount
151+
ExportOptions exportOptions =
152+
ExportOptions.builder("namespace", "table", null, FileFormat.JSON)
153+
.sortOrders(Collections.emptyList())
154+
.scanRange(new ScanRange(null, null, false, false))
155+
.maxThreadCount(8)
156+
.build();
157+
158+
// Verify the explicit value was used
159+
Assertions.assertEquals(8, exportOptions.getMaxThreadCount());
160+
}
133161
}

0 commit comments

Comments
 (0)