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

reduce contention on startup index generation #31006

Merged
merged 1 commit into from
Mar 31, 2023

Conversation

jeffwashington
Copy link
Contributor

@jeffwashington jeffwashington commented Mar 31, 2023

Problem

See #30711

As part of #30736, we stopped writing duplicate entries to the disk index and held them for later.
The issue is the duplicate code shared a mutex with the insertion code.
This meant that insertion into the index from the 432k append vecs was blocking while a disk bucket was flushing. This slowed down overall index generation time which slows down startup time.

Summary of Changes

separate the 2 parts of startup index generation into their own mutexes. This does not affect steady state operation at all.

Fixes #

@codecov
Copy link

codecov bot commented Mar 31, 2023

Codecov Report

Merging #31006 (e02897c) into master (3f6c33d) will increase coverage by 0.0%.
The diff coverage is 100.0%.

@@           Coverage Diff           @@
##           master   #31006   +/-   ##
=======================================
  Coverage    81.5%    81.5%           
=======================================
  Files         728      728           
  Lines      205479   205483    +4     
=======================================
+ Hits       167560   167565    +5     
+ Misses      37919    37918    -1     

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

@@ -1087,13 +1093,16 @@ impl<T: IndexValue, U: DiskIndexValue + From<T> + Into<T>> InMemAccountsIndex<T,
/// These were collected for this bin when we did batch inserts in the bg flush threads.
/// Insert these into the in-mem index, then return the duplicate (Slot, Pubkey)
pub(crate) fn populate_and_retrieve_duplicate_keys_from_startup(&self) -> Vec<(Slot, Pubkey)> {
let mut write = self.startup_info.lock().unwrap();
let inserts = self.startup_info.insert.lock().unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Could collapse this into the assert! as well, right?

assert!(self.startup_info.insert.lock().unwrap().is_empty());

(Does not need to go into this PR)

@jeffwashington jeffwashington merged commit 9600643 into solana-labs:master Mar 31, 2023
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.

2 participants