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

ledger: fix accounts cache ordering #4611

Merged
merged 1 commit into from
Sep 30, 2022

Conversation

algorandskiy
Copy link
Contributor

@algorandskiy algorandskiy commented Sep 29, 2022

Summary

There was a copy-paste error in account in persistedOnlineAccountData.before used by online accounts LRU cache. persistedOnlineAccountData.updRound must be used for ordering since online accounts entries are not squashed together in a commit range but every entry generates a new row in onlineaccounts table. Having that, the persistedOnlineAccountData.round field set to the trackerDB round and used in accounts LRU cache ordering prevent entries of the same address being added into the cache. As a result this is a cache miss and additional lookups.

Test

Added a unit test

@codecov
Copy link

codecov bot commented Sep 29, 2022

Codecov Report

Merging #4611 (40503eb) into master (5be155b) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #4611      +/-   ##
==========================================
- Coverage   54.20%   54.19%   -0.01%     
==========================================
  Files         402      402              
  Lines       51830    51830              
==========================================
- Hits        28096    28091       -5     
- Misses      21370    21374       +4     
- Partials     2364     2365       +1     
Impacted Files Coverage Δ
ledger/accountdb.go 72.87% <100.00%> (ø)
catchup/peerSelector.go 98.95% <0.00%> (-1.05%) ⬇️
catchup/service.go 68.64% <0.00%> (-0.25%) ⬇️
network/wsNetwork.go 64.63% <0.00%> (-0.20%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@algorandskiy algorandskiy merged commit abe1377 into algorand:master Sep 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants