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

in shrink, get stores prior to shrink starting #33194

Merged
merged 1 commit into from
Sep 8, 2023

Conversation

jeffwashington
Copy link
Contributor

@jeffwashington jeffwashington commented Sep 8, 2023

Problem

Fix assert on canaries.
Parallel iter can start a shrink, then we ask for a storage from a slot, expecting shrink to not be allowed to be active.

panicked at 'assertion failed: self.no_shrink_in_progress()', accounts-db/src/account_storage.rs:78:9)

Introduced here:
ShrinkCandidates only holds slot (#33173)

Summary of Changes

Get all stores for shrinks prior to starting shrinking.

Fixes #

@jeffwashington jeffwashington marked this pull request as ready for review September 8, 2023 14:12
Copy link
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.

lgtm

@codecov
Copy link

codecov bot commented Sep 8, 2023

Codecov Report

Merging #33194 (09f67e9) into master (b588beb) will decrease coverage by 0.1%.
The diff coverage is 62.5%.

@@            Coverage Diff            @@
##           master   #33194     +/-   ##
=========================================
- Coverage    82.1%    82.1%   -0.1%     
=========================================
  Files         786      786             
  Lines      211590   211596      +6     
=========================================
- Hits       173835   173818     -17     
- Misses      37755    37778     +23     

📢 Have feedback on the report? Share it here.

@jeffwashington jeffwashington merged commit dc6b1eb into solana-labs:master Sep 8, 2023
16 checks passed
@ilya-bobyr
Copy link
Contributor

ilya-bobyr commented Sep 9, 2023

I had this assertion triggered on the v1.16 branch.
So this should probably be backported?


Update: Sorry, I was actually running a master build %)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants