Add transaction mode support for data export in dataloader core and CLI #2740

inv-jishnu · 2025-06-05T10:39:01Z

Description

This PR adds support for transactional mode in data export within the Dataloader core, along with corresponding updates to the CLI.

Previously, the export functionality only supported storage mode because there was no transactional scanner API that allowed streaming data without loading everything into memory. With the introduction of a suitable transactional scanner, we can now support export in transaction mode as well.

Related issues and/or PRs

NA

Changes made

On core

Added changes to support transaction for export in TableMetadataService, ExportManager, and ScalarDBDao
Added/updated unit tests.
On CLI
Added a parameter to input the Scalardb mode.
Added checks to use the transaction based on input for metadata retrieval and export.

Checklist

The following is a best-effort checklist. If any items in this checklist are not applicable to this PR or are dependent on other, unmerged PRs, please still mark the checkboxes after you have read and understood each item.

I have commented my code, particularly in hard-to-understand areas.
I have updated the documentation to reflect the changes.
I have considered whether similar issues could occur in other products, components, or modules if this PR is for bug fixes.
Any remaining open issues linked to this PR are documented and up-to-date (Jira, GitHub, etc.).
Tests (unit, integration, etc.) have been added for the changes.
My changes generate no new warnings.
Any dependent changes in other PRs have been merged and published.

Additional notes (optional)

NA

Release notes

Added transaction mode in data export on data loader core and CLI

komamitsu · 2025-06-11T07:42:36Z

@inv-jishnu I think there are many if conditions to check whether it's storage or transaction. This is just an idea, but ScalarDbDao might be a good wrapper of the storage and transaction. How about making ScalarDbDao an interface (or abstract class) and adding concrete classes something like ScalarDbStorageDao and ScalarDbTransactionDao which contain a storage or a transaction, respectively, instead of accepting either of storage or transaction at method level (e.g., get()) ? I'm not sure it's a reasonable suggestion in terms of release schedule and so on, though.

komamitsu · 2025-06-11T07:13:49Z

...der/core/src/main/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataService.java

@@ -18,7 +19,17 @@
 public class TableMetadataService {

  private final DistributedStorageAdmin storageAdmin;
+  private final DistributedTransactionAdmin transactionAdmin;


This class seems the union of TableMetadataService for storage and for transaction with some internal if branches. How about making TableMetadataService abstract and creating 2 child classes that return the TableMetadata for storage or transaction from something like getTableMetadataInternal()?

@komamitsu san,
I have split up table metadata service as suggested.
Thank you.

inv-jishnu · 2025-06-11T10:37:43Z

@inv-jishnu I think there are many if conditions to check whether it's storage or transaction. This is just an idea, but ScalarDbDao might be a good wrapper of the storage and transaction. How about making ScalarDbDao an interface (or abstract class) and adding concrete classes something like ScalarDbStorageDao and ScalarDbTransactionDao which contain a storage or a transaction, respectively, instead of accepting either of storage or transaction at method level (e.g., get()) ? I'm not sure it's a reasonable suggestion in terms of release schedule and so on, though.

@komamitsu san,
Thank you for the suggestion.
I will work on this in another PR (as you mentioned about the release schedule). I will add the necessary changes in that one.

brfrn169 · 2025-06-12T02:27:25Z

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

@@ -144,4 +145,11 @@ public class ExportCommandOptions {
      description = "Size of the data chunk to process in a single task (default: 200)",
      defaultValue = "200")
  protected int dataChunkSize;
+
+  @CommandLine.Option(
+      names = {"--mode", "-sm"},


@brfrn169 san,
-m is already used by the --include-matadata input as this was already present

@CommandLine.Option( names = {"--include-metadata", "-m"}, description = "Include transaction metadata in the exported data (default: false)", defaultValue = "false") protected boolean includeTransactionMetadata;

I didn't want to change the existing value as it has been there from the always and there may be users who use it.
Then also the --mode in import is -m(which also was added in latest change) Should I change the mode to-mand include metadata to something else like -im` ?

@brfrn169 san,
After discussing with Yves san, I have set -m for mode and -im to inlcude-metadata to make it consistent with import.
Thank you.

brfrn169 · 2025-06-12T02:37:43Z

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

+    Scan scan =
+        createScan(namespace, table, null, null, new ArrayList<>(), projectionColumns, limit);
+    try {
+      return transaction.getScanner(scan);


Please use DistributedTransactionManager.getScanner(). You don’t need to begin or commit a transaction when using it.

@brfrn169 san,
I had initially used DistributedTransactionManager directly but then was a bit confused whether I can use it like that (in import transaction was used in the Dao). So I used transaction. I have updated this as suggested.
Thank you.

brfrn169 · 2025-06-12T02:48:57Z

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

+            exportOptions.getMaxThreadCount() > 0
+                ? exportOptions.getMaxThreadCount()
+                : Runtime.getRuntime().availableProcessors();
+        executorService = Executors.newFixedThreadPool(threadCount);


My understanding is as follows:

The main thread (a single thread) scans the table.

Multiple threads process the scan results in batches and write them through a single writer (a single file).

Is that correct?

Yes, that is correct.

ypeckstadt · 2025-06-12T04:43:58Z

@brfrn169 Hiro would like is to focus on the migration of existing features only for the 3.16 release. So no need to handle/merge this PR yet this week.

Is it best that I add this to the 3.16.1 project when available instead? (and remove it from 3.16 for now)?

inv-jishnu · 2025-06-12T07:54:46Z

@brfrn169 san
I have made changes as suggested.
Please take a look at this again when you get a chance.
Thank you.

brfrn169 · 2025-06-13T02:09:33Z

Is it best that I add this to the 3.16.1 project when available instead? (and remove it from 3.16 for now)?

@ypeckstadt Basically, we don’t include new features in patch versions.

As discussed with @feeblefakie, we’ve decided not to release 3.16 this week but to release it next week instead. So I think we can include this PR in that release. Thanks.

thongdk8 · 2025-06-18T07:08:22Z

FYI, as discussed in another place Slack conv. With the default config of transaction manager consensus-commit when exporting data in transaction mode, the OMM will occur if the export data exceeds the available memory. So we are expecting to run exporting in transaction mode with scalar.db.transaction_manager=single-crud-operation, with single-crud-operation transaction manager the memory usage will be safe as exporting in storage mode.

inv-jishnu · 2025-06-18T11:06:25Z

@brfrn169 san, @komamitsu san,
I have made updates based on feedback.
Please take a look at this PR when you get a chance.
Thank you

komamitsu · 2025-06-23T01:39:17Z

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

+      DistributedStorage storage = StorageFactory.create(scalarDbPropertiesFilePath).getStorage();
+      return createExportManagerWithStorage(storage, scalarDbDao, fileFormat, taskFactory);
+    } else {
+      DistributedTransactionManager distributedTransactionManager =


Don't need to consider #2740 (comment) ?

inv-jishnu · 2025-06-23T09:39:09Z

@komamitsu san, @brfrn169 san (cc @ypeckstadt )

Based on comment from @thongdk8 , for export operations in transaction mode, is it acceptable to explicitly set scalar.db.transaction_manager to single-crud-operation, even if the user hasn’t specified this value in the database configuration file?

komamitsu · 2025-06-23T11:01:06Z

@inv-jishnu I think it's a reasonable and safe option. But it depends on the requirement (e.g., users can use consensus-commit to export snapshot data even it has the risk of OOM). Probably, you can talk with @ypeckstadt to clarify the requirement.

brfrn169 · 2025-06-25T04:55:47Z

Based on comment from @thongdk8 , for export operations in transaction mode, is it acceptable to explicitly set scalar.db.transaction_manager to single-crud-operation, even if the user hasn’t specified this value in the database configuration file?

@inv-jishnu Let me discuss it with @feeblefakie.

brfrn169 and others added 12 commits May 28, 2025 16:23

Add getScanner() method to transaction interfaces (#2698)

da1cb57

Merge branch 'master' into add-scanner-api-to-transaction-abstraction

29ca31b

Implement scanner API for single CRUD transactions (#2701)

8ea08fa

Implement scanner API for JDBC transactions (#2702)

5042c49

Merge branch 'master' into add-scanner-api-to-transaction-abstraction

e4db961

Implement scanner API for Consensus Commit (#2711)

7b91326

Add more integration tests for scanner API (#2724)

9ad1219

Merge branch 'master' into add-scanner-api-to-transaction-abstraction

7ab8b65

Merge branch 'master' into feat/data-loader/export-trn

b3414fb

Initial core and cli changes

e32fd44

minor test change

24ce20c

Added unit test for Dao

68059f8

inv-jishnu self-assigned this Jun 5, 2025

inv-jishnu added the enhancement New feature or request label Jun 5, 2025

inv-jishnu marked this pull request as draft June 5, 2025 10:39

inv-jishnu and others added 4 commits June 5, 2025 16:09

Merge branch 'master' into feat/data-loader/export-trn

eb62e60

Changes

f4b0571

Updated javadoc

735c47e

Spotless applied on CLI

43ef7bc

ypeckstadt changed the title ~~Add transaction mode in export on dataloader core and CLI~~ Add transaction mode support for data export in dataloader core and CLI Jun 9, 2025

Resolved conflicts and merged changes from master

7c15643

ypeckstadt marked this pull request as ready for review June 11, 2025 00:26

Merge branch 'master' into feat/data-loader/export-trn

a53757b

ypeckstadt requested review from a team, komamitsu, brfrn169, feeblefakie, Torch3333 and ypeckstadt and removed request for a team June 11, 2025 00:26

komamitsu reviewed Jun 11, 2025

View reviewed changes

inv-jishnu and others added 2 commits June 11, 2025 15:49

Table metadata service split to storage and transaction

303b288

Merge branch 'master' into feat/data-loader/export-trn

37fa8af

inv-jishnu requested a review from komamitsu June 11, 2025 10:35

brfrn169 reviewed Jun 12, 2025

View reviewed changes

inv-jishnu and others added 2 commits June 12, 2025 10:38

Merge branch 'master' into feat/data-loader/export-trn

8c189e8

Changed implemenatation to use DistributedTransactionManager

36f9b43

inv-jishnu requested a review from brfrn169 June 12, 2025 07:54

inv-jishnu and others added 4 commits June 13, 2025 11:48

Renamed CLI input short names for mode and inlude metadata

374e833

Merge branch 'master' into feat/data-loader/export-trn

39a4abd

Javadocs added

761deca

Merged changes and resolved conflicts with master branch

9fe9c83

thongdk8 self-requested a review June 18, 2025 07:09

inv-jishnu added 2 commits June 18, 2025 14:24

Merge branch 'master' into feat/data-loader/export-trn

d45f4cd

Merge branch 'master' into feat/data-loader/export-trn

ed66ab1

inv-jishnu added 2 commits June 18, 2025 18:18

Merge branch 'master' into feat/data-loader/export-trn

2138113

Merge branch 'master' into feat/data-loader/export-trn

b9cbd6b

komamitsu reviewed Jun 23, 2025

View reviewed changes

Merged changes from master after resolving conflicts

c2192da

Merge branch 'master' into feat/data-loader/export-trn

42bc80f

Add transaction mode support for data export in dataloader core and CLI #2740

Are you sure you want to change the base?

Add transaction mode support for data export in dataloader core and CLI #2740

Uh oh!

Conversation

inv-jishnu commented Jun 5, 2025 • edited by ypeckstadt Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related issues and/or PRs

Changes made

Checklist

Additional notes (optional)

Release notes

Uh oh!

komamitsu commented Jun 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

inv-jishnu commented Jun 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ypeckstadt commented Jun 12, 2025

Uh oh!

inv-jishnu commented Jun 12, 2025

Uh oh!

brfrn169 commented Jun 13, 2025

Uh oh!

thongdk8 commented Jun 18, 2025

Uh oh!

inv-jishnu commented Jun 18, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

inv-jishnu commented Jun 23, 2025

Uh oh!

komamitsu commented Jun 23, 2025

Uh oh!

brfrn169 commented Jun 25, 2025

Uh oh!

Uh oh!

inv-jishnu commented Jun 5, 2025 •

edited by ypeckstadt

Loading