Rerun conditional migrations #6565

mgoelswirlds · 2023-08-01T20:42:41Z

Description:
This PR is created to address re-running certain time sensitive migrations, for newer mirror nodes.

This PR modifies:

SyntheticCryptoTransferApprovalMigration to rerun when the HAPI version changes to 0.38.10 or above.
SyntheticNftAllowanceOwnerMigration and SyntheticTokenAllowanceOwnerMigration to rerun when the HAPI version changes to 0.37.0 or above.
InitializeEntityBalanceMigration and TokenAccountBalanceMigration to rerun, when the first account balance file comes in.

Related issue(s):

Fixes #6397

Notes for reviewer:

Checklist

Documented (Code comments, README, etc.)
Tested (unit, integration, etc.)

codecov · 2023-08-01T21:06:55Z

Codecov Report

Patch coverage: 81.72% and project coverage change: +7.29% 🎉

Comparison is base (edd4ce5) 85.24% compared to head (e7ddc3a) 92.53%.
Report is 6 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #6565      +/-   ##
============================================
+ Coverage     85.24%   92.53%   +7.29%     
- Complexity      453     6555    +6102     
============================================
  Files            95      848     +753     
  Lines          1945    27566   +25621     
  Branches        124     3157    +3033     
============================================
+ Hits           1658    25508   +23850     
- Misses          236     1342    +1106     
- Partials         51      716     +665

Files Changed	Coverage Δ
...orter/parser/balance/AccountBalanceFileParser.java	`100.00% <ø> (ø)`
...er/balance/CompositeBalanceStreamFileListener.java	`41.66% <41.66%> (ø)`
...era/mirror/importer/parser/StreamFileListener.java	`66.66% <66.66%> (ø)`
...tion/SyntheticCryptoTransferApprovalMigration.java	`78.16% <81.25%> (ø)`
...orter/migration/TimeSensitiveBalanceMigration.java	`85.71% <85.71%> (ø)`
...migration/SyntheticNftAllowanceOwnerMigration.java	`95.45% <92.30%> (ø)`
...gration/SyntheticTokenAllowanceOwnerMigration.java	`95.45% <92.30%> (ø)`
...era/mirror/importer/migration/ErrataMigration.java	`93.79% <100.00%> (ø)`
...er/migration/InitializeEntityBalanceMigration.java	`100.00% <100.00%> (ø)`
...porter/migration/TokenAccountBalanceMigration.java	`100.00% <100.00%> (ø)`
... and 1 more

... and 767 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

mgoelswirlds · 2023-08-02T17:51:49Z

...irror-importer/src/main/java/com/hedera/mirror/importer/parser/AbstractStreamFileParser.java

@@ -44,6 +47,9 @@ protected AbstractStreamFileParser(
            MeterRegistry meterRegistry,
            ParserProperties parserProperties,
            StreamFileRepository<T, Long> streamFileRepository) {
+
+        this.last = new AtomicReference<>();
+        this.lastFromDb = new AtomicReference<>();


This value is leveraging the last read from the repository.

mgoelswirlds · 2023-08-02T17:53:12Z

...porter/src/main/java/com/hedera/mirror/importer/parser/balance/AccountBalanceFileParser.java

+        var lastBalanceFile = last.get();
+        // If no account balance file has been processed since last startup.
+        if (lastBalanceFile == null && lastFromDb.get() == null) {
+            applicationEventPublisher.publishEvent(new InitializeBalanceEvent(this));


If this is the first account balance file being processed and the DB is empty then publishing InitializeBalanceEvent which will rerun balance migrations.

mgoelswirlds · 2023-08-02T17:55:35Z

...porter/src/main/java/com/hedera/mirror/importer/parser/balance/AccountBalanceFileParser.java

+    }
+
+    @Nullable
+    private AccountBalanceFile publishInitializeAccountBalanceEvent() {


Moved this into another function to reduce cognitive complexity.

mgoelswirlds · 2023-08-03T14:53:42Z

...main/java/com/hedera/mirror/importer/migration/SyntheticCryptoTransferApprovalMigration.java

+            return;
+        }
+
+        /*if(shouldRerun(streamFile,HAPI_VERSION_0_38_10,recordFileRepository)) {


These comments will be removed once we Merge Xin's changes to have async migration extend repeatable migration.

Signed-off-by: mgoelswirlds <mugdha.goel@swirldslabs.com>

mgoelswirlds · 2023-08-04T00:26:22Z

.../src/main/java/com/hedera/mirror/importer/migration/SyntheticNftAllowanceOwnerMigration.java

@@ -107,9 +115,13 @@ insert into nft_allowance (approved_for_all, owner, payer_account_id, spender, t
    private final JdbcTemplate jdbcTemplate;

    @Lazy
-    public SyntheticNftAllowanceOwnerMigration(@Owner JdbcTemplate jdbcTemplate, MirrorProperties mirrorProperties) {


This migration took 779 ms on mainnet.So not converting it to Async.

mgoelswirlds · 2023-08-04T00:26:42Z

...rc/main/java/com/hedera/mirror/importer/migration/SyntheticTokenAllowanceOwnerMigration.java

 import org.springframework.jdbc.core.JdbcTemplate;

 @Named
-public class SyntheticTokenAllowanceOwnerMigration extends RepeatableMigration {
+public class SyntheticTokenAllowanceOwnerMigration extends RepeatableMigration implements RecordStreamFileListener {


This migration took 10158 ms on mainnet.So not converting it to Async.

I doubt it can work in environment where the owner and regular user are different, e.g., mirror_node and mirror_importer.

The migration uses the owner jdbcTemplate bean because it deletes rows. in some environment, mirror_importer may not be able to delete rows.

When the migration runs inside onEnd, it's in the same database transaction, and the connection in the thread local storage uses regular user mirror_importer. The migration most likely uses the same connection, and may fail to delete rows.

did you check if running it inside onEnd would actually pick a connection with different credentials other than owner?

this is a blocker for both SyntheticNftAllowanceOwnerMigration and SyntheticTokenAllowanceOwnerMigration

No I did not. Checking now.

No I did not. Will need to check this. looking now.
Even if it is a different user i.e. mirror_importer, I need to verify that it can delete the rows right?

we can discuss. running with mirror_importer may not break ourselves but can break other mirrornode operators in case in their environment mirror_importer doesn't have permission to delete rows.

then we may have to turn these two migrations to be async.

Ok. Lets discuss further offline.

I verified that even when migrate is called from with the onEnd it uses the mirror_node user.

...main/java/com/hedera/mirror/importer/migration/SyntheticCryptoTransferApprovalMigration.java

.../src/main/java/com/hedera/mirror/importer/migration/SyntheticNftAllowanceOwnerMigration.java

...rc/main/java/com/hedera/mirror/importer/parser/record/MigrationRecordStreamFileListener.java

...ter/src/main/java/com/hedera/mirror/importer/migration/InitializeEntityBalanceMigration.java

...porter/src/main/java/com/hedera/mirror/importer/parser/balance/AccountBalanceFileParser.java

…r/migration/InitializeEntityBalanceMigration.java Co-authored-by: Xin Li <59580070+xin-hedera@users.noreply.github.com> Signed-off-by: Mugdha Goel <106084778+mgoelswirlds@users.noreply.github.com>

…r/migration/SyntheticCryptoTransferApprovalMigration.java Co-authored-by: Xin Li <59580070+xin-hedera@users.noreply.github.com> Signed-off-by: Mugdha Goel <106084778+mgoelswirlds@users.noreply.github.com>

…r/migration/InitializeEntityBalanceMigration.java Co-authored-by: Xin Li <59580070+xin-hedera@users.noreply.github.com> Signed-off-by: Mugdha Goel <106084778+mgoelswirlds@users.noreply.github.com>

Signed-off-by: mgoelswirlds <mugdha.goel@swirldslabs.com>

… for tokenAccountBalance or InitializeEntityBalance Signed-off-by: mgoelswirlds <mugdha.goel@swirldslabs.com>

Signed-off-by: Mugdha Goel <106084778+mgoelswirlds@users.noreply.github.com>

Adding missing test cases.

docs/database/README.md

...porter/src/main/java/com/hedera/mirror/importer/migration/TimeSensitiveBalanceMigration.java

...ter/src/test/java/com/hedera/mirror/importer/migration/TokenAccountBalanceMigrationTest.java

...java/com/hedera/mirror/importer/migration/SyntheticCryptoTransferApprovalsMigrationTest.java

...est/java/com/hedera/mirror/importer/migration/SyntheticTokenAllowanceOwnerMigrationTest.java

.../test/java/com/hedera/mirror/importer/migration/SyntheticNftAllowanceOwnerMigrationTest.java

…igrations

Signed-off-by: mgoelswirlds <mugdha.goel@swirldslabs.com>

…igrations

...src/test/java/com/hedera/mirror/importer/migration/InitializeEntityBalanceMigrationTest.java

Signed-off-by: mgoelswirlds <mugdha.goel@swirldslabs.com>

...src/test/java/com/hedera/mirror/importer/migration/InitializeEntityBalanceMigrationTest.java

...java/com/hedera/mirror/importer/migration/SyntheticCryptoTransferApprovalsMigrationTest.java

...porter/src/main/java/com/hedera/mirror/importer/migration/TimeSensitiveBalanceMigration.java

...java/com/hedera/mirror/importer/migration/SyntheticCryptoTransferApprovalsMigrationTest.java

.../test/java/com/hedera/mirror/importer/migration/SyntheticNftAllowanceOwnerMigrationTest.java

…yBalanceMigrationTest and TokenAccountBalanceMigrationTest Arranging fields. Signed-off-by: mgoelswirlds <mugdha.goel@swirldslabs.com>

Signed-off-by: mgoelswirlds <mugdha.goel@swirldslabs.com>

xin-hedera

LGTM

...src/test/java/com/hedera/mirror/importer/migration/InitializeEntityBalanceMigrationTest.java

Signed-off-by: mgoelswirlds <mugdha.goel@swirldslabs.com>

steven-sheehy

LGTM

sonarcloud · 2023-08-18T17:00:42Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.3% Duplication

xin-hedera

LGTM

Description: This PR is created to address re-running certain time sensitive migrations, for newer and partial mirror nodes. This PR modifies: - SyntheticCryptoTransferApprovalMigration to rerun when the HAPI version changes to 0.38.10 or above. - SyntheticNftAllowanceOwnerMigration and SyntheticTokenAllowanceOwnerMigration to rerun when the HAPI version changes to 0.37.0 or above. - InitializeEntityBalanceMigration and TokenAccountBalanceMigration to rerun, after the first account balance file has been parsed. Related issue(s): Fixes #6397

mgoelswirlds added 2 commits August 1, 2023 16:29

Initial commit to rerun said migrations.

102dff1

Changes to fix sonarcloud issues.

ae2ad08

mgoelswirlds added 2 commits August 1, 2023 19:34

Changes to fix sonarcloud issues.

901eb32

Adding common code to repeatable migration.

bc8af67

mgoelswirlds commented Aug 2, 2023

View reviewed changes

Adding test cases and renaming variables.

ae0ac55

mgoelswirlds commented Aug 3, 2023

View reviewed changes

mgoelswirlds added 3 commits August 3, 2023 17:49

Adding code back to respective migration tests.

0388007

Removing unneeded variables.

488f89c

Removing extra file.

30aa94a

Signed-off-by: mgoelswirlds <mugdha.goel@swirldslabs.com>

mgoelswirlds commented Aug 4, 2023

View reviewed changes

mgoelswirlds marked this pull request as ready for review August 4, 2023 00:27

steven-sheehy changed the title ~~Initial commit to rerun said migrations.~~ Rerun conditional migrations Aug 4, 2023

steven-sheehy requested a review from a team August 4, 2023 17:08

Set items to null.

9580a62

steven-sheehy assigned mgoelswirlds Aug 4, 2023

steven-sheehy added enhancement Type: New feature parser Area: File parsing labels Aug 4, 2023

steven-sheehy added this to the 0.87.0 milestone Aug 4, 2023

xin-hedera requested changes Aug 4, 2023

View reviewed changes

mgoelswirlds and others added 6 commits August 4, 2023 14:14

Update hedera-mirror-importer/src/main/java/com/hedera/mirror/importe…

ea2991a

…r/migration/InitializeEntityBalanceMigration.java Co-authored-by: Xin Li <59580070+xin-hedera@users.noreply.github.com> Signed-off-by: Mugdha Goel <106084778+mgoelswirlds@users.noreply.github.com>

Update hedera-mirror-importer/src/main/java/com/hedera/mirror/importe…

7f575b9

…r/migration/SyntheticCryptoTransferApprovalMigration.java Co-authored-by: Xin Li <59580070+xin-hedera@users.noreply.github.com> Signed-off-by: Mugdha Goel <106084778+mgoelswirlds@users.noreply.github.com>

Update hedera-mirror-importer/src/main/java/com/hedera/mirror/importe…

2c6e69e

…r/migration/InitializeEntityBalanceMigration.java Co-authored-by: Xin Li <59580070+xin-hedera@users.noreply.github.com> Signed-off-by: Mugdha Goel <106084778+mgoelswirlds@users.noreply.github.com>

PR comment changes.TokenAccountBalanceMigration

8888db3

Signed-off-by: mgoelswirlds <mugdha.goel@swirldslabs.com>

Undoing the last changes and adding transaction based event listeners…

305bc8b

… for tokenAccountBalance or InitializeEntityBalance Signed-off-by: mgoelswirlds <mugdha.goel@swirldslabs.com>

Merge branch 'main' into 6397-Time-sensitive-migrations

0653952

Signed-off-by: Mugdha Goel <106084778+mgoelswirlds@users.noreply.github.com>

Providing order priority to Migrations.

1ac1042

Adding missing test cases.

steven-sheehy requested changes Aug 16, 2023

View reviewed changes

mgoelswirlds added 5 commits August 16, 2023 11:39

Merge remote-tracking branch 'origin/main' into 6397-Time-sensitive-m…

97a1395

…igrations

Merge remote-tracking branch 'origin/main' into 6397-Time-sensitive-m…

e127396

…igrations

Adding coverage.

45702ab

Signed-off-by: mgoelswirlds <mugdha.goel@swirldslabs.com>

Refactoring tests to add common setup methods.

c4acb56

Signed-off-by: mgoelswirlds <mugdha.goel@swirldslabs.com>

Merge remote-tracking branch 'origin/main' into 6397-Time-sensitive-m…

180015f

…igrations

xin-hedera reviewed Aug 17, 2023

View reviewed changes

...src/test/java/com/hedera/mirror/importer/migration/InitializeEntityBalanceMigrationTest.java Show resolved Hide resolved

mgoelswirlds added 3 commits August 17, 2023 13:40

Fixing Initialize balance test coverage.

c393971

Signed-off-by: mgoelswirlds <mugdha.goel@swirldslabs.com>

Fixing Initialize balance test coverage.

604bd3e

Signed-off-by: mgoelswirlds <mugdha.goel@swirldslabs.com>

Adding more coverage.

4e752d9

Signed-off-by: mgoelswirlds <mugdha.goel@swirldslabs.com>

mgoelswirlds requested review from steven-sheehy and xin-hedera August 17, 2023 23:14

xin-hedera reviewed Aug 18, 2023

View reviewed changes

...src/test/java/com/hedera/mirror/importer/migration/InitializeEntityBalanceMigrationTest.java Outdated Show resolved Hide resolved

steven-sheehy requested changes Aug 18, 2023

View reviewed changes

mgoelswirlds added 2 commits August 18, 2023 11:40

Creating new migration objects in each onEnd test for InitializeEntit…

00815e1

…yBalanceMigrationTest and TokenAccountBalanceMigrationTest Arranging fields. Signed-off-by: mgoelswirlds <mugdha.goel@swirldslabs.com>

Deleting leftover method.

6e8ee7a

Signed-off-by: mgoelswirlds <mugdha.goel@swirldslabs.com>

xin-hedera previously approved these changes Aug 18, 2023

View reviewed changes

mgoelswirlds requested a review from steven-sheehy August 18, 2023 16:12

steven-sheehy reviewed Aug 18, 2023

View reviewed changes

...src/test/java/com/hedera/mirror/importer/migration/InitializeEntityBalanceMigrationTest.java Outdated Show resolved Hide resolved

moving migration initialization to before each.

e7ddc3a

Signed-off-by: mgoelswirlds <mugdha.goel@swirldslabs.com>

mgoelswirlds dismissed xin-hedera’s stale review via e7ddc3a August 18, 2023 16:57

steven-sheehy approved these changes Aug 18, 2023

View reviewed changes

mgoelswirlds requested a review from xin-hedera August 18, 2023 17:05

xin-hedera approved these changes Aug 18, 2023

View reviewed changes

mgoelswirlds merged commit 74fb92b into main Aug 18, 2023
24 of 25 checks passed

mgoelswirlds deleted the 6397-Time-sensitive-migrations branch August 18, 2023 18:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rerun conditional migrations #6565

Rerun conditional migrations #6565

mgoelswirlds commented Aug 1, 2023 •

edited

Loading

codecov bot commented Aug 1, 2023 •

edited

Loading

mgoelswirlds Aug 2, 2023

mgoelswirlds Aug 2, 2023

mgoelswirlds Aug 2, 2023

mgoelswirlds Aug 3, 2023

mgoelswirlds Aug 4, 2023

mgoelswirlds Aug 4, 2023

xin-hedera Aug 4, 2023 •

edited

Loading

xin-hedera Aug 9, 2023 •

edited by mgoelswirlds

Loading

mgoelswirlds Aug 9, 2023 •

edited

Loading

xin-hedera Aug 9, 2023

mgoelswirlds Aug 10, 2023

mgoelswirlds Aug 10, 2023

xin-hedera left a comment

steven-sheehy left a comment

sonarcloud bot commented Aug 18, 2023

xin-hedera left a comment

Rerun conditional migrations #6565

Rerun conditional migrations #6565

Conversation

mgoelswirlds commented Aug 1, 2023 • edited Loading

codecov bot commented Aug 1, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xin-hedera Aug 4, 2023 • edited Loading

Choose a reason for hiding this comment

xin-hedera Aug 9, 2023 • edited by mgoelswirlds Loading

Choose a reason for hiding this comment

mgoelswirlds Aug 9, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xin-hedera left a comment

Choose a reason for hiding this comment

steven-sheehy left a comment

Choose a reason for hiding this comment

sonarcloud bot commented Aug 18, 2023

xin-hedera left a comment

Choose a reason for hiding this comment

mgoelswirlds commented Aug 1, 2023 •

edited

Loading

codecov bot commented Aug 1, 2023 •

edited

Loading

xin-hedera Aug 4, 2023 •

edited

Loading

xin-hedera Aug 9, 2023 •

edited by mgoelswirlds

Loading

mgoelswirlds Aug 9, 2023 •

edited

Loading