network: don't log re-discovered addresses (#6881)

* network: move LruHashSet to network crate utils

* network: don't log re-discovered external addresses

* Update client/network/src/utils.rs

Co-authored-by: mattrutherford <44339188+mattrutherford@users.noreply.github.com>

Co-authored-by: mattrutherford <44339188+mattrutherford@users.noreply.github.com>
This commit is contained in:
André Silva
2020-08-13 19:38:14 +01:00
committed by GitHub
parent bcc1b7c146
commit 0777a93532
4 changed files with 94 additions and 89 deletions
+25 -4
View File
@@ -46,6 +46,7 @@
//!
use crate::config::ProtocolId;
use crate::utils::LruHashSet;
use futures::prelude::*;
use futures_timer::Delay;
use ip_network::IpNetwork;
@@ -63,10 +64,15 @@ use libp2p::swarm::toggle::Toggle;
use libp2p::mdns::{Mdns, MdnsEvent};
use libp2p::multiaddr::Protocol;
use log::{debug, info, trace, warn};
use std::{cmp, collections::{HashMap, HashSet, VecDeque}, io, time::Duration};
use std::{cmp, collections::{HashMap, HashSet, VecDeque}, io, num::NonZeroUsize, time::Duration};
use std::task::{Context, Poll};
use sp_core::hexdisplay::HexDisplay;
/// Maximum number of known external addresses that we will cache.
/// This only affects whether we will log whenever we (re-)discover
/// a given address.
const MAX_KNOWN_EXTERNAL_ADDRESSES: usize = 32;
/// `DiscoveryBehaviour` configuration.
///
/// Note: In order to discover nodes or load and store values via Kademlia one has to add at least
@@ -190,7 +196,11 @@ impl DiscoveryConfig {
} else {
None.into()
},
allow_non_globals_in_dht: self.allow_non_globals_in_dht
allow_non_globals_in_dht: self.allow_non_globals_in_dht,
known_external_addresses: LruHashSet::new(
NonZeroUsize::new(MAX_KNOWN_EXTERNAL_ADDRESSES)
.expect("value is a constant; constant is non-zero; qed.")
),
}
}
}
@@ -221,7 +231,9 @@ pub struct DiscoveryBehaviour {
/// Number of active connections over which we interrupt the discovery process.
discovery_only_if_under_num: u64,
/// Should non-global addresses be added to the DHT?
allow_non_globals_in_dht: bool
allow_non_globals_in_dht: bool,
/// A cache of discovered external addresses. Only used for logging purposes.
known_external_addresses: LruHashSet<Multiaddr>,
}
impl DiscoveryBehaviour {
@@ -507,7 +519,16 @@ impl NetworkBehaviour for DiscoveryBehaviour {
fn inject_new_external_addr(&mut self, addr: &Multiaddr) {
let new_addr = addr.clone()
.with(Protocol::P2p(self.local_peer_id.clone().into()));
info!(target: "sub-libp2p", "🔍 Discovered new external address for our node: {}", new_addr);
// NOTE: we might re-discover the same address multiple times
// in which case we just want to refrain from logging.
if self.known_external_addresses.insert(new_addr.clone()) {
info!(target: "sub-libp2p",
"🔍 Discovered new external address for our node: {}",
new_addr,
);
}
for k in self.kademlias.values_mut() {
NetworkBehaviour::inject_new_external_addr(k, addr)
}
+1 -3
View File
@@ -21,7 +21,7 @@ use crate::{
chain::{Client, FinalityProofProvider},
config::{BoxFinalityProofRequestBuilder, ProtocolId, TransactionPool, TransactionImportFuture, TransactionImport},
error,
utils::interval
utils::{interval, LruHashSet},
};
use bytes::{Bytes, BytesMut};
@@ -60,11 +60,9 @@ use std::fmt::Write;
use std::{cmp, io, num::NonZeroUsize, pin::Pin, task::Poll, time};
use log::{log, Level, trace, debug, warn, error};
use sc_client_api::{ChangesProof, StorageProof};
use util::LruHashSet;
use wasm_timer::Instant;
mod generic_proto;
mod util;
pub mod message;
pub mod event;
@@ -1,76 +0,0 @@
// Copyright 2019-2020 Parity Technologies (UK) Ltd.
// This file is part of Substrate.
// Substrate is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
// Substrate is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
// You should have received a copy of the GNU General Public License
// along with Substrate. If not, see <http://www.gnu.org/licenses/>.
use linked_hash_set::LinkedHashSet;
use std::{hash::Hash, num::NonZeroUsize};
/// Wrapper around `LinkedHashSet` which grows bounded.
///
/// In the limit, for each element inserted the oldest existing element will be removed.
#[derive(Debug, Clone)]
pub(crate) struct LruHashSet<T: Hash + Eq> {
set: LinkedHashSet<T>,
limit: NonZeroUsize
}
impl<T: Hash + Eq> LruHashSet<T> {
/// Create a new `LruHashSet` with the given (exclusive) limit.
pub(crate) fn new(limit: NonZeroUsize) -> Self {
Self { set: LinkedHashSet::new(), limit }
}
/// Insert element into the set.
///
/// Returns `true` if this is a new element to the set, `false` otherwise.
/// Maintains the limit of the set by removing the oldest entry if necessary.
/// Inserting the same element will update its LRU position.
pub(crate) fn insert(&mut self, e: T) -> bool {
if self.set.insert(e) {
if self.set.len() == usize::from(self.limit) {
self.set.pop_front(); // remove oldest entry
}
return true
}
false
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn maintains_limit() {
let three = NonZeroUsize::new(3).unwrap();
let mut set = LruHashSet::<u8>::new(three);
// First element.
assert!(set.insert(1));
assert_eq!(vec![&1], set.set.iter().collect::<Vec<_>>());
// Second element.
assert!(set.insert(2));
assert_eq!(vec![&1, &2], set.set.iter().collect::<Vec<_>>());
// Inserting the same element updates its LRU position.
assert!(!set.insert(1));
assert_eq!(vec![&2, &1], set.set.iter().collect::<Vec<_>>());
// We reached the limit. The next element forces the oldest one out.
assert!(set.insert(3));
assert_eq!(vec![&1, &3], set.set.iter().collect::<Vec<_>>());
}
}
+68 -6
View File
@@ -14,12 +14,74 @@
// You should have received a copy of the GNU General Public License
// along with Substrate. If not, see <http://www.gnu.org/licenses/>.
use std::time::Duration;
use futures::{FutureExt, Stream, StreamExt, stream::unfold};
use futures::{stream::unfold, FutureExt, Stream, StreamExt};
use futures_timer::Delay;
use linked_hash_set::LinkedHashSet;
use std::time::Duration;
use std::{hash::Hash, num::NonZeroUsize};
pub fn interval(duration: Duration) -> impl Stream<Item=()> + Unpin {
unfold((), move |_| {
Delay::new(duration).map(|_| Some(((), ())))
}).map(drop)
/// Creates a stream that returns a new value every `duration`.
pub fn interval(duration: Duration) -> impl Stream<Item = ()> + Unpin {
unfold((), move |_| Delay::new(duration).map(|_| Some(((), ())))).map(drop)
}
/// Wrapper around `LinkedHashSet` with bounded growth.
///
/// In the limit, for each element inserted the oldest existing element will be removed.
#[derive(Debug, Clone)]
pub struct LruHashSet<T: Hash + Eq> {
set: LinkedHashSet<T>,
limit: NonZeroUsize,
}
impl<T: Hash + Eq> LruHashSet<T> {
/// Create a new `LruHashSet` with the given (exclusive) limit.
pub fn new(limit: NonZeroUsize) -> Self {
Self {
set: LinkedHashSet::new(),
limit,
}
}
/// Insert element into the set.
///
/// Returns `true` if this is a new element to the set, `false` otherwise.
/// Maintains the limit of the set by removing the oldest entry if necessary.
/// Inserting the same element will update its LRU position.
pub fn insert(&mut self, e: T) -> bool {
if self.set.insert(e) {
if self.set.len() == usize::from(self.limit) {
self.set.pop_front(); // remove oldest entry
}
return true;
}
false
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn maintains_limit() {
let three = NonZeroUsize::new(3).unwrap();
let mut set = LruHashSet::<u8>::new(three);
// First element.
assert!(set.insert(1));
assert_eq!(vec![&1], set.set.iter().collect::<Vec<_>>());
// Second element.
assert!(set.insert(2));
assert_eq!(vec![&1, &2], set.set.iter().collect::<Vec<_>>());
// Inserting the same element updates its LRU position.
assert!(!set.insert(1));
assert_eq!(vec![&2, &1], set.set.iter().collect::<Vec<_>>());
// We reached the limit. The next element forces the oldest one out.
assert!(set.insert(3));
assert_eq!(vec![&1, &3], set.set.iter().collect::<Vec<_>>());
}
}