protocols/gossipsub: Improve bandwidth #2327
Conversation
| if let Some((peer_score, ..)) = &mut self.peer_score { | ||
| peer_score.duplicated_message(propagation_source, &msg_id, &raw_message.topic); | ||
| // Report the duplicate | ||
| if self.message_is_valid(&msg_id, &mut raw_message, propagation_source) { |
There was a problem hiding this comment.
Why just report if the message is valid now?
There was a problem hiding this comment.
I think this was a bug in the previous code. In principle we shouldn't have invalid messages in the duplicates, and if we do, we penalize them but we don't register it as a duplicate (for scoring). The duplicate would benefit the peer, but we dont want to improve scores for peers that are sending us invalid messages.
Co-authored-by: Divma <26765164+divagant-martian@users.noreply.github.com>
Co-authored-by: Divma <26765164+divagant-martian@users.noreply.github.com>
|
Yep. Just want to run a bit more and run with the new metrics to quantify the gains here. Will report back once tests are complete |
Co-authored-by: Max Inden <mail@max-inden.de>
Co-authored-by: Max Inden <mail@max-inden.de>
|
@mxinden - Have confirmed this has a net positive effect and works as expected. |
protocols/gossipsub/CHANGELOG.md
Outdated
| @@ -4,7 +4,11 @@ | |||
|
|
|||
| - Migrate to Rust edition 2021 (see [PR 2339]). | |||
|
|
|||
| - Improve bandwidth performance by tracking IWANTs and reducing duplicate sends | |||
| (see PR 2327). | |||
There was a problem hiding this comment.
| (see PR 2327). | |
| (see [PR 2327]). |
protocols/gossipsub/src/behaviour.rs
Outdated
| let message_ids = iwant_ids_vec | ||
| .into_iter() | ||
| .map(|id| { | ||
| // Add all messages to the pending list | ||
| self.pending_iwant_msgs.insert(id.clone()); | ||
| id.clone() | ||
| }) | ||
| .collect::<Vec<_>>(); |
There was a problem hiding this comment.
Nit pick: A side effect (self.pending_iwant_msgs) in a map is a bit surprising to me. Would an additional imperative for msg in message_ids be ok as well? I would guess this does not have any impact on the performance.
There was a problem hiding this comment.
I've re-written it with a for loop. Just makes the message_ids vec mutable. Just wanted to do the clone and the copy in the one iteration :p
|
Hopefully this guy is ready now :) |
mxinden
changed the title
This PR adds some bandwidth improvements to gossipsub.
After a bit of inspection on live networks a number of improvements have been made that can help reduce unnecessary bandwidth on gossipsub networks. This PR introduces the following: