Skip to content

feat: Add migration backfill for mptoken_issuance_history - #3128

Open
BryanJ1ang wants to merge 19 commits into
XRPLF:developfrom
BryanJ1ang:brjiang-mpt-rpc-migration
Open

feat: Add migration backfill for mptoken_issuance_history#3128
BryanJ1ang wants to merge 19 commits into
XRPLF:developfrom
BryanJ1ang:brjiang-mpt-rpc-migration

Conversation

@BryanJ1ang

@BryanJ1ang BryanJ1ang commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Note: this PR depends on previous ETL indexing PR here

Summary

Adds MPTTransactionHistoryMigrator, a Cassandra migration that backfills historical MPT issuance transaction history into the new MPT index tables:

  • mptoken_issuance_transactions
  • account_mptoken_issuance_transactions

The migrator full-scans the transactions table, reuses the live ETL MPT extractor, writes the same deterministic index rows produced by live indexing, and waits for queued async writes before marking the migration as complete.

Details

  • Registers MPTTransactionHistoryMigrator with the Cassandra migration manager.
  • Uses migration scan settings for full-table scanning:
    • migration.full_scan_threads
    • migration.full_scan_jobs
    • migration.cursors_per_job
  • Adds Cassandra driver paging support so token-range scans read all result pages.
  • Makes migration scans select explicit columns instead of relying on table column order.
  • Propagates full-table scanner worker failures so failed scans do not get marked migrated.
  • Keeps the migration non-blocking for Clio startup.

Testing

Added coverage for:

  • Migrator status before and after running.
  • Backfilling MPT issuance create paths and multi-issuance transaction fan-out.
  • Idempotent reruns.
  • Empty transaction tables.
  • Parity between migration backfill rows and live ETL rows.
  • Failed transactions with explicit MPT issuance references.
  • Cassandra paging across more than one driver page.
  • Full-table scanner validation and worker error propagation.

@BryanJ1ang
BryanJ1ang force-pushed the brjiang-mpt-rpc-migration branch 2 times, most recently from a0b2942 to 891846e Compare July 7, 2026 16:59
@codecov

codecov Bot commented Jul 7, 2026

Copy link
Copy Markdown

@BryanJ1ang
BryanJ1ang marked this pull request as ready for review July 8, 2026 16:16
@BryanJ1ang

Copy link
Copy Markdown
Contributor Author

/ai-reviewer

@Kassaking7 Kassaking7 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may also want to add missing unit tests based on the codecov report

Comment thread src/data/cassandra/impl/Statement.hpp Outdated
Comment thread src/migration/cassandra/impl/TransactionsAdapter.cpp Outdated
@BryanJ1ang
BryanJ1ang force-pushed the brjiang-mpt-rpc-migration branch from 2565b28 to 9e0d9fe Compare July 22, 2026 02:26
@BryanJ1ang

Copy link
Copy Markdown
Contributor Author

You may also want to add missing unit tests based on the codecov report

Added some unit tests but for CassandraMigrationBackend.hpp MPTTransactionHistoryMigrator.cpp and MigrationApplication.cpp these can't be unit tested since they need a Cassandra db. They are covered by integration tests.

@BryanJ1ang

Copy link
Copy Markdown
Contributor Author

@PeterChen13579 @yinyiqian1 I can't add you guys as reviewers but please take a look when you guys can!

@yinyiqian1
yinyiqian1 self-requested a review July 22, 2026 20:55
@PeterChen13579
PeterChen13579 self-requested a review July 23, 2026 16:20
@BryanJ1ang
BryanJ1ang requested a review from Kassaking7 July 24, 2026 03:21

@Kassaking7 Kassaking7 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment thread src/data/cassandra/impl/Collection.hpp

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a Cassandra migration to backfill historical MPToken issuance transaction history into the new MPT index tables, reusing the existing live ETL extractor and improving migration full-table scan robustness (paging + error propagation).

Changes:

  • Introduces MPTTransactionHistoryMigrator and registers it with the Cassandra migration manager.
  • Enhances full-table scanning: explicit column selection via kSelectColumns, Cassandra driver paging support, and propagation of worker/read failures.
  • Adds/extends unit + integration coverage around scanner behavior, adapter decoding failures, migrator status semantics, and parity with live ETL indexing.

Reviewed changes

Copilot reviewed 36 out of 36 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/unit/util/BatchingTests.cpp Adds unit coverage for util::BatchBuffer batching behavior and concurrency.
tests/unit/migration/MigratorRegisterTests.cpp Verifies failed migrators don’t write “Migrated” status.
tests/unit/migration/MigrationApplicationTests.cpp Adds unit tests for migrator CLI/application behavior via a mock manager.
tests/unit/migration/cassandra/TransactionsAdapterTests.cpp Tests transaction row deserialization and decode-failure signaling.
tests/unit/migration/cassandra/SpecTests.cpp Updates table spec test to include explicit select-column lists.
tests/unit/migration/cassandra/FullTableScannerTests.cpp Updates tests for new scanner wait/error behavior and adds new validations.
tests/unit/CMakeLists.txt Registers new unit test sources.
tests/integration/migration/cassandra/MPTTransactionHistoryMigratorTests.cpp End-to-end integration coverage for MPT backfill, idempotency, paging, and ETL parity.
tests/integration/migration/cassandra/ExampleTransactionsMigrator.cpp Updates example migrator to new adapter + scanner wait API.
tests/integration/migration/cassandra/ExampleObjectsMigrator.cpp Updates example migrator to new scanner wait API.
tests/integration/migration/cassandra/CassandraMigrationManagerTests.cpp Adds integration test to ensure migration scans read all driver pages.
tests/integration/data/cassandra/BaseTests.cpp Adds base Cassandra test for paging-state iteration across pages.
tests/integration/CMakeLists.txt Registers new integration test source.
tests/common/util/MockMigrationManager.hpp Adds a mock MigrationManagerInterface for application-layer tests.
tests/common/migration/TestMigrators.hpp Adds a throwing migrator used to validate failure semantics.
src/util/Batching.hpp Adds util::BatchBuffer for streaming batching + flush semantics.
src/migration/MigrationApplication.hpp Adds constructor overload allowing injection of a prebuilt migration manager (tests).
src/migration/MigrationApplication.cpp Avoids double Prometheus init and implements injected-manager constructor.
src/migration/CMakeLists.txt Links migration library against clio_etl to reuse extractor for backfill.
src/migration/cassandra/MPTTransactionHistoryMigrator.hpp Declares new backfill migrator for historical MPT issuance tx indexing.
src/migration/cassandra/MPTTransactionHistoryMigrator.cpp Implements full-table scan + batching + decode-failure handling + durable completion.
src/migration/cassandra/impl/TransactionsAdapter.hpp Adds explicit select columns + decode-failure callback + logging support.
src/migration/cassandra/impl/TransactionsAdapter.cpp Implements deserialization with controlled handling of decode failures.
src/migration/cassandra/impl/Spec.hpp Requires kSelectColumns and enforces tuple/column-count alignment.
src/migration/cassandra/impl/ObjectsAdapter.hpp Adds explicit kSelectColumns matching row tuple order.
src/migration/cassandra/impl/FullTableScanner.hpp Adds settings validation and replaces silent waits with error-propagating join.
src/migration/cassandra/impl/CassandraMigrationSchema.hpp Prepares full-scan statements using explicit column lists.
src/migration/cassandra/CassandraMigrationManager.hpp Registers MPTTransactionHistoryMigrator for Cassandra migrations.
src/migration/cassandra/CassandraMigrationBackend.hpp Adds prepared-statement caching, paging-size/state handling, and fail-closed scan errors.
src/main/Main.cpp Initializes Prometheus in the migrate CLI path before creating the migrator app.
src/etl/impl/ext/MPT.cpp Simplifies issuance-index row counting (removes per-tx warning threshold logic).
src/data/cassandra/impl/Statement.hpp Adds paging-size/state helpers and centralizes error throwing.
src/data/cassandra/impl/Result.hpp Adds hasMorePages() API.
src/data/cassandra/impl/Result.cpp Implements hasMorePages().
src/data/cassandra/impl/Collection.hpp Switches to shared Cassandra error helper (removes local throw helper).
src/data/cassandra/Error.hpp Adds shared throwErrorIfNeeded helper used by statement/binding/paging APIs.
Comments suppressed due to low confidence (1)

src/util/Batching.hpp:111

  • Same capacity issue as in add(): std::exchange(buffer_, {}) drops buffer_ capacity, so the next accumulation may reallocate. Reserving batchSize_ after clearing helps keep allocation behavior stable across flush cycles.
            std::scoped_lock const lock{mutex_};
            batch = std::exchange(buffer_, {});
        }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/util/Batching.hpp

@PeterChen13579 PeterChen13579 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just realized you have to merge with latest develop which updates to the newest libxrpl. Make sure to do a clean build. I will continue to review after this as merging with latest develop will change a lot

Comment thread src/data/cassandra/impl/Statement.hpp Outdated
Comment thread src/data/cassandra/impl/Statement.hpp Outdated
Comment thread src/data/cassandra/impl/Statement.hpp Outdated
Comment thread src/etl/impl/ext/MPT.cpp Outdated
Comment thread src/migration/cassandra/impl/FullTableScanner.hpp Outdated
Comment thread src/migration/cassandra/impl/FullTableScanner.hpp Outdated
Comment thread src/migration/cassandra/impl/FullTableScanner.hpp Outdated
Comment thread src/migration/cassandra/impl/FullTableScanner.hpp Outdated
Comment thread src/migration/cassandra/impl/FullTableScanner.hpp Outdated
@BryanJ1ang
BryanJ1ang force-pushed the brjiang-mpt-rpc-migration branch 2 times, most recently from 03dd0ca to 6914780 Compare July 28, 2026 02:12
@mathbunnyru
mathbunnyru requested a review from Copilot July 28, 2026 15:11

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 33 out of 33 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/migration/MigrationApplication.cpp:48

  • The injecting constructor accepts a potentially null migrationManager; run()/printStatus()/migrate() dereference migrationManager_ unconditionally, so constructing with nullptr would crash. Add a defensive check (throw/ASSERT) here to fail fast with a clear error.
MigratorApplication::MigratorApplication(
    MigrateSubCmd command,
    std::shared_ptr<migration::MigrationManagerInterface> migrationManager
)
    : migrationManager_(std::move(migrationManager)), cmd_(std::move(command))
{
}

BryanJ1ang and others added 18 commits July 29, 2026 11:07
Statement.hpp and Collection.hpp each carried a private static copy of
the same CassError-to-logic_error helper; both now use one shared free
function in data::cassandra::impl.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nst ref

A transaction that fails to deserialize is a deterministic data error, so
rerunning the migration could never get past it; log and skip the row
instead of permanently wedging the backfill. Callback and DB read errors
still fail closed. The scan callback now takes STTx/TxMeta by const ref,
removing a deep copy per scanned row.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Buffer extracted records across transactions and flush in batches
(mirroring the live ETL path's per-ledger accumulation) so the backend
coalesces them into full-size write batches instead of submitting one
tiny write pair per transaction.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The warning was not operator-actionable and per-tx fanout beyond the
threshold should not occur in practice; the Prometheus counter still
tracks index rows written.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Key the prepared-statement cache by table name alone (partition key and
select columns are fixed per TableDesc), replace the rowsRead counter
with a bool, and author the read-failure message once for both the log
and the thrown exception.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The TableSpec concept now requires the kSelectColumns list to have
exactly one column per Row tuple element, so the two cannot silently
drift when columns are added or removed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Restores Error.hpp, Collection.hpp and Statement.hpp::bindAt to their
develop state.

The shared throwErrorIfNeeded helper extracted into Error.hpp was an
unrelated dedup, and it left four other call sites (Result.hpp,
Tuple.hpp x2, Cluster.cpp) on the old inline pattern, so it did not
deliver the consistency it aimed for. That cleanup belongs in its own PR
that converts every site and settles on one message format.

Reverting bindAt also drops two incidental changes this branch had made
to it: an eagerly-evaluated fmt::format on every successful bind, and a
change to the error message shape from "[label] at idx N" to
"[label at idx N]".

Drop the private static throwErrorIfNeeded too, and inline its check
into the two paging methods that were its only callers. Keeping it would
have added a sixth copy of the same check-and-throw to this subsystem,
which is the duplication the reverted commit set out to remove. Removing
it also leaves bindAt's local lambda with no class-scope name to shadow.

Statement.hpp now differs from develop only by setPagingSize and
setPagingState, both of which the MPT backfill token-range scan
requires.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Restores the per-transaction MPT issuance index fanout warning in
MPTExt::writeMPTDataFromTransactions to match develop; the removal was
unrelated to the migration work on this branch.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@BryanJ1ang
BryanJ1ang force-pushed the brjiang-mpt-rpc-migration branch from 6914780 to b61c37c Compare August 5, 2026 15:35
@BryanJ1ang
BryanJ1ang requested a review from yinyiqian1 August 5, 2026 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants