removes epoch_authorized_voters from VoteTracker #22192
Conversation
behzadnouri
force-pushed
the
rm-filter-gossip-votes-squashed
branch
from
e85e4b8
to
5e21175
Compare
behzadnouri
force-pushed
the
rm-filter-gossip-votes-squashed
branch
from
5e21175
to
44de4ff
Compare
Codecov Report
@@ Coverage Diff @@
## master #22192 +/- ##
=========================================
- Coverage 81.0% 81.0% -0.1%
=========================================
Files 523 523
Lines 146497 146337 -160
=========================================
- Hits 118739 118608 -131
+ Misses 27758 27729 -29 |
behzadnouri
force-pushed
the
rm-filter-gossip-votes-squashed
branch
from
44de4ff
to
8bd5717
Compare
mergify
bot
dismissed
t-nelson’s stale review
Pull request has been modified.
|
|
||
| slot_tracker.unwrap() | ||
| fn get_or_insert_slot_tracker(&self, slot: Slot) -> Arc<RwLock<SlotVoteTracker>> { | ||
| let mut slot_vote_trackers = self.slot_vote_trackers.write().unwrap(); |
There was a problem hiding this comment.
I would prefer to keep the original locking structure to avoid any perf differences (read -> write only in case the slot doesn't exist).
This is because in the original code each slot should only take the top level slot_vote_trackers lock once on creation, whereas in this new code, the top level write lock will be taken for every validator, for every slot. This may affect replay's propagation check which is also reading this lock for specific slots
There was a problem hiding this comment.
This was changed because the original code has a race condition.
Between the time the read-lock is released and the write-lock is obtained, the object can be modified by a different thread. So always need to check the condition again under the write-lock.
I added an additional look-up under a read-lock, but note that this new code still avoids the race condition above.
There was a problem hiding this comment.
ah yeah this code was written under the assumption the vote listener thread would be the only one modifying this structure, kind of like the cluster_slots_service as well. This does look much cleaner though, thanks!
solana-labs#22169 verifies authorized-voter early on in vote-listener pipeline; and so VoteTracker no longer needs to maintain and check for epoch authorized voters.
behzadnouri
force-pushed
the
rm-filter-gossip-votes-squashed
branch
from
8bd5717
to
1eeaff5
Compare
mergify
bot
dismissed
t-nelson’s stale review
Pull request has been modified.
…22248) * removes epoch_authorized_voters from VoteTracker (#22192) #22169 verifies authorized-voter early on in vote-listener pipeline; and so VoteTracker no longer needs to maintain and check for epoch authorized voters. (cherry picked from commit 69d71f8) # Conflicts: # core/src/cluster_info_vote_listener.rs * removes backport merge conflicts Co-authored-by: behzad nouri <behzadnouri@gmail.com>
…22250) * removes epoch_authorized_voters from VoteTracker (#22192) #22169 verifies authorized-voter early on in vote-listener pipeline; and so VoteTracker no longer needs to maintain and check for epoch authorized voters. (cherry picked from commit 69d71f8) # Conflicts: # core/src/cluster_info_vote_listener.rs * removes backport merge conflicts Co-authored-by: behzad nouri <behzadnouri@gmail.com>
Problem
#22169
verifies authorized-voter early on in vote-listener; so VoteTracker no
longer needs to maintain and check for epoch authorized voters.
Summary of Changes
removed epoch_authorized_voters from VoteTracker