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

Don't hold storage accounts lock during scan #12132

Closed
wants to merge 2 commits into from

Conversation

carllin
Copy link
Contributor

@carllin carllin commented Sep 9, 2020

Problem

Accounts scans from RPC hold:

  1. the account storage lock duration the entire duration of the scan, which blocks replay on create_and_insert_store() during account commit.

  2. the account index lock duration the entire duration of the scan, which blocks replay in AccountsDb store()->update_index() during account commit.

Summary of Changes

Addresses problem 1). Release storage lock after each account lookup in scan.

Compared to #12126:

Pros: Is less disruptive

Cons:

  1. is less efficient, need to repeatedly grab storage lock,
  2. storage lock lookup still contends briefly with storage lock write in create_and_insert_store()
  3. Doesn't generalize as easily to also solve problem 2) the accounts index lock above, need to handle this problem separately

Fixes #

@carllin carllin changed the title Don't hold storage lock during scan Don't hold storage accounts lock during scan Sep 9, 2020
@codecov
Copy link

codecov bot commented Sep 9, 2020

Codecov Report

Merging #12132 into master will increase coverage by 0.0%.
The diff coverage is 100.0%.

@@           Coverage Diff           @@
##           master   #12132   +/-   ##
=======================================
  Coverage    82.0%    82.0%           
=======================================
  Files         337      337           
  Lines       79249    79250    +1     
=======================================
+ Hits        65024    65031    +7     
+ Misses      14225    14219    -6     

@stale
Copy link

stale bot commented Sep 16, 2020

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale [bot only] Added to stale content; results in auto-close after a week. label Sep 16, 2020
@carllin carllin closed this Sep 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale [bot only] Added to stale content; results in auto-close after a week.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant