Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

protocols/gossipsub: Allow score buckets to be set by the user and/or adjusted from the scoring thresholds #2595

Merged
merged 5 commits into from
Apr 8, 2022
Merged

protocols/gossipsub: Allow score buckets to be set by the user and/or adjusted from the scoring thresholds #2595

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6ba8599
allow score buckets to be set by the user and/or adjusted from the s…
divagant-martian Mar 28, 2022
eb2bb2c
add some docs
divagant-martian Mar 28, 2022
bc9f8ee
initial CHANGELOG
divagant-martian Mar 28, 2022
c55d173
Update protocols/gossipsub/CHANGELOG.md
mxinden Apr 8, 2022
c886a1d
Merge branch 'master' into better-default-hist-buckets
mxinden Apr 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions protocols/gossipsub/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@

- Fix gossipsub metric (see [PR 2558]).

- Allow the user to set the buckets for the score histogram, and to adjust them from the score thresholds. See [PR 2595].

[PR 2558]: https://github.com/libp2p/rust-libp2p/pull/2558
[PR 2595]: https://github.com/libp2p/rust-libp2p/pull/2595

# 0.36.0 [2022-02-22]

Expand Down
53 changes: 37 additions & 16 deletions protocols/gossipsub/src/metrics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,48 @@ pub struct Config {
/// determined by users on the network. This limit permits a fixed amount of topics to allow,
/// in-addition to the mesh topics.
pub max_never_subscribed_topics: usize,
/// Buckets used for the score histograms.
pub score_buckets: Vec<f64>,
}

impl Config {
/// Create buckets for the score histograms based on score thresholds.
pub fn buckets_using_scoring_thresholds(&mut self, params: &crate::PeerScoreThresholds) {
self.score_buckets = vec![
params.graylist_threshold,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure how valuable values calculated based on the bucket values are if the buckets themselves don't follow some consistent distribution. Would it not make sense to take the smallest of all the values and do an exponential distribution to the max value (100.0)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You probably know best from your current measurements. How well does the histogram mirror real world values?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The value here is to understand how often nodes are treated in a special way because they have passed the thresholds, more than to understand the distribution of scores itself. In a way is more a measure of network health/score param behaviour than a measure of the scores

params.publish_threshold,
params.gossip_threshold,
params.gossip_threshold / 2.0,
params.gossip_threshold / 4.0,
0.0,
1.0,
10.0,
100.0,
];
}
}

impl Default for Config {
fn default() -> Self {
// Some sensible defaults
let gossip_threshold = -4000.0;
let publish_threshold = -8000.0;
let graylist_threshold = -16000.0;
let score_buckets: Vec<f64> = vec![
graylist_threshold,
publish_threshold,
gossip_threshold,
gossip_threshold / 2.0,
gossip_threshold / 4.0,
0.0,
1.0,
10.0,
100.0,
];
Config {
max_topics: DEFAULT_MAX_TOPICS,
max_never_subscribed_topics: DEFAULT_MAX_NEVER_SUBSCRIBED_TOPICS,
score_buckets,
}
}
}
Expand Down Expand Up @@ -147,6 +182,7 @@ impl Metrics {
let Config {
max_topics,
max_never_subscribed_topics,
score_buckets,
} = config;

macro_rules! register_family {
Expand Down Expand Up @@ -224,24 +260,9 @@ impl Metrics {
"topic_msg_recv_bytes",
"Bytes received from gossip messages for each topic"
);
// TODO: Update default variables once a builder pattern is used.
let gossip_threshold = -4000.0;
let publish_threshold = -8000.0;
let greylist_threshold = -16000.0;
let histogram_buckets: Vec<f64> = vec![
greylist_threshold,
publish_threshold,
gossip_threshold,
gossip_threshold / 2.0,
gossip_threshold / 4.0,
0.0,
1.0,
10.0,
100.0,
];

let hist_builder = HistBuilder {
buckets: histogram_buckets,
buckets: score_buckets,
};

let score_per_mesh: Family<_, _, HistBuilder> = Family::new_with_constructor(hist_builder);
Expand Down