-
Notifications
You must be signed in to change notification settings - Fork 115
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
Remove foreign keys on entity id #817
Conversation
5ebf23d
to
132863a
Compare
Signed-off-by: Apekshit Sharma <[email protected]>
Signed-off-by: Apekshit Sharma <[email protected]>
Signed-off-by: Apekshit Sharma <[email protected]>
Signed-off-by: Apekshit Sharma <[email protected]>
Signed-off-by: Apekshit Sharma <[email protected]>
dbb6688
to
6ca7b7d
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.
Overall looks good. There's a couple that need to be fixed and the rest can be addressed in a follow up.
@Id | ||
@GeneratedValue(strategy = GenerationType.IDENTITY) | ||
private Long id; |
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.
We should later convert this id to EntityId
hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/domain/Entities.java
Show resolved
Hide resolved
hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/domain/Transaction.java
Show resolved
Hide resolved
...ter/src/main/java/com/hedera/mirror/importer/parser/record/entity/sql/SqlEntityListener.java
Show resolved
Hide resolved
.../src/main/java/com/hedera/mirror/importer/parser/record/entity/EntityRecordItemListener.java
Show resolved
Hide resolved
hedera-mirror-importer/src/main/resources/db/migration/V1.25.0__denormalize_entities.sql
Outdated
Show resolved
Hide resolved
...ra-mirror-importer/src/main/java/com/hedera/mirror/importer/repository/EntityRepository.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Apekshit Sharma <[email protected]>
Signed-off-by: Apekshit Sharma <[email protected]>
Signed-off-by: Apekshit Sharma <[email protected]>
6ca7b7d
to
487486f
Compare
hedera-mirror-importer/src/main/resources/db/migration/V1.25.0__denormalize_entities.sql
Show resolved
Hide resolved
hedera-mirror-importer/src/main/resources/db/migration/V1.25.0__denormalize_entities.sql
Show resolved
Hide resolved
hedera-mirror-importer/src/main/resources/db/migration/V1.25.0__denormalize_entities.sql
Show resolved
Hide resolved
hedera-mirror-importer/src/main/resources/db/migration/V1.25.0__denormalize_entities.sql
Show resolved
Hide resolved
hedera-mirror-importer/src/main/resources/db/migration/V1.25.0__denormalize_entities.sql
Show resolved
Hide resolved
hedera-mirror-importer/src/test/java/com/hedera/mirror/importer/util/EntityIdEndecTest.java
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.
Huge change. Nice job.
Left some comments but you can address them if you so chose in a future smaller PR :)
Thanks for the review guys, will post quick followup PR. |
Signed-off-by: Apekshit Sharma <[email protected]>
Signed-off-by: Apekshit Sharma <[email protected]>
Signed-off-by: Apekshit Sharma <[email protected]>
Signed-off-by: Apekshit Sharma <[email protected]>
Signed-off-by: Apekshit Sharma <[email protected]>
Detailed description:
t_entities
tableidx_t_transactions_node_account
is not created for the new table (Explore usefulness of indexes idx_t_transactions_node_account and idx_t_transactions_payer_id #548)Perf testing:
Importer:
- Setup
Db preloaded with 2.93 million transactions
Workload: ~700k txns (20k tps, 7 files)
- Stats
Migration time: 20s
TPS:
Which issue(s) this PR fixes:
Fixes #570
Partly addresses #548
Special notes for your reviewer:
Checklist