v2.2: stop unstaked nodes from pushing EpochSlots into the cluster (backport of #5141) #5286
Conversation
![mergify[bot]](https://avatars.githubusercontent.com/in/10562?s=60&v=4){kind=small}
* stop unstaked nodes from pushing EpochSlots into the cluster * reload own stake on every epoch in case I become staked * use epoch specs to reduce contention for bank forks --------- Co-authored-by: Alex Pyattaev <[email protected]> Big thanks to Behzad for code suggestions. (cherry picked from commit 145d562)
|
According to @kobi-lizard epoch slots bandwidth cost over 1M$ daily on MNB. |
| break; | ||
| } | ||
| let mut epoch_specs = EpochSpecs::from(bank_forks.clone()); | ||
| while !exit.load(Ordering::Relaxed) { |
There was a problem hiding this comment.
in the future, please avoid superfluous refactors like this for changes that are intended for backport. they can go into master as a followup
| .current_epoch_staked_nodes() | ||
| .get(&node_id) | ||
| .cloned() | ||
| .unwrap_or_default(); |
There was a problem hiding this comment.
avoid smearing over potential unexpected error scenarios with dubious generics (Default is a bug). this would have been more obvious as
let maybe_my_stake = epoch_specs
.current_epoch_staked_nodes()
.get(&node_id)
.cloned();
match maybe_my_stake {
Some(my_stake) if my_stake > 0 => {
...process
}
() => {
...drain
}
}There was a problem hiding this comment.
Arguably, unwrap_or(0) would work even better here. Agree that default can do surprising things. Should we add recommendations to avoid using default() where reasonable?
| @@ -100,12 +98,25 @@ impl ClusterSlotsService { | |||
| lowest_slot_elapsed.stop(); | |||
| let mut process_cluster_slots_updates_elapsed = | |||
| Measure::start("process_cluster_slots_updates_elapsed"); | |||
|
|
|||
|
@alessandrod are you willing to sme this one? |
Problem
Summary of Changes
Fixes #
Partially #5034
This is an automatic backport of pull request #5141 done by [Mergify](https://mergify.com).