Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Update to libp2p-0.23. #6870

Merged
11 commits merged into from
Aug 14, 2020
Merged

Update to libp2p-0.23. #6870

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
d064271
Update to libp2p-0.23.
Aug 7, 2020
c7ff520
Tweak dependencies for wasm32 compilation.
Aug 11, 2020
a941680
Simplify dependencies.
Aug 11, 2020
4ce6c02
Simplify.
Aug 11, 2020
f4c3b76
Cleanup
Aug 11, 2020
d6f412f
Re-add lost dev dependency.
Aug 11, 2020
6c96875
Avoid division by zero.
Aug 11, 2020
03d3f75
Remove redundant metric.
Aug 11, 2020
df43ead
Enable sending of noise legacy handshakes.
Aug 11, 2020
3ea8dd9
Add comment about monotonic gauge.
Aug 12, 2020
4ff525e
CI
Aug 13, 2020
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
15 changes: 11 additions & 4 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion bin/node/browser-testing/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ license = "Apache-2.0"

[dependencies]
futures-timer = "3.0.2"
libp2p = { version = "0.22.0", default-features = false }
libp2p = { version = "0.23.0", default-features = false }
jsonrpc-core = "14.2.0"
serde = "1.0.106"
serde_json = "1.0.48"
Expand Down
2 changes: 1 addition & 1 deletion bin/utils/subkey/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ derive_more = { version = "0.99.2" }
sc-rpc = { version = "2.0.0-rc5", path = "../../../client/rpc" }
jsonrpc-core-client = { version = "14.2.0", features = ["http"] }
hyper = "0.12.35"
libp2p = { version = "0.22.0", default-features = false }
libp2p = { version = "0.23.0", default-features = false }
serde_json = "1.0"

[features]
Expand Down
2 changes: 1 addition & 1 deletion client/authority-discovery/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ codec = { package = "parity-scale-codec", default-features = false, version = "1
derive_more = "0.99.2"
futures = "0.3.4"
futures-timer = "3.0.1"
libp2p = { version = "0.22.0", default-features = false, features = ["kad"] }
libp2p = { version = "0.23.0", default-features = false, features = ["kad"] }
log = "0.4.8"
prometheus-endpoint = { package = "substrate-prometheus-endpoint", path = "../../utils/prometheus", version = "0.8.0-rc5"}
prost = "0.6.1"
Expand Down
32 changes: 27 additions & 5 deletions client/informant/src/display.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ pub struct InformantDisplay<B: BlockT> {
last_number: Option<NumberFor<B>>,
/// The last time `display` or `new` has been called.
last_update: Instant,
/// The last seen total of bytes received.
last_total_bytes_inbound: u64,
/// The last seen total of bytes sent.
last_total_bytes_outbound: u64,
/// The format to print output in.
format: OutputFormat,
}
Expand All @@ -55,6 +59,8 @@ impl<B: BlockT> InformantDisplay<B> {
InformantDisplay {
last_number: None,
last_update: Instant::now(),
last_total_bytes_inbound: 0,
last_total_bytes_outbound: 0,
format,
}
}
Expand All @@ -66,9 +72,25 @@ impl<B: BlockT> InformantDisplay<B> {
let finalized_number = info.chain.finalized_number;
let num_connected_peers = net_status.num_connected_peers;
let speed = speed::<B>(best_number, self.last_number, self.last_update);
self.last_update = Instant::now();
let total_bytes_inbound = net_status.total_bytes_inbound;
let total_bytes_outbound = net_status.total_bytes_outbound;

let now = Instant::now();
let elapsed = (now - self.last_update).as_secs();
self.last_update = now;
self.last_number = Some(best_number);

let diff_bytes_inbound = total_bytes_inbound - self.last_total_bytes_inbound;
let diff_bytes_outbound = total_bytes_outbound - self.last_total_bytes_outbound;
let (avg_bytes_per_sec_inbound, avg_bytes_per_sec_outbound) =
if elapsed > 0 {
self.last_total_bytes_inbound = total_bytes_inbound;
self.last_total_bytes_outbound = total_bytes_outbound;
(diff_bytes_inbound / elapsed, diff_bytes_outbound / elapsed)
} else {
(diff_bytes_inbound, diff_bytes_outbound)
};

let (level, status, target) = match (net_status.sync_state, net_status.best_seen_block) {
(SyncState::Idle, _) => ("💤", "Idle".into(), "".into()),
(SyncState::Downloading, None) => ("⚙️ ", format!("Preparing{}", speed), "".into()),
Expand All @@ -92,8 +114,8 @@ impl<B: BlockT> InformantDisplay<B> {
best_hash,
Colour::White.bold().paint(format!("{}", finalized_number)),
info.chain.finalized_hash,
Colour::Green.paint(format!("⬇ {}", TransferRateFormat(net_status.average_download_per_sec))),
Colour::Red.paint(format!("⬆ {}", TransferRateFormat(net_status.average_upload_per_sec))),
Colour::Green.paint(format!("⬇ {}", TransferRateFormat(avg_bytes_per_sec_inbound))),
Colour::Red.paint(format!("⬆ {}", TransferRateFormat(avg_bytes_per_sec_outbound))),
)
} else {
info!(
Expand All @@ -108,8 +130,8 @@ impl<B: BlockT> InformantDisplay<B> {
best_hash,
finalized_number,
info.chain.finalized_hash,
TransferRateFormat(net_status.average_download_per_sec),
TransferRateFormat(net_status.average_upload_per_sec),
TransferRateFormat(avg_bytes_per_sec_inbound),
TransferRateFormat(avg_bytes_per_sec_outbound),
)
}
}
Expand Down
2 changes: 1 addition & 1 deletion client/network-gossip/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ targets = ["x86_64-unknown-linux-gnu"]
[dependencies]
futures = "0.3.4"
futures-timer = "3.0.1"
libp2p = { version = "0.22.0", default-features = false }
libp2p = { version = "0.23.0", default-features = false }
log = "0.4.8"
lru = "0.4.3"
sc-network = { version = "0.8.0-rc5", path = "../network" }
Expand Down
6 changes: 3 additions & 3 deletions client/network/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ erased-serde = "0.3.9"
fnv = "1.0.6"
fork-tree = { version = "2.0.0-rc5", path = "../../utils/fork-tree" }
futures = "0.3.4"
futures-timer = "3.0.1"
futures-timer = "3.0.2"
futures_codec = "0.4.0"
hex = "0.4.0"
ip_network = "0.3.4"
Expand Down Expand Up @@ -61,15 +61,15 @@ wasm-timer = "0.2"
zeroize = "1.0.0"

[dependencies.libp2p]
version = "0.22.0"
version = "0.23.0"
default-features = false
features = ["identify", "kad", "mdns", "mplex", "noise", "ping", "tcp-async-std", "websocket", "yamux"]

[dev-dependencies]
async-std = "1.6.2"
assert_matches = "1.3"
env_logger = "0.7.0"
libp2p = { version = "0.22.0", default-features = false, features = ["secio"] }
libp2p = { version = "0.23.0", default-features = false, features = ["secio"] }
quickcheck = "0.9.0"
rand = "0.7.2"
sp-keyring = { version = "2.0.0-rc5", path = "../../primitives/keyring" }
Expand Down
8 changes: 4 additions & 4 deletions client/network/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,8 +309,8 @@ pub struct NetworkStatus<B: BlockT> {
pub num_connected_peers: usize,
/// Total number of active peers.
pub num_active_peers: usize,
/// Downloaded bytes per second averaged over the past few seconds.
pub average_download_per_sec: u64,
/// Uploaded bytes per second averaged over the past few seconds.
pub average_upload_per_sec: u64,
/// The total number of bytes received.
pub total_bytes_inbound: u64,
/// The total number of bytes sent.
pub total_bytes_outbound: u64,
}
8 changes: 4 additions & 4 deletions client/network/src/network_state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@ pub struct NetworkState {
pub connected_peers: HashMap<String, Peer>,
/// List of node that we know of but that we're not connected to.
pub not_connected_peers: HashMap<String, NotConnectedPeer>,
/// Downloaded bytes per second averaged over the past few seconds.
pub average_download_per_sec: u64,
/// Uploaded bytes per second averaged over the past few seconds.
pub average_upload_per_sec: u64,
/// The total number of bytes received.
pub total_bytes_inbound: u64,
/// The total number of bytes sent.
pub total_bytes_outbound: u64,
/// State of the peerset manager.
pub peerset: serde_json::Value,
}
Expand Down
30 changes: 16 additions & 14 deletions client/network/src/service.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,14 +387,14 @@ impl<B: BlockT + 'static, H: ExHashT> NetworkWorker<B, H> {
})
}

