Skip to content

Commit e93d143

Browse files
authored
Merge branch 'master' into fix/data-loader/executor-shutdown-to-finally-block
2 parents 342a5cf + 0f200ff commit e93d143

File tree

5 files changed

+146
-19
lines changed

5 files changed

+146
-19
lines changed

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ public class ExportCommandOptions {
9494
protected Integer maxThreads;
9595

9696
/**
97-
* @deprecated As of release 3.6.2. Will be removed in release 4.0.0. Use --max-threads instead
97+
* @deprecated As of release 3.17.0. Will be removed in release 4.0.0. Use --max-threads instead
9898
*/
9999
@Deprecated
100100
@CommandLine.Option(
@@ -118,7 +118,10 @@ public class ExportCommandOptions {
118118
// TODO: test that -si false, works
119119
protected boolean scanStartInclusive;
120120

121-
// Deprecated option - kept for backward compatibility
121+
/**
122+
* @deprecated As of release 3.17.0. Will be removed in release 4.0.0. Use --start-inclusive
123+
* instead (inverted logic).
124+
*/
122125
@CommandLine.Option(
123126
names = {DEPRECATED_START_EXCLUSIVE_OPTION},
124127
description = "Deprecated: Use --start-inclusive instead (inverted logic)",
@@ -139,7 +142,10 @@ public class ExportCommandOptions {
139142
defaultValue = "true")
140143
protected boolean scanEndInclusive;
141144

142-
// Deprecated option - kept for backward compatibility
145+
/**
146+
* @deprecated As of release 3.17.0. Will be removed in release 4.0.0. Use --end-inclusive instead
147+
* (inverted logic).
148+
*/
143149
@CommandLine.Option(
144150
names = {DEPRECATED_END_EXCLUSIVE_OPTION},
145151
description = "Deprecated: Use --end-inclusive instead (inverted logic)",

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

Lines changed: 74 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,12 @@
44
import static com.scalar.db.dataloader.cli.util.CommandLineInputUtils.validatePositiveValue;
55

66
import com.fasterxml.jackson.databind.ObjectMapper;
7+
import com.scalar.db.api.DistributedTransaction;
78
import com.scalar.db.api.DistributedTransactionAdmin;
89
import com.scalar.db.api.TableMetadata;
910
import com.scalar.db.dataloader.core.DataLoaderError;
1011
import com.scalar.db.dataloader.core.FileFormat;
12+
import com.scalar.db.dataloader.core.ScalarDbMode;
1113
import com.scalar.db.dataloader.core.dataimport.ImportManager;
1214
import com.scalar.db.dataloader.core.dataimport.ImportOptions;
1315
import com.scalar.db.dataloader.core.dataimport.controlfile.ControlFile;
@@ -67,20 +69,32 @@ public Integer call() throws Exception {
6769
spec.commandLine(), dataChunkQueueSize, DataLoaderError.INVALID_DATA_CHUNK_QUEUE_SIZE);
6870
ControlFile controlFile = parseControlFileFromPath(controlFilePath).orElse(null);
6971
ImportOptions importOptions = createImportOptions(controlFile);
70-
ImportLoggerConfig config =
72+
ImportLoggerConfig importLoggerConfig =
7173
ImportLoggerConfig.builder()
7274
.logDirectoryPath(logDirectory)
7375
.isLogRawSourceRecordsEnabled(importOptions.isLogRawRecord())
7476
.isLogSuccessRecordsEnabled(importOptions.isLogSuccessRecords())
7577
.prettyPrint(prettyPrint)
7678
.build();
77-
LogWriterFactory logWriterFactory = createLogWriterFactory(config);
79+
LogWriterFactory logWriterFactory = createLogWriterFactory(importLoggerConfig);
80+
File configFile = new File(configFilePath);
81+
TransactionFactory transactionFactory = TransactionFactory.create(configFile);
82+
83+
// Validate transaction mode configuration before proceeding
84+
validateTransactionMode(transactionFactory);
85+
7886
Map<String, TableMetadata> tableMetadataMap =
79-
createTableMetadataMap(controlFile, namespace, tableName);
87+
createTableMetadataMap(controlFile, namespace, tableName, transactionFactory);
8088
try (BufferedReader reader =
8189
Files.newBufferedReader(Paths.get(sourceFilePath), Charset.defaultCharset())) {
8290
ImportManager importManager =
83-
createImportManager(importOptions, tableMetadataMap, reader, logWriterFactory, config);
91+
createImportManager(
92+
importOptions,
93+
tableMetadataMap,
94+
reader,
95+
importLoggerConfig,
96+
logWriterFactory,
97+
transactionFactory);
8498
importManager.startImport();
8599
}
86100
return 0;
@@ -101,14 +115,16 @@ private LogWriterFactory createLogWriterFactory(ImportLoggerConfig config) {
101115
* @param controlFile control file
102116
* @param namespace Namespace
103117
* @param tableName Single table name
118+
* @param transactionFactory transaction factory to use
104119
* @return {@code Map<String, TableMetadata>} a table metadata map
105120
* @throws ParameterException if one of the argument values is wrong
106121
*/
107122
private Map<String, TableMetadata> createTableMetadataMap(
108-
ControlFile controlFile, String namespace, String tableName)
109-
throws IOException, TableMetadataException {
110-
File configFile = new File(configFilePath);
111-
TransactionFactory transactionFactory = TransactionFactory.create(configFile);
123+
ControlFile controlFile,
124+
String namespace,
125+
String tableName,
126+
TransactionFactory transactionFactory)
127+
throws TableMetadataException {
112128
try (DistributedTransactionAdmin transactionAdmin = transactionFactory.getTransactionAdmin()) {
113129
TableMetadataService tableMetadataService = new TableMetadataService(transactionAdmin);
114130
Map<String, TableMetadata> tableMetadataMap = new HashMap<>();
@@ -133,18 +149,19 @@ private Map<String, TableMetadata> createTableMetadataMap(
133149
* @param importOptions import options
134150
* @param tableMetadataMap table metadata map
135151
* @param reader buffered reader with source data
152+
* @param importLoggerConfig import logging config
136153
* @param logWriterFactory log writer factory object
137-
* @param config import logging config
154+
* @param transactionFactory transaction factory to use
138155
* @return ImportManager object
139156
*/
140157
private ImportManager createImportManager(
141158
ImportOptions importOptions,
142159
Map<String, TableMetadata> tableMetadataMap,
143160
BufferedReader reader,
161+
ImportLoggerConfig importLoggerConfig,
144162
LogWriterFactory logWriterFactory,
145-
ImportLoggerConfig config)
163+
TransactionFactory transactionFactory)
146164
throws IOException {
147-
File configFile = new File(configFilePath);
148165
ImportProcessorFactory importProcessorFactory = new DefaultImportProcessorFactory();
149166
ImportManager importManager =
150167
new ImportManager(
@@ -153,11 +170,12 @@ private ImportManager createImportManager(
153170
importOptions,
154171
importProcessorFactory,
155172
scalarDbMode,
156-
TransactionFactory.create(configFile).getTransactionManager());
173+
transactionFactory.getTransactionManager());
157174
if (importOptions.getLogMode().equals(LogMode.SPLIT_BY_DATA_CHUNK)) {
158-
importManager.addListener(new SplitByDataChunkImportLogger(config, logWriterFactory));
175+
importManager.addListener(
176+
new SplitByDataChunkImportLogger(importLoggerConfig, logWriterFactory));
159177
} else {
160-
importManager.addListener(new SingleFileImportLogger(config, logWriterFactory));
178+
importManager.addListener(new SingleFileImportLogger(importLoggerConfig, logWriterFactory));
161179
}
162180
return importManager;
163181
}
@@ -236,6 +254,48 @@ private void validateLogDirectory(String logDirectory) throws ParameterException
236254
}
237255
}
238256

257+
/**
258+
* Validate transaction mode configuration by attempting to start and abort a transaction
259+
*
260+
* @param transactionFactory transaction factory to test
261+
* @throws ParameterException if transaction mode is incompatible with the configured transaction
262+
* manager
263+
*/
264+
void validateTransactionMode(TransactionFactory transactionFactory) {
265+
// Only validate when in TRANSACTION mode
266+
if (scalarDbMode != ScalarDbMode.TRANSACTION) {
267+
return;
268+
}
269+
270+
DistributedTransaction transaction = null;
271+
try {
272+
// Try to start a read only transaction to verify the transaction manager is properly
273+
// configured
274+
transaction = transactionFactory.getTransactionManager().startReadOnly();
275+
} catch (UnsupportedOperationException e) {
276+
// Transaction mode is not supported by the configured transaction manager
277+
throw new ParameterException(
278+
spec.commandLine(),
279+
DataLoaderError.INVALID_TRANSACTION_MODE.buildMessage(e.getMessage()),
280+
e);
281+
} catch (Exception e) {
282+
// Other exceptions - configuration or runtime error
283+
throw new ParameterException(
284+
spec.commandLine(),
285+
DataLoaderError.TRANSACTION_MODE_VALIDATION_FAILED.buildMessage(e.getMessage()),
286+
e);
287+
} finally {
288+
// Ensure transaction is aborted
289+
if (transaction != null) {
290+
try {
291+
transaction.abort();
292+
} catch (Exception ignored) {
293+
// Ignore errors during cleanup
294+
}
295+
}
296+
}
297+
}
298+
239299
/**
240300
* Generate control file from a valid control file path
241301
*

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public class ImportCommandOptions {
4646
protected Integer maxThreads;
4747

4848
/**
49-
* @deprecated As of release 3.6.2. Will be removed in release 4.0.0. Use --max-threads instead
49+
* @deprecated As of release 3.17.0. Will be removed in release 4.0.0. Use --max-threads instead
5050
*/
5151
@Deprecated
5252
@CommandLine.Option(
@@ -75,7 +75,7 @@ public class ImportCommandOptions {
7575
protected String controlFilePath;
7676

7777
/**
78-
* @deprecated As of release 3.6.2. Will be removed in release 4.0.0. Use --enable-log-success
78+
* @deprecated As of release 3.17.0. Will be removed in release 4.0.0. Use --enable-log-success
7979
* instead
8080
*/
8181
@Deprecated

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

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,14 @@
33
import static org.junit.jupiter.api.Assertions.assertEquals;
44
import static org.junit.jupiter.api.Assertions.assertThrows;
55
import static org.junit.jupiter.api.Assertions.assertTrue;
6+
import static org.mockito.Mockito.mock;
7+
import static org.mockito.Mockito.when;
68

9+
import com.scalar.db.api.DistributedTransactionManager;
710
import com.scalar.db.dataloader.core.FileFormat;
11+
import com.scalar.db.dataloader.core.ScalarDbMode;
812
import com.scalar.db.dataloader.core.dataimport.ImportMode;
13+
import com.scalar.db.service.TransactionFactory;
914
import java.io.IOException;
1015
import java.nio.file.Files;
1116
import java.nio.file.Path;
@@ -279,4 +284,52 @@ void call_withoutMaxThreads_shouldDefaultToAvailableProcessors() {
279284
// Verify it was set to available processors
280285
assertEquals(Runtime.getRuntime().availableProcessors(), command.maxThreads);
281286
}
287+
288+
@Test
289+
void validateTransactionMode_withUnsupportedOperation_shouldThrowException() throws Exception {
290+
// Arrange - Mock TransactionFactory to throw UnsupportedOperationException
291+
TransactionFactory mockFactory = mock(TransactionFactory.class);
292+
DistributedTransactionManager mockManager = mock(DistributedTransactionManager.class);
293+
294+
when(mockFactory.getTransactionManager()).thenReturn(mockManager);
295+
when(mockManager.startReadOnly())
296+
.thenThrow(new UnsupportedOperationException("Transaction mode is not supported"));
297+
298+
ImportCommand command = new ImportCommand();
299+
CommandLine cmd = new CommandLine(command);
300+
command.spec = cmd.getCommandSpec();
301+
command.scalarDbMode = ScalarDbMode.TRANSACTION;
302+
303+
// Act & Assert
304+
CommandLine.ParameterException thrown =
305+
assertThrows(
306+
CommandLine.ParameterException.class,
307+
() -> command.validateTransactionMode(mockFactory));
308+
309+
assertTrue(thrown.getMessage().contains("TRANSACTION mode is not compatible"));
310+
}
311+
312+
@Test
313+
void validateTransactionMode_withOtherException_shouldThrowException() throws Exception {
314+
// Arrange - Mock TransactionFactory to throw a different exception
315+
TransactionFactory mockFactory = mock(TransactionFactory.class);
316+
DistributedTransactionManager mockManager = mock(DistributedTransactionManager.class);
317+
318+
when(mockFactory.getTransactionManager()).thenReturn(mockManager);
319+
when(mockManager.startReadOnly()).thenThrow(new RuntimeException("Connection failed"));
320+
321+
ImportCommand command = new ImportCommand();
322+
CommandLine cmd = new CommandLine(command);
323+
command.spec = cmd.getCommandSpec();
324+
command.scalarDbMode = ScalarDbMode.TRANSACTION;
325+
326+
// Act & Assert
327+
CommandLine.ParameterException thrown =
328+
assertThrows(
329+
CommandLine.ParameterException.class,
330+
() -> command.validateTransactionMode(mockFactory));
331+
332+
assertTrue(thrown.getMessage().contains("Failed to validate TRANSACTION mode"));
333+
assertTrue(thrown.getMessage().contains("Connection failed"));
334+
}
282335
}

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,14 @@ public enum DataLoaderError implements ScalarDbError {
216216
"Cannot specify both deprecated option '%s' and new option '%s'. Please use only '%s'",
217217
"",
218218
""),
219+
INVALID_TRANSACTION_MODE(
220+
Category.USER_ERROR,
221+
"0058",
222+
"TRANSACTION mode is not compatible with the current configuration. Please try with STORAGE mode or check your ScalarDB configuration. Details: %s",
223+
"",
224+
""),
225+
TRANSACTION_MODE_VALIDATION_FAILED(
226+
Category.USER_ERROR, "0059", "Failed to validate TRANSACTION mode. Details: %s", "", ""),
219227

220228
//
221229
// Errors for the internal error category

0 commit comments

Comments
 (0)