-
Notifications
You must be signed in to change notification settings - Fork 40
Deprecate --mode option in Import Command and auto-determine mode from transaction manager configuration #3177
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
c05d147
eed73a0
87f06f9
b34e95e
a00b608
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| package com.scalar.db.dataloader.cli; | ||
|
|
||
| /** | ||
| * @deprecated As of release 3.17.0 This enum is deprecated and will be removed in release 4.0.0. | ||
| * The behavior is now determined by the transaction manager configuration in the ScalarDB | ||
| * properties file. This enum is kept only for backward compatibility with the deprecated --mode | ||
| * CLI option. | ||
| */ | ||
| @Deprecated | ||
| public enum ScalarDbMode { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While it may seem strange to create a new class that is deprecated right away, this class is added to the cli code as the original class, and its enums have been renamed in the data loader core code for clarity. |
||
| /** Storage mode (single-crud operations). */ | ||
| STORAGE, | ||
|
|
||
| /** Transaction mode (consensus commit). */ | ||
| TRANSACTION | ||
| } | ||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| package com.scalar.db.dataloader.core; | ||
|
|
||
| /** | ||
| * The available transaction modes for data import operations. Determines how data is imported based | ||
| * on the transaction manager configuration. | ||
| */ | ||
| public enum TransactionMode { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This class replaces the ScalarDbMode enum class. |
||
|
|
||
| /** | ||
| * Single-crud mode: Each operation is committed immediately without separate transaction | ||
| * management. Used when the transaction manager is configured as single-crud operation mode. Each | ||
| * CRUD operation is executed and committed individually. | ||
| */ | ||
| SINGLE_CRUD, | ||
|
|
||
| /** | ||
| * Consensus commit mode: Operations are grouped into transaction batches with managed | ||
| * transactions. Used with standard transaction managers that provide ACID guarantees across | ||
| * multiple operations. Enables commit coordination across multiple operations. | ||
| */ | ||
| CONSENSUS_COMMIT | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,7 +2,7 @@ | |
|
|
||
| import com.scalar.db.api.DistributedTransactionManager; | ||
| import com.scalar.db.api.TableMetadata; | ||
| import com.scalar.db.dataloader.core.ScalarDbMode; | ||
| import com.scalar.db.dataloader.core.TransactionMode; | ||
| import com.scalar.db.dataloader.core.dataimport.dao.ScalarDbDao; | ||
| import com.scalar.db.dataloader.core.dataimport.datachunk.ImportDataChunkStatus; | ||
| import com.scalar.db.dataloader.core.dataimport.processor.ImportProcessor; | ||
|
|
@@ -12,6 +12,7 @@ | |
| import com.scalar.db.dataloader.core.dataimport.task.result.ImportTaskResult; | ||
| import com.scalar.db.dataloader.core.dataimport.transactionbatch.ImportTransactionBatchResult; | ||
| import com.scalar.db.dataloader.core.dataimport.transactionbatch.ImportTransactionBatchStatus; | ||
| import com.scalar.db.transaction.singlecrudoperation.SingleCrudOperationTransactionManager; | ||
| import java.io.BufferedReader; | ||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
@@ -33,29 +34,41 @@ | |
| * <li>Notifying listeners of various import events | ||
| * </ul> | ||
| */ | ||
| @SuppressWarnings("SameNameButDifferent") | ||
| @AllArgsConstructor | ||
| @SuppressWarnings("SameNameButDifferent") | ||
| public class ImportManager implements ImportEventListener { | ||
|
|
||
| @NonNull private final Map<String, TableMetadata> tableMetadata; | ||
| @NonNull private final BufferedReader importFileReader; | ||
| @NonNull private final ImportOptions importOptions; | ||
| private final ImportProcessorFactory importProcessorFactory; | ||
| private final List<ImportEventListener> listeners = new ArrayList<>(); | ||
| private final ScalarDbMode scalarDbMode; | ||
| private final DistributedTransactionManager distributedTransactionManager; | ||
| @NonNull private final DistributedTransactionManager distributedTransactionManager; | ||
|
|
||
| /** | ||
| * Starts the import process using the configured parameters. | ||
| * | ||
| * <p>If the data chunk size in {@link ImportOptions} is set to 0, the entire file will be | ||
| * processed as a single chunk. Otherwise, the file will be processed in chunks of the specified | ||
| * size. | ||
| * | ||
| * <p>The transaction mode is determined automatically based on the transaction manager | ||
| * configuration: if the transaction manager is configured as single-crud, SINGLE_CRUD mode is | ||
| * used; otherwise, CONSENSUS_COMMIT mode is used by default. | ||
| */ | ||
| public void startImport() { | ||
| // Determine the transaction mode based on transaction manager type. This mode will be | ||
| // refactored later | ||
| // and removed so that the whole code can just use one way to import all the data with the | ||
| // correct interface and not depend on making this distinction. | ||
| TransactionMode transactionMode = | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This check is mainly required because in the data loader for code, we have to make a difference between transactional and non-transactional. It might be worth checking to see if we can do a refactor in the future to avoid this kind of check, though. But for now, it's required; otherwise, too much refactoring needs to be done. |
||
| distributedTransactionManager instanceof SingleCrudOperationTransactionManager | ||
| ? TransactionMode.SINGLE_CRUD | ||
| : TransactionMode.CONSENSUS_COMMIT; | ||
|
|
||
| ImportProcessorParams params = | ||
| ImportProcessorParams.builder() | ||
| .scalarDbMode(scalarDbMode) | ||
| .transactionMode(transactionMode) | ||
| .importOptions(importOptions) | ||
| .tableMetadataByTableName(tableMetadata) | ||
| .dao(new ScalarDbDao()) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should not be merged until #3147 is completed. Have to update the target branch before that.