This repository has been archived by the owner on Jan 22, 2025. It is now read-only.
Handle cleaning of zero-lamport accounts w.r.t. Incremental Snapshots #18870
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #18870 +/- ##
========================================
Coverage 82.8% 82.9%
========================================
Files 453 453
Lines 128741 128967 +226
========================================
+ Hits 106705 106977 +272
+ Misses 22036 21990 -46 |
brooksprumo
force-pushed
the
iss-v3-zero-lamport-accounts
branch
from
d882d4a to
886697b
Compare
|
@carllin @lijunwangs @ryoqun This PR is now ready for review, but it is built on top of PR #18565. So this PR currently contains extra changes that will go away once the other PR is merged in. Feel free to review these changes, which are mostly in Thanks in advance! |
brooksprumo
force-pushed
the
iss-v3-zero-lamport-accounts
branch
from
886697b to
e2a0ad2
Compare
carllin
reviewed
Jul 31, 2021
carllin
reviewed
Jul 31, 2021
carllin
reviewed
Jul 31, 2021
carllin
reviewed
Jul 31, 2021
carllin
reviewed
Aug 2, 2021
carllin
reviewed
Aug 2, 2021
|
Moving this back to Draft as I iterate on it. |
brooksprumo
force-pushed
the
iss-v3-zero-lamport-accounts
branch
6 times, most recently
from
2db8884 to
0572bc0
Compare
|
@carllin OK, PR should be good-to-review again! I've attempted to put the correct tests/checks in place, but I may've missed some. Hopefully this is moving in the right direction though! |
carllin
reviewed
Aug 10, 2021
carllin
reviewed
Aug 10, 2021
runtime/src/accounts_db.rs
Outdated
Comment on lines
1785
to
1802
| assert!( | ||
| last_full_snapshot_slot.is_some() || self.zero_lamport_accounts_to_purge_later.is_empty(), | ||
| "if snapshots are disabled, then zero_lamport_accounts_to_purge_later should always be empty" | ||
| ); |
carllin
reviewed
Aug 10, 2021
carllin
reviewed
Aug 10, 2021
carllin
reviewed
Aug 10, 2021
runtime/src/accounts_db.rs
Outdated
Comment on lines
2013
to
2014
| self.zero_lamport_accounts_to_purge_later | ||
| .insert((*slot, *pubkey)); |
There was a problem hiding this comment.
looks like the crux of it 😃
lijunwangs
reviewed
Aug 10, 2021
Handle cleaning zero-lamport accounts in slots higher than the last full snapshot slot. This is part of the Incremental Snapshot work. Fixes solana-labs#18825
brooksprumo
force-pushed
the
iss-v3-zero-lamport-accounts
branch
from
e68dbb8 to
3af2c62
Compare
brooksprumo
force-pushed
the
iss-v3-zero-lamport-accounts
branch
from
76c0527 to
450be98
Compare
carllin
reviewed
Aug 10, 2021
carllin
reviewed
Aug 11, 2021
carllin
reviewed
Aug 12, 2021
Comment on lines
+12290
to
+12296
| let do_test = |test_params: TestParameters| { | ||
| let account_info = AccountInfo { | ||
| store_id: 42, | ||
| offset: 123, | ||
| stored_size: 234, | ||
| lamports: 0, | ||
| }; |
carllin
approved these changes
Aug 12, 2021
steviez
pushed a commit
to steviez/solana
that referenced
this pull request
Aug 12, 2021
steviez
pushed a commit
that referenced
this pull request
Aug 12, 2021
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
Zero-lamport accounts are improperly cleaned up in the context of incremental snapshots.
Summary of Changes
Option<Slot>parameter toAccountsDb::clean_accounts()for thelast_full_snapshot_slot. IfSome, thenpurges_zero_lamportswill be checked against this slot. Any purges for slots higher than the last full snapshot slot will be filtered out (i.e. _not_purged).last_full_snapshot_slotparameterclean_accounts(). Most all callers passNonenow.Fixes #18825