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

Add structure to collect and coalesce vote packets #27558

Merged
merged 3 commits into from
Sep 14, 2022

Conversation

AshwinSekar
Copy link
Contributor

@AshwinSekar AshwinSekar commented Sep 1, 2022

Will be used in banking stage to throw out extraneous vote packets
before processing

Problem

Picks up from #26722 .

Summary of Changes

Creates a thread safe structure to store vote packets - only latest by slot # for each validator.

Fixes #

@AshwinSekar AshwinSekar requested a review from carllin September 1, 2022 19:52
@AshwinSekar AshwinSekar force-pushed the latest-unprocessed-votes branch from bd17a83 to 06d4f46 Compare September 9, 2022 19:03
@AshwinSekar AshwinSekar force-pushed the latest-unprocessed-votes branch from f938608 to 2f9f132 Compare September 10, 2022 16:42
@AshwinSekar AshwinSekar requested a review from carllin September 12, 2022 09:15
Comment on lines +193 to +197
self.latest_votes_per_pubkey
.read()
.unwrap()
.get(&pubkey)
.cloned()
Copy link
Contributor

Choose a reason for hiding this comment

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

Just double checking, there's no danger of this Arc<RwLock> for this same pubkey being replaced with a new one right?

i.e. something like

Read A
A gets removed from the hashmap
A' gets re-inserted in to the hashmap
Some other thread gets A'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think that's possible, there's no operations to remove an entry, only clear the packet it contains. Also we only add new pubkeys if they don't currently exist in the map


#[derive(Debug, Default)]
pub struct LatestUnprocessedVotes {
latest_votes_per_pubkey: RwLock<HashMap<Pubkey, Arc<RwLock<LatestValidatorVotePacket>>>>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a size limit on the number of pubkeys this can hold before we start dropping?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not currently, but I can add a capacity - which pubkeys would we decide to drop though?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will address it as part of #27409

Copy link
Contributor

Choose a reason for hiding this comment

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

kk just gotta make sure if we add removal of pubkeys this doesn't become an issue: #27558 (comment)

@AshwinSekar AshwinSekar force-pushed the latest-unprocessed-votes branch from 2f9f132 to 6502f45 Compare September 14, 2022 02:02
@AshwinSekar AshwinSekar requested a review from carllin September 14, 2022 02:16
@AshwinSekar AshwinSekar merged commit c74df83 into solana-labs:master Sep 14, 2022
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.

2 participants