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

Bank drop service #21322

Merged
merged 1 commit into from
Nov 19, 2021
Merged

Bank drop service #21322

merged 1 commit into from
Nov 19, 2021

Conversation

sakridge
Copy link
Contributor

Problem

Dropping banks can be expensive and it's bad to stall the replay stage loop.

Summary of Changes

Drop banks in another thread.

Fixes #

@sakridge sakridge added the CI Pull Request is ready to enter CI label Nov 17, 2021
@solana-grimes solana-grimes removed the CI Pull Request is ready to enter CI label Nov 17, 2021
@codecov
Copy link

codecov bot commented Nov 18, 2021

Codecov Report

Merging #21322 (9265214) into master (f707dac) will decrease coverage by 0.0%.
The diff coverage is 80.4%.

@@            Coverage Diff            @@
##           master   #21322     +/-   ##
=========================================
- Coverage    81.5%    81.5%   -0.1%     
=========================================
  Files         500      501      +1     
  Lines      140511   140590     +79     
=========================================
+ Hits       114655   114664      +9     
- Misses      25856    25926     +70     

@sakridge
Copy link
Contributor Author

1.8.5 node:
7np41-set-root

Node with this fix:
2yhp-set-root

let mut dropped_banks_time = Measure::start("drop_banks");
drop(banks);
dropped_banks_time.stop();
if dropped_banks_time.as_ms() > 10 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe worth logging a datapoint for each individual bank for each slot in case it's something bank specific?

Also not a big deal, but I think it's possible some other thread has an outstanding reference to some Arc root bank even after it was removed in the replay path (something like, RPC or the vote listener) but this at least unblocks replay.

Copy link
Contributor

@CriesofCarrots CriesofCarrots left a comment

Choose a reason for hiding this comment

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

I've been thinking along the same lines as Carl, but unblocking replay is still good.
Thanks for pulling the metrics

@sakridge sakridge merged commit 0bda0c3 into solana-labs:master Nov 19, 2021
@sakridge sakridge deleted the bank-drop-service branch November 19, 2021 16:20
@sakridge sakridge added the v1.8 label Nov 19, 2021
mergify bot pushed a commit that referenced this pull request Nov 19, 2021
(cherry picked from commit 0bda0c3)

# Conflicts:
#	core/src/lib.rs
#	core/src/replay_stage.rs
#	core/src/tvu.rs
#	core/src/vote_simulator.rs
sakridge added a commit that referenced this pull request Nov 19, 2021
(cherry picked from commit 0bda0c3)
sakridge added a commit that referenced this pull request Nov 19, 2021
(cherry picked from commit 0bda0c3)
jeffwashington pushed a commit to jeffwashington/solana that referenced this pull request Nov 19, 2021
sakridge added a commit that referenced this pull request Nov 19, 2021
(cherry picked from commit 0bda0c3)
sakridge added a commit that referenced this pull request Nov 19, 2021
(cherry picked from commit 0bda0c3)
dankelleher pushed a commit to identity-com/solana that referenced this pull request Nov 24, 2021
frits-metalogix added a commit to identity-com/solana that referenced this pull request Nov 24, 2021
mergify bot added a commit that referenced this pull request Nov 24, 2021
(cherry picked from commit 0bda0c3)

Co-authored-by: sakridge <sakridge@gmail.com>
t-nelson added a commit to t-nelson/solana that referenced this pull request Dec 13, 2021
tao-stones pushed a commit that referenced this pull request Dec 13, 2021
CriesofCarrots pushed a commit to CriesofCarrots/solana that referenced this pull request Dec 16, 2021
CriesofCarrots pushed a commit that referenced this pull request Dec 17, 2021
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.

4 participants