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

BankingStage: Gossip votes held #2183

Closed
apfitzge opened this issue Jul 18, 2024 · 2 comments · Fixed by #2509
Closed

BankingStage: Gossip votes held #2183

apfitzge opened this issue Jul 18, 2024 · 2 comments · Fixed by #2509
Assignees

Comments

@apfitzge
Copy link

Problem

Migrating issue from solana-labs#34973

  • Gossip votes are held in ClusterInfoVoteListener until we become leader.
  • This seems unneccessary now since both voting threads insert into a shared data-structure which only keeps the latest vote.
  • Holding the votes can lead to misleading metrics since tpu votes are a sustained throughput, whereas gossip votes come in nearly all at once

Proposed Solution

  • ClusterInfoVoteListener just verifies the votes and sends to BankingStage for it to handle
@apfitzge
Copy link
Author

@carllin @AshwinSekar - I migrated this issue. My understanding from previous issue is it is not necessary for us to continue doing this holding.

@carllin if that is not the case, please let us know.

@AshwinSekar
Copy link

AshwinSekar commented Jul 18, 2024

Confirmed with carl in April that votes do not need to be held in ClusterInfoVoteListener with the caveat that banking stage only drains the votes for slots on the working_bank's fork and leaves the remaining untouched.

The benefit of holding votes in ClusterInfoVoteListener is that if the leader decides to build it's first block off of fork1 and it's second block off of fork2, gossip votes for fork2 will still make it into the second block, even if they are filtered out for the first block.

Essentially we want to move this check:

// Filter out votes on the wrong fork (or too old to be)
// on this fork

Into here:

/// Drains all votes yet to be processed sorted by a weighted random ordering by stake
pub fn drain_unprocessed(&self, bank: Arc<Bank>) -> Vec<Arc<ImmutableDeserializedPacket>> {

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 a pull request may close this issue.

2 participants