-
Notifications
You must be signed in to change notification settings - Fork 108
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
Fix slow domain object ingestion #2006
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2006 +/- ##
=============================================
+ Coverage 57.92% 79.96% +22.03%
- Complexity 0 2065 +2065
=============================================
Files 56 387 +331
Lines 2491 9619 +7128
Branches 325 892 +567
=============================================
+ Hits 1443 7692 +6249
- Misses 903 1660 +757
- Partials 145 267 +122
Continue to review full report at Codecov.
|
hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/domain/Entity.java
Outdated
Show resolved
Hide resolved
hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/parser/PgCopy.java
Outdated
Show resolved
Hide resolved
hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/domain/Entity.java
Outdated
Show resolved
Hide resolved
hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/domain/Entity.java
Show resolved
Hide resolved
hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/domain/Entity.java
Outdated
Show resolved
Hide resolved
hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/domain/Schedule.java
Outdated
Show resolved
Hide resolved
hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/domain/Entity.java
Outdated
Show resolved
Hide resolved
hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/domain/Token.java
Outdated
Show resolved
Hide resolved
...ter/src/main/java/com/hedera/mirror/importer/parser/record/entity/sql/SqlEntityListener.java
Outdated
Show resolved
Hide resolved
96042de
to
6b6a9af
Compare
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.
Some really good work here, just a few things.
hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/domain/TokenAccountId.java
Show resolved
Hide resolved
hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/parser/PgCopy.java
Outdated
Show resolved
Hide resolved
hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/parser/PgCopy.java
Outdated
Show resolved
Hide resolved
.../java/com/hedera/mirror/importer/parser/balance/AccountBalanceFileParserPerformanceTest.java
Outdated
Show resolved
Hide resolved
...java/com/hedera/mirror/importer/parser/record/entity/EntityRecordItemListenerCryptoTest.java
Outdated
Show resolved
Hide resolved
hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/domain/TokenId.java
Show resolved
Hide resolved
...ter/src/main/java/com/hedera/mirror/importer/parser/record/entity/sql/SqlEntityListener.java
Outdated
Show resolved
Hide resolved
...ter/src/main/java/com/hedera/mirror/importer/parser/record/entity/sql/SqlEntityListener.java
Outdated
Show resolved
Hide resolved
...ter/src/main/java/com/hedera/mirror/importer/parser/record/entity/sql/SqlEntityListener.java
Outdated
Show resolved
Hide resolved
...er/src/test/java/com/hedera/mirror/importer/repository/EntityIdRepositoryCustomImplTest.java
Outdated
Show resolved
Hide resolved
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.
Still need to look at the sql generation more
...or-importer/src/main/java/com/hedera/mirror/importer/converter/NullableStringSerializer.java
Outdated
Show resolved
Hide resolved
...or-importer/src/main/java/com/hedera/mirror/importer/converter/NullableStringSerializer.java
Outdated
Show resolved
Hide resolved
hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/domain/Entity.java
Outdated
Show resolved
Hide resolved
hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/domain/TokenAccountId.java
Show resolved
Hide resolved
hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/parser/PgCopy.java
Outdated
Show resolved
Hide resolved
.../java/com/hedera/mirror/importer/parser/record/entity/EntityRecordItemListenerTokenTest.java
Outdated
Show resolved
Hide resolved
.../hedera/mirror/importer/parser/record/transactionhandler/AbstractTransactionHandlerTest.java
Outdated
Show resolved
Hide resolved
...porter/src/main/java/com/hedera/mirror/importer/repository/ScheduleRepositoryCustomImpl.java
Outdated
Show resolved
Hide resolved
...er/src/main/java/com/hedera/mirror/importer/repository/TokenAccountRepositoryCustomImpl.java
Outdated
Show resolved
Hide resolved
...ter/src/main/java/com/hedera/mirror/importer/repository/UpdatableDomainRepositoryCustom.java
Outdated
Show resolved
Hide resolved
485e4d3
to
87ab15c
Compare
hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/parser/UpsertPgCopy.java
Show resolved
Hide resolved
...or-importer/src/main/java/com/hedera/mirror/importer/converter/NullableStringSerializer.java
Outdated
Show resolved
Hide resolved
...or-importer/src/main/java/com/hedera/mirror/importer/converter/NullableStringSerializer.java
Outdated
Show resolved
Hide resolved
hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/parser/PgCopy.java
Outdated
Show resolved
Hide resolved
hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/parser/UpsertPgCopy.java
Outdated
Show resolved
Hide resolved
hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/parser/UpsertPgCopy.java
Show resolved
Hide resolved
...ter/src/main/java/com/hedera/mirror/importer/parser/record/entity/sql/SqlEntityListener.java
Show resolved
Hide resolved
...ter/src/main/java/com/hedera/mirror/importer/parser/record/entity/sql/SqlEntityListener.java
Outdated
Show resolved
Hide resolved
.../java/com/hedera/mirror/importer/parser/balance/AccountBalanceFileParserPerformanceTest.java
Outdated
Show resolved
Hide resolved
...r/src/main/java/com/hedera/mirror/importer/repository/upsert/EntityUpsertQueryGenerator.java
Outdated
Show resolved
Hide resolved
...ter/src/main/java/com/hedera/mirror/importer/parser/record/entity/sql/SqlEntityListener.java
Show resolved
Hide resolved
...r/src/main/java/com/hedera/mirror/importer/repository/upsert/EntityUpsertQueryGenerator.java
Outdated
Show resolved
Hide resolved
...or-importer/src/main/java/com/hedera/mirror/importer/converter/NullableStringSerializer.java
Outdated
Show resolved
Hide resolved
...orter/src/main/java/com/hedera/mirror/importer/migration/HistoricalAccountInfoMigration.java
Outdated
Show resolved
Hide resolved
hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/parser/UpsertPgCopy.java
Show resolved
Hide resolved
...main/java/com/hedera/mirror/importer/repository/upsert/TokenAccountUpsertQueryGenerator.java
Outdated
Show resolved
Hide resolved
...src/main/java/com/hedera/mirror/importer/repository/upsert/AbstractUpsertQueryGenerator.java
Show resolved
Hide resolved
...com/hedera/mirror/importer/parser/record/entity/repository/RepositoryEntityListenerTest.java
Outdated
Show resolved
Hide resolved
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.
Will give another pass, other than the mentioned unused code I haven't seen anything.
Signed-off-by: Nana-EC <nana.essilfie-conduah@hedera.com>
Signed-off-by: Nana-EC <nana.essilfie-conduah@hedera.com>
Signed-off-by: Nana-EC <nana.essilfie-conduah@hedera.com>
Signed-off-by: Nana-EC <nana.essilfie-conduah@hedera.com>
Signed-off-by: Nana-EC <nana.essilfie-conduah@hedera.com>
Signed-off-by: Nana-EC <nana.essilfie-conduah@hedera.com>
Signed-off-by: Nana-EC <nana.essilfie-conduah@hedera.com>
Signed-off-by: Nana-EC <nana.essilfie-conduah@hedera.com>
Signed-off-by: Nana-EC <nana.essilfie-conduah@hedera.com>
Signed-off-by: Nana-EC <nana.essilfie-conduah@hedera.com>
Signed-off-by: Nana-EC <nana.essilfie-conduah@hedera.com>
Signed-off-by: Nana-EC <nana.essilfie-conduah@hedera.com>
Signed-off-by: Nana-EC <nana.essilfie-conduah@hedera.com>
Signed-off-by: Nana-EC <nana.essilfie-conduah@hedera.com>
Signed-off-by: Nana-EC <nana.essilfie-conduah@hedera.com>
Signed-off-by: Nana-EC <nana.essilfie-conduah@hedera.com>
Signed-off-by: Nana-EC <nana.essilfie-conduah@hedera.com>
Signed-off-by: Nana-EC <nana.essilfie-conduah@hedera.com>
ed02c44
to
d7bab05
Compare
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.
LGTM
Signed-off-by: Nana-EC <nana.essilfie-conduah@hedera.com>
SonarCloud Quality Gate failed. |
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.
LGTM
Detailed description:
Slow entity ingestions flows exists. This is mainly because we do multiple db reference calls to check for existing entities to get the up to date info.
Insert only flows are fast with PgCopy. Taking inspiration from this update scenarios could utilize PgCopy to copy domain objects over to db server, then follow up with an upsert query to manage the insert vs update scenarios.
EntityRecordItemListener
EntityRecordItemListener
to pass all entities, tokens and schedules domain objects with available info toEntityListener
onX()
equivalentsonEntityId()
withonEntityId()
UpsertPgCopy
class that creates a temp table for an entity, copies into this, and then runs a domain specificinsertSql
andupdateSql
queries to manage upsert to main tableEntityIdSerializer
usage inPgCopy
and remove domain field annotation usage@JsonSerialize
annotation to correctly serialize in PgCopy flowgetNewAccountFreezeStatus()
andgetNewAccountKycStatus()
and move logic into insert sql commandNullableStringSerializer
to support differentiation of String intention to set null vs unset, since PgCipy serializes empty string and null to be nullhibernate-jpamodelgen
dependency to automatically generate Models based on domain classes for type safetyUpsertQueryGenerator
interface with implementations for updatable domains that automatically generate sql queries for temp table creation, insert and update to final tableNEVER_EXPIRE_LARGE
cache creation and usageentity.deleted
Which issue(s) this PR fixes:
Fixes #1926
Fixes #1440
Special notes for your reviewer:
multiEntityUpdate() test outputs
Upsert
Inserting 6000 new entities took 78 s previously now 223.7 ms
Inserting 6000 entities with 3000 being updates took 34 s now takes 400 ms
Insert & Update
Inserting 6000 new entities took 78 s previously now ~160.7 ms
Inserting 6000 entities with 3000 being updates took 34 s now takes ~180 ms
Logs outputs
Summary:
Ingestion times improved 97%, new time is 3% of previous time
Insert only: before ~21s, after ~600ms
Insert & Update: before: ~22s, after ~500 ms
Extended:
Before (current master code)
6k entities insert only (Run 1)
6k entities insert only (Run 2)
6k entities insert with 3k being updates (Run 1)
6k entities insert with 3k being updates (Run 2)
After (PR code)
6k entities insert only (Run 1)
6k entities insert only (Run 2)
6k entities insert with 3k being updates (Run 1)
6k entities insert with 3k being updates (Run 2)
Still need to
Consider whether expandingOnX(domain)
of entityListener toOnX(domain, update)
has value. This would allow it to know update vs new and would allow for insert vs update pathChecklist