Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Creating an address takes a massive amount of time on large wallets #2051

Closed
KtorZ opened this issue Aug 19, 2020 · 3 comments
Closed

Creating an address takes a massive amount of time on large wallets #2051

KtorZ opened this issue Aug 19, 2020 · 3 comments
Assignees
@KtorZ
Labels
Bug PRIORITY:HIGH Require immediate attention. SEVERITY:MEDIUM Visible impact on a core functionality or some important performance degradation.

Comments

@KtorZ
Copy link
Member

KtorZ commented Aug 19, 2020 •
edited
Loading

Context

Information -
Version
Platform
Installation

Steps to Reproduce

  1. Have a large Byron wallet (100K+ transactions, 50K+ UTxO, 50K+ existing addresses).
  2. Import a single address using the /byron-wallets/addresses endpoint

Expected behavior

  1. The address is promptly imported.

Actual behavior

  1. The wallet hangs for several minutes before completing the operation.

Resolution

  • A possible started would be to look more closely at the strictness of the scanl used in the handler.

QA

BenchRndResults:
  importOneAddressTime: 1.510 ms
  importManyAddressesTime: 187.3 ms
  utxoStatistics:
    = Total value of -0 lovelace across 0 UTxOs
 
BenchRndResults:
  importOneAddressTime: 328.9 ms
  importManyAddressesTime: 789.7 ms
  utxoStatistics:
    = Total value of 21515725326443 lovelace across 747 UTxOs
 
BenchRndResults:
  importOneAddressTime: 942.2 ms
  importManyAddressesTime: 1.886 s
  utxoStatistics:
    = Total value of 31020480179736 lovelace across 1392 UTxOs

BenchRndResults:
  importOneAddressTime: 1.770 s
  importManyAddressesTime: 3.551 s
  utxoStatistics:
    = Total value of 69284306530582 lovelace across 2779 UTxOs

Note: many = 1000
@KtorZ KtorZ added Bug SEVERITY:MEDIUM Visible impact on a core functionality or some important performance degradation. PRIORITY:HIGH Require immediate attention. labels Aug 19, 2020
@KtorZ KtorZ self-assigned this Aug 19, 2020
@KtorZ KtorZ added this to Backlog in Adrestia via automation Aug 19, 2020
@KtorZ
Copy link
Member Author

KtorZ commented Aug 20, 2020 •
edited
Loading

Even on a rather "small" wallet, importing 10K addresses took 5s, and then, importing a new address took more than 10minute. So there's something quite fishy.

@KtorZ
Copy link
Member Author

KtorZ commented Aug 27, 2020

On different runs, it seems like the behaviour is not entirely consistent. Somehow, the stake pool restoration worker also seem to have an impact when creating addresses. Once the wallet is fully synced and done fetching metadata, it seems like creating new addresses is done in a decent amount of time. I have however been very inconsistent in my testing here so I am planning to extend the nightly restoration benchmark to see how this actually performs there.

@KtorZ KtorZ mentioned this issue Sep 4, 2020
Merged
iohk-bors bot added a commit that referenced this issue Sep 7, 2020
@iohk-bors @KtorZ
Merge #2112
1ffa324 
2112: simplify implementation of 'estimateMaxNumberOfInputs' & increase coverage r=KtorZ a=KtorZ

# Issue Number

<!-- Put here a reference to the issue this PR relates to and which requirements it tackles -->

#2051 

# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- I've added scenarios for Byron and Icarus keys
- I've added a 'within' clause to some scenarios in order to make sure that the properties are executed within a reasonable time.

# Comments

<!-- Additional comments or screenshots to attach if any -->

This didn't really improve execution time significantly, but it is still useful to keep.

<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: KtorZ <matthias.benkort@gmail.com>
iohk-bors bot added a commit that referenced this issue Sep 7, 2020
@iohk-bors @KtorZ
Merge #2112
889a91a 
2112: simplify implementation of 'estimateMaxNumberOfInputs' & increase coverage r=KtorZ a=KtorZ

# Issue Number

<!-- Put here a reference to the issue this PR relates to and which requirements it tackles -->

#2051 

# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- I've added scenarios for Byron and Icarus keys
- I've added a 'within' clause to some scenarios in order to make sure that the properties are executed within a reasonable time.

# Comments

<!-- Additional comments or screenshots to attach if any -->

This didn't really improve execution time significantly, but it is still useful to keep.

<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: KtorZ <matthias.benkort@gmail.com>
iohk-bors bot added a commit that referenced this issue Sep 7, 2020
@iohk-bors @KtorZ
Merge #2112
09d008a 
2112: simplify implementation of 'estimateMaxNumberOfInputs' & increase coverage r=KtorZ a=KtorZ

# Issue Number

<!-- Put here a reference to the issue this PR relates to and which requirements it tackles -->

#2051 

# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- I've added scenarios for Byron and Icarus keys
- I've added a 'within' clause to some scenarios in order to make sure that the properties are executed within a reasonable time.

# Comments

<!-- Additional comments or screenshots to attach if any -->

This didn't really improve execution time significantly, but it is still useful to keep.

<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: KtorZ <matthias.benkort@gmail.com>
iohk-bors bot added a commit that referenced this issue Sep 8, 2020
@iohk-bors @KtorZ
Merge #2112
3feb1d0 
2112: simplify implementation of 'estimateMaxNumberOfInputs' & increase coverage r=KtorZ a=KtorZ

# Issue Number

<!-- Put here a reference to the issue this PR relates to and which requirements it tackles -->

#2051 

# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- I've added scenarios for Byron and Icarus keys
- I've added a 'within' clause to some scenarios in order to make sure that the properties are executed within a reasonable time.

# Comments

<!-- Additional comments or screenshots to attach if any -->

This didn't really improve execution time significantly, but it is still useful to keep.

<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: KtorZ <matthias.benkort@gmail.com>
@KtorZ KtorZ moved this from Backlog to QA in Adrestia Sep 9, 2020
@piotr-iohk
Copy link
Contributor

lgtm

Adrestia automation moved this from QA to Closed Sep 9, 2020
@CharlesMorgan-IOHK CharlesMorgan-IOHK removed this from Closed in Adrestia Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@KtorZ KtorZ

Labels
Bug PRIORITY:HIGH Require immediate attention. SEVERITY:MEDIUM Visible impact on a core functionality or some important performance degradation.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@KtorZ @piotr-iohk