/// Returns the downloaded bytes per second averaged over the past few seconds.
pub fn average_download_per_sec(&self) -> u64 {
self.service.bandwidth.average_download_per_sec()
/// Returns the total number of bytes received so far.
pub fn total_bytes_inbound(&self) -> u64 {
self.service.bandwidth.total_inbound()
}

/// Returns the uploaded bytes per second averaged over the past few seconds.
pub fn average_upload_per_sec(&self) -> u64 {
self.service.bandwidth.average_upload_per_sec()
/// Returns the total number of bytes sent so far.
pub fn total_bytes_outbound(&self) -> u64 {
self.service.bandwidth.total_outbound()
}

/// Returns the number of peers we're connected to.
Expand Down Expand Up @@ -526,8 +526,8 @@ impl<B: BlockT + 'static, H: ExHashT> NetworkWorker<B, H> {
peer_id: Swarm::<B, H>::local_peer_id(&swarm).to_base58(),
listened_addresses: Swarm::<B, H>::listeners(&swarm).cloned().collect(),
external_addresses: Swarm::<B, H>::external_addresses(&swarm).cloned().collect(),
average_download_per_sec: self.service.bandwidth.average_download_per_sec(),
average_upload_per_sec: self.service.bandwidth.average_upload_per_sec(),
total_bytes_inbound: self.service.bandwidth.total_inbound(),
total_bytes_outbound: self.service.bandwidth.total_outbound(),
connected_peers,
not_connected_peers,
peerset: swarm.user_protocol_mut().peerset_debug_info(),
Expand Down Expand Up @@ -1133,7 +1133,9 @@ struct Metrics {
kbuckets_num_nodes: GaugeVec<U64>,
listeners_local_addresses: Gauge<U64>,
listeners_errors_total: Counter<U64>,
network_per_sec_bytes: GaugeVec<U64>,
// Note: `network_bytes_total` is a monotonic gauge obtained by
// sampling an existing counter.
network_bytes_total: GaugeVec<U64>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
network_bytes_total: GaugeVec<U64>,
network_bytes_total: CounterVec<U64>,

Gauge is for metrics that go up and down, and Counter is for metrics that only ever increase over time except for restarts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe I'm mistaken, but from the prometheus crate docs, counters have essentially inc() and inc_by(). Gauges can be set, which is what we need to do for these totals, i.e. these are counter values from existing counters (the running totals) reported as a gauge over time, so this is a monotonic gauge. I'm happy to be corrected.

Copy link
Contributor

@tomaka tomaka Aug 11, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I see. The problem is that this is not only a change in the programmatic API of the prometheus library.
Whether a metric is a counter or a gauge is ultimately reported to the Prometheus server running as a separate process, and then (I think?) to Grafana.

Since we know that the value can only every increase, we could use Counter::get to get the current value, then call inc_by.
Doing this would be racy if there were multiple clones of that Counter, but since we only ever access that Counter from a single location, we know that nothing will modify the counter between the call to get and the call to inc_by.

@mxinden Any opinion?

Copy link
Contributor

Choose a reason for hiding this comment

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

The problem of updating a counter from an existing counter is often discussed in the Prometheus ecosystem. In general we suggest to implement a custom collector, see robustperception for general information, tikv/rust-prometheus#303 specifically for the Rust client and my comment at the very end tikv/rust-prometheus#303 (comment).

The reason why Counter does not implement set is, that it allows newcomers to easily shoot themselfes in the foot and thus it was not deemed worth the additional ergonomic for this advanced use case.

I see 3 ways moving forward:

  • Keep it as a Gauge. This would expose the metric with the TYPE gauge header. As @tomaka mentioned above this is ingested by Prometheus and forwarded to Grafana, but only used for query suggestions, thus not a big issue.

  • Use a Counter. As mentioned by @tomaka this is potentially racy if it ever gets set concurrently.

  • Use a custom impl Collector. Quite verbose.

I would be fine with using a Gauge here for now. I would just ask to add a comment above stating that it is monotonic. In the future we can look into a custom Collector.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mxinden Thanks for your input.

I would be fine with using a Gauge here for now. I would just ask to add a comment above stating that it is monotonic. In the future we can look into a custom Collector.

I added a comment to the code. I don't have a strong preference in any case.

Out of curiosity, if gauges "must go up and down" and counters "must only increase", what do you use for a monotonically decreasing metric? I'm not sure I understand the significance of the discussions of counter vs gauge. It would almost seem more sensible to directly offer and use Atomic for both use-cases, designating it as a "gauge" or "counter" based on the semantic meaning of the metric. I guess I just don't find the distinction between gauge and counter very useful, technically, especially if the counter can "jump", both forward and back to 0.

Copy link
Contributor

Choose a reason for hiding this comment

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

what do you use for a monotonically decreasing metric

So far I have never come across a use case for a monotonically decreasing metric. Can you come up with something that can not also be modeled by inverting a monotonically increasing counter?

I'm not sure I understand the significance of the discussions of counter vs gauge.

The benefit of differentiating the two comes into play only at query (aggregation time). E.g. one can apply the rate function to a counter knowing that the underlying data never breaks monotonicity. I think the best summary is given by robust perception here.

Let me know if the above makes sense @romanb. More than happy to put more effort into a detailed explanation.

Copy link
Contributor Author

@romanb romanb Aug 12, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

So far I have never come across a use case for a monotonically decreasing metric. [..]

A metric for a bounded resource with fixed capacity and its exhaustion over time? I don't know. The restriction not to be able to do so just seems a bit arbitrary.

The benefit of differentiating the two comes into play only at query (aggregation time). E.g. one can apply the rate function to a counter knowing that the underlying data never breaks monotonicity. I think the best summary is given by robust perception here.

Ok, so there are special query functions with specific characteristics for gauges and counters, like rate() (counter) vs deriv() for gauges, both essentially yielding the derivative but the former makes special use of counter properties to better handle things like resets on restarts. Thanks for the pointers @mxinden.

notifications_sizes: HistogramVec,
notifications_streams_closed_total: CounterVec<U64>,
notifications_streams_opened_total: CounterVec<U64>,
Expand Down Expand Up @@ -1250,10 +1252,10 @@ impl Metrics {
"sub_libp2p_listeners_errors_total",
"Total number of non-fatal errors reported by a listener"
)?, registry)?,
network_per_sec_bytes: register(GaugeVec::new(
network_bytes_total: register(GaugeVec::new(
Opts::new(
"sub_libp2p_network_per_sec_bytes",
"Average bandwidth usage per second"
"sub_libp2p_network_bytes_total",
"Total bandwidth usage"
),
&["direction"]
)?, registry)?,
Expand Down Expand Up @@ -1704,8 +1706,8 @@ impl<B: BlockT + 'static, H: ExHashT> Future for NetworkWorker<B, H> {
this.is_major_syncing.store(is_major_syncing, Ordering::Relaxed);

if let Some(metrics) = this.metrics.as_ref() {
metrics.network_per_sec_bytes.with_label_values(&["in"]).set(this.service.bandwidth.average_download_per_sec());
metrics.network_per_sec_bytes.with_label_values(&["out"]).set(this.service.bandwidth.average_upload_per_sec());
metrics.network_bytes_total.with_label_values(&["in"]).set(this.service.bandwidth.total_inbound());
metrics.network_bytes_total.with_label_values(&["out"]).set(this.service.bandwidth.total_outbound());
metrics.is_major_syncing.set(is_major_syncing as u64);
for (proto, num_entries) in this.network_service.num_kbuckets_entries() {
let proto = maybe_utf8_bytes_to_string(proto.as_bytes());
Expand Down
23 changes: 15 additions & 8 deletions client/network/src/transport.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use libp2p::{
};
#[cfg(not(target_os = "unknown"))]
use libp2p::{tcp, dns, websocket};
use std::{io, sync::Arc, time::Duration, usize};
use std::{io, sync::Arc, time::Duration};

pub use self::bandwidth::BandwidthSinks;

Expand All @@ -43,7 +43,11 @@ pub fn build_transport(
memory_only: bool,
wasm_external_transport: Option<wasm_ext::ExtTransport>,
use_yamux_flow_control: bool
) -> (Boxed<(PeerId, StreamMuxerBox), io::Error>, Arc<bandwidth::BandwidthSinks>) {
) -> (Boxed<(PeerId, StreamMuxerBox), io::Error>, Arc<BandwidthSinks>) {
// Legacy noise configurations for backward compatibility.
let mut noise_legacy = noise::LegacyConfig::default();
noise_legacy.send_legacy_handshake = true;

// Build configuration objects for encryption mechanisms.
let noise_config = {
// For more information about these two panics, see in "On the Importance of
Expand All @@ -58,10 +62,12 @@ pub fn build_transport(
once and at initialization, we're taking the bet that the inconvenience of a very \
rare panic here is basically zero");

core::upgrade::SelectUpgrade::new(
noise::NoiseConfig::xx(noise_keypair_spec),
noise::NoiseConfig::ix(noise_keypair_legacy)
)
let mut xx_config = noise::NoiseConfig::xx(noise_keypair_spec);
xx_config.set_legacy_config(noise_legacy.clone());
let mut ix_config = noise::NoiseConfig::ix(noise_keypair_legacy);
ix_config.set_legacy_config(noise_legacy);

core::upgrade::SelectUpgrade::new(xx_config, ix_config)
};

// Build configuration objects for multiplexing mechanisms.
Expand Down Expand Up @@ -104,7 +110,7 @@ pub fn build_transport(
OptionalTransport::none()
});

let (transport, sinks) = bandwidth::BandwidthLogging::new(transport, Duration::from_secs(5));
let (transport, bandwidth) = bandwidth::BandwidthLogging::new(transport);

// Encryption
let transport = transport.and_then(move |stream, endpoint| {
Expand Down Expand Up @@ -145,5 +151,6 @@ pub fn build_transport(
.map_err(|err| io::Error::new(io::ErrorKind::Other, err))
.boxed();

(transport, sinks)
(transport, bandwidth)
}

mxinden marked this conversation as resolved.
Show resolved Hide resolved
2 changes: 1 addition & 1 deletion client/network/test/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ parking_lot = "0.10.0"
futures = "0.3.4"
futures-timer = "3.0.1"
rand = "0.7.2"
libp2p = { version = "0.22.0", default-features = false }
libp2p = { version = "0.23.0", default-features = false }
sp-consensus = { version = "0.8.0-rc5", path = "../../../primitives/consensus/common" }
sc-consensus = { version = "0.8.0-rc5", path = "../../../client/consensus/common" }
sc-client-api = { version = "2.0.0-rc5", path = "../../api" }
Expand Down
2 changes: 1 addition & 1 deletion client/peerset/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ targets = ["x86_64-unknown-linux-gnu"]

[dependencies]
futures = "0.3.4"
libp2p = { version = "0.22.0", default-features = false }
libp2p = { version = "0.23.0", default-features = false }
sp-utils = { version = "2.0.0-rc5", path = "../../primitives/utils"}
log = "0.4.8"
serde_json = "1.0.41"
Expand Down
8 changes: 4 additions & 4 deletions client/rpc/src/system/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ fn api<T: Into<Option<Status>>>(sync: T) -> System<Block> {
external_addresses: Default::default(),
connected_peers: Default::default(),
not_connected_peers: Default::default(),
average_download_per_sec: 0,
average_upload_per_sec: 0,
total_bytes_inbound: 0,
total_bytes_outbound: 0,
peerset: serde_json::Value::Null,
}).unwrap());
},
Expand Down Expand Up @@ -282,8 +282,8 @@ fn system_network_state() {
external_addresses: Default::default(),
connected_peers: Default::default(),
not_connected_peers: Default::default(),
average_download_per_sec: 0,
average_upload_per_sec: 0,
total_bytes_inbound: 0,
total_bytes_outbound: 0,
peerset: serde_json::Value::Null,
}
);
Expand Down
4 changes: 2 additions & 2 deletions client/service/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,8 +322,8 @@ async fn build_network_future<
num_sync_peers: network.num_sync_peers(),
num_connected_peers: network.num_connected_peers(),
num_active_peers: network.num_active_peers(),
average_download_per_sec: network.average_download_per_sec(),
average_upload_per_sec: network.average_upload_per_sec(),
total_bytes_inbound: network.total_bytes_inbound(),
total_bytes_outbound: network.total_bytes_outbound(),
};
let state = network.network_state();
ready_sink.send((status, state));
Expand Down
Loading