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

Gossip Votes Held Before BankingStage #34973

Open
apfitzge opened this issue Jan 26, 2024 · 2 comments
Open

Gossip Votes Held Before BankingStage #34973

apfitzge opened this issue Jan 26, 2024 · 2 comments

Comments

@apfitzge
Copy link
Contributor

apfitzge commented Jan 26, 2024

Problem

  • 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
Contributor Author

@AshwinSekar Am I way off here? This appears to be no longer necessary since your changes to the vote threads have now landed.

@AshwinSekar
Copy link
Contributor

AshwinSekar commented Jan 26, 2024

It looks like the buffer to hold 1k votes was introduced in #20873 in order to solve the following issues:

  1. ClusterInfoVoteListener has too short of a window (20 slots) for sending votes to BankingStage
  2. ClusterInfoVoteListener sends all votes to BankingStage, even outdated/ones on wrong fork
  3. ClusterInfoVoteListener may send votes out of order

With the new vote format enabled last year VoteStateUpdate I believe these issues are no longer a concern as only the latest vote matters, so there is no reason to hold gossip votes at all in ClusterInfoVoteListener. cc: @carllin since you wrote the initial change

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

No branches or pull requests

2 participants