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.

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

Improve Inactive Reconciler Performance when --lookup-balance-by-block=false #66

Merged
merged 9 commits into from
Jul 15, 2020

Conversation

patrick-ogrady
Copy link
Contributor

@patrick-ogrady patrick-ogrady commented Jul 14, 2020

Motivation

Fix #65

Solution

  • Ensure all seen accounts are added to the inactiveAccountQueue even if we are far behind tip
  • Ensure accounts aren't leaked from the inactiveAccountQueue (could occur if accounts
  • Improve performance by changing frequent operation (seenAccounts check on each block) from O(n) to ~O(1) (caused reconciler to get much slower as more accounts seen).
  • Ensure inactive reconciler respects the highWaterMark (so useless queries are not performed during syncing).
  • Fix edge case issue where an account could be added to the queue multiple times (in different go routines). This fix was possible because a lookup is performed while holding the lock and that lookup is now ~O(1) time now.

Other Thoughts

We could further optimize the reconciler to cache balances it retrieved when behind tip so that reconciliation could begin before making it to tip. This would require some careful thinking and testing and should be attempted in another PR.

@coveralls
Copy link

coveralls commented Jul 14, 2020

Pull Request Test Coverage Report for Build 2540

  • 14 of 96 (14.58%) changed or added relevant lines in 2 files are covered.
  • 3 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-1.0%) to 64.759%

Changes Missing Coverage Covered Lines Changed/Added Lines %
reconciler/configuration.go 1 9 11.11%
reconciler/reconciler.go 13 87 14.94%
Files with Coverage Reduction New Missed Lines %
reconciler/reconciler.go 3 27.7%
Totals Coverage Status
Change from base Build 2433: -1.0%
Covered Lines: 1867
Relevant Lines: 2883

💛 - Coveralls

@patrick-ogrady patrick-ogrady changed the title [WIP] Improve Inactive Reconciler [WIP] Improve Inactive Reconciler when --lookup-balance-by-block=false Jul 14, 2020
@patrick-ogrady patrick-ogrady force-pushed the patrick/inactive-reconciler-improvements branch from 37e2236 to 0cc8cda Compare July 14, 2020 22:25
@patrick-ogrady patrick-ogrady changed the title [WIP] Improve Inactive Reconciler when --lookup-balance-by-block=false Improve Inactive Reconciler when --lookup-balance-by-block=false Jul 14, 2020
@patrick-ogrady patrick-ogrady changed the title Improve Inactive Reconciler when --lookup-balance-by-block=false Inactive Reconciler Performance when --lookup-balance-by-block=false Jul 14, 2020
@patrick-ogrady patrick-ogrady force-pushed the patrick/inactive-reconciler-improvements branch from ff0c8ad to 48272b8 Compare July 14, 2020 23:15
@patrick-ogrady patrick-ogrady force-pushed the patrick/inactive-reconciler-improvements branch from c249946 to 745b911 Compare July 15, 2020 01:07
@heimdall-asguard
Copy link

Review Error for juliankoh @ 2020-07-15 11:03:36 UTC
User failed mfa authentication, public email is not set on your github profile. see go/mfa-help

@heimdall-asguard
Copy link

Review Error for juliankoh @ 2020-07-15 11:04:24 UTC
User failed mfa authentication, public email is not set on your github profile. see go/mfa-help

@patrick-ogrady patrick-ogrady merged commit 5b24300 into master Jul 15, 2020
@patrick-ogrady patrick-ogrady deleted the patrick/inactive-reconciler-improvements branch July 15, 2020 14:36
@patrick-ogrady patrick-ogrady changed the title Inactive Reconciler Performance when --lookup-balance-by-block=false Improve Inactive Reconciler Performance when --lookup-balance-by-block=false Jul 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants