feat(census): add peer scoring #1579
Conversation
|
|
||
| weighted_peers | ||
| .choose_multiple_weighted(&mut thread_rng(), self.limit, |(_peer, weight)| *weight) | ||
| .expect("choosing random sample shouldn't fail") |
There was a problem hiding this comment.
Does this fail if there are zero peers?
There was a problem hiding this comment.
No, it doesn't.
That can fail if weight is negative (api allows any type that can be converted to f64), which can't happen in our case because we use u32.
Weight 0 would also be picked, but only if there isn't enough weights that are non-zero, which is what we want.
One thing that we don't differentiate at the moment is that weight can be 0 if:
- content doesn't fall within radius
- peer has very negative weight
It might be better to never select peer whose radius doesn't include content. But I left that for future optimization
looks good| #[allow(dead_code)] | ||
| pub content_value_size: usize, | ||
| #[allow(dead_code)] | ||
| pub duration: Duration, |
There was a problem hiding this comment.
Are there plans to use these in the future?
There was a problem hiding this comment.
Yes.
Idea is to measure bandwidth and prefer peers that are faster than average.
But I didn't want to complicate initial work, and I don't think it works well with current implementation of "Weight" because peers that are working would quickly increase their weight to maximum and this wouldn't make a difference.
What was wrong?
Census doesn't use information about previous successful/failed requests when selecting peers (instead, it selects them randomly).
Previous PRs made this information available.
Part of #1501
How was it fixed?
Added
Weighttrait that can be used to calculate peers weights, andPeerSelectorthat picks peers based on weights.Current implementation of
Weightis based on Piper's suggestion:In the future, we can implement multiple versions of
Weight, make them configurable via cli flags, and compare them in practice.