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

Set drop callback on first root bank#23999

Merged
carllin merged 2 commits intosolana-labs:masterfrom
carllin:FixBlockstoreProcessor
Apr 5, 2022
Merged

Set drop callback on first root bank#23999
carllin merged 2 commits intosolana-labs:masterfrom
carllin:FixBlockstoreProcessor

Conversation

@carllin
Copy link
Copy Markdown
Contributor

@carllin carllin commented Mar 30, 2022

Problem

See comment here: #23970 (comment)

Summary of Changes

  1. Set the drop callback on the first bank after snapshot unpacking
  2. blockstore_processor::load_frozen_forks() now handles the Bank::drop() -> purge_slot() call like AccountsBackgroundService would as part of its periodic cleanup

Fixes #

Copy link
Copy Markdown
Contributor

@brooksprumo brooksprumo left a comment

Choose a reason for hiding this comment

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

This design looks good to me! Can you re-request a review once CI checks are ✅?

Comment thread ledger/src/blockstore_processor.rs
@carllin carllin force-pushed the FixBlockstoreProcessor branch from f1f2172 to 1774d0f Compare April 1, 2022 00:10
@carllin carllin force-pushed the FixBlockstoreProcessor branch from 1774d0f to 5b2fc43 Compare April 1, 2022 19:00
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 2, 2022

Codecov Report

Merging #23999 (5b2fc43) into master (c9a476e) will increase coverage by 0.0%.
The diff coverage is 86.2%.

@@           Coverage Diff           @@
##           master   #23999   +/-   ##
=======================================
  Coverage    81.6%    81.7%           
=======================================
  Files         589      589           
  Lines      160642   160668   +26     
=======================================
+ Hits       131242   131308   +66     
+ Misses      29400    29360   -40     

@carllin carllin requested a review from brooksprumo April 5, 2022 00:29
Copy link
Copy Markdown
Contributor

@brooksprumo brooksprumo left a comment

Choose a reason for hiding this comment

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

This looks good to me. One thought, since we've only seen the panic on v1.10, maybe we shouldn't backport to v1.9?

@carllin carllin removed the v1.9 label Apr 5, 2022
@carllin carllin merged commit 4ea59d8 into solana-labs:master Apr 5, 2022
mergify Bot pushed a commit that referenced this pull request Apr 5, 2022
mergify Bot added a commit that referenced this pull request Apr 5, 2022
(cherry picked from commit 4ea59d8)

Co-authored-by: carllin <carl@solana.com>
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.

2 participants