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

Call AccountsDb::clean_accounts() directly, inside Bank::verify_snapshot_bank() #27258

Conversation

brooksprumo
Copy link
Contributor

@brooksprumo brooksprumo commented Aug 19, 2022

Problem

On startup from a snapshot, the bank is verified after deserialization. In that fn, Bank::verify_snapshot_bank(), it calls clean and shrink. However, clean and shrink specify different max_clean_roots. These should be the same.

Since were at startup, there's no outstanding scans that could occur, and no reason why we cannot clean all slots. We do need to make sure 0-lamport accounts are still kept around for slots past the last full snapshot, and that behavior has not changed.

See #27200 for full context

Summary of Changes

In Bank::verify_snapshot_bank(), call AccountsDb::clean_accounts() directly, and set max_clean_roots to None.

@brooksprumo brooksprumo added work in progress This isn't quite right yet noCI Suppress CI on this Pull Request labels Aug 19, 2022
@brooksprumo brooksprumo force-pushed the clean-accounts/call-clean-accounts-directly branch from 90a4b6d to 748ffc6 Compare August 19, 2022 16:14
@brooksprumo brooksprumo added CI Pull Request is ready to enter CI and removed noCI Suppress CI on this Pull Request labels Aug 19, 2022
@solana-grimes solana-grimes removed the CI Pull Request is ready to enter CI label Aug 19, 2022
@brooksprumo brooksprumo force-pushed the clean-accounts/call-clean-accounts-directly branch from 748ffc6 to 4e96e43 Compare August 19, 2022 16:44
@brooksprumo brooksprumo force-pushed the clean-accounts/call-clean-accounts-directly branch from 4e96e43 to 0a56276 Compare August 19, 2022 22:16
@brooksprumo brooksprumo removed the work in progress This isn't quite right yet label Aug 19, 2022
Comment on lines +7162 to +7165
self.rc
.accounts
.accounts_db
.clean_accounts(None, true, Some(last_full_snapshot_slot));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that previously we'd call clean_accounts() with max_clean_root = Some(self.slot() - 1), and now we call clean_accounts() with max_root = None.

I believe this is safe for the reasons I put in the PR description. Please let me know if I've missed something!

@brooksprumo brooksprumo marked this pull request as ready for review August 22, 2022 16:57
Copy link
Contributor

@jeffwashington jeffwashington left a comment

Choose a reason for hiding this comment

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

lgtm

@brooksprumo brooksprumo merged commit 9daa410 into solana-labs:master Aug 22, 2022
@brooksprumo brooksprumo deleted the clean-accounts/call-clean-accounts-directly branch August 22, 2022 17:56
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.

3 participants