-
Notifications
You must be signed in to change notification settings - Fork 4.6k
reduce locking in propagated check for VoteStateUpdate #33997
reduce locking in propagated check for VoteStateUpdate #33997
Conversation
302d55b
to
59bf14f
Compare
Codecov Report
@@ Coverage Diff @@
## master #33997 +/- ##
=======================================
Coverage 81.9% 81.9%
=======================================
Files 811 811
Lines 219424 219497 +73
=======================================
+ Hits 179761 179830 +69
- Misses 39663 39667 +4 |
@@ -733,6 +752,8 @@ impl ClusterInfoVoteListener { | |||
.or_insert(is_gossip_vote); | |||
} | |||
|
|||
*latest_vote_slot = max(*latest_vote_slot, last_vote_slot); | |||
|
|||
if is_new_vote { | |||
subscriptions.notify_vote(*vote_pubkey, vote, vote_transaction_signature); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to move this RPC notification before the new check as well so that we still get notified of last voted slots X
in votes where X < latest_vote_slot
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This RPC notification is outside the loop, the behavior should remain unchanged. If the last vote was for a new slot as decided by the vote tracker, it will still notify.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah dam the github formatting messed me up, thanks
59bf14f
to
29a33f9
Compare
Problem
logic is not updated for
VoteStateUpdate
, end up checking all 31 slots every time which grabs the lock shared with replaySummary of Changes
filter out seen slots, no need to be precise and check every single slot, can filter by latest slot because propagated check rolls up to parent anyway.
potentially helps #33017