Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

calculate ref counts earlier to prevent bad clean#9147

Merged
sakridge merged 1 commit intosolana-labs:masterfrom
sakridge:calc-refs-earlier
Mar 29, 2020
Merged

calculate ref counts earlier to prevent bad clean#9147
sakridge merged 1 commit intosolana-labs:masterfrom
sakridge:calc-refs-earlier

Conversation

@sakridge
Copy link
Copy Markdown
Contributor

@sakridge sakridge commented Mar 29, 2020

Problem

Storage ref counts can prevent an account from being cleaned, but that also affects if another item would be cleaned. We don't go back and update the storage counts of those other items.

Summary of Changes

Calculate ref counts in earlier loop which does re-calculate the storage counts.

Fixes #9137

@sakridge sakridge requested a review from ryoqun March 29, 2020 03:55
@sakridge sakridge force-pushed the calc-refs-earlier branch from 837f129 to 027f570 Compare March 29, 2020 03:58
Comment thread runtime/src/accounts_db.rs Outdated
@ryoqun
Copy link
Copy Markdown
Contributor

ryoqun commented Mar 29, 2020

if possible, we'd like to have a test for this although writing one would be cumbersome as this is a bit a corner case.

@ryoqun
Copy link
Copy Markdown
Contributor

ryoqun commented Mar 29, 2020

@mvines Good news! I think this fixes yesterday's snapshot error with highly-likely chance. it seems that @sakridge and I independently reached to the same root cause and fix for it, although @sakridge was a ahead than me. ;)

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 29, 2020

Codecov Report

Merging #9147 into master will decrease coverage by 0.0%.
The diff coverage is 100.0%.

@@           Coverage Diff            @@
##           master   #9147     +/-   ##
========================================
- Coverage    80.5%   80.5%   -0.1%     
========================================
  Files         271     271             
  Lines       59610   59611      +1     
========================================
  Hits        48026   48026             
- Misses      11584   11585      +1     

@sakridge sakridge force-pushed the calc-refs-earlier branch from 027f570 to 6497848 Compare March 29, 2020 18:38
@sakridge sakridge force-pushed the calc-refs-earlier branch from 6497848 to aa6cc73 Compare March 29, 2020 19:11
@sakridge sakridge removed the v1.0 label Mar 29, 2020
@sakridge sakridge merged commit b1771b9 into solana-labs:master Mar 29, 2020
@sakridge sakridge deleted the calc-refs-earlier branch March 29, 2020 21:42
mergify Bot pushed a commit that referenced this pull request Mar 29, 2020
Copy link
Copy Markdown
Contributor

@ryoqun ryoqun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for cleaning up the logic!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MismatchedBankHash when loading a mainnet-beta snapshot

3 participants