Skip to content

Commit

Permalink
Downgrades a valid log (#2057)
Browse files Browse the repository at this point in the history
## Issue Addressed

#2046 

## Proposed Changes

The log was originally intended to verify the correct logic and ordering of events when scoring peers. The queued tasks can be structured in such a way that peers can be banned after they are disconnected. Therefore the error log is now downgraded to  debug log.
  • Loading branch information
AgeManning committed Dec 8, 2020
1 parent ddd2eab commit 655b1b7
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 19 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

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

5 changes: 3 additions & 2 deletions beacon_node/eth2_libp2p/src/peer_manager/peerdb.rs
Expand Up @@ -474,8 +474,9 @@ impl<TSpec: EthSpec> PeerDB<TSpec> {
// Check and verify all the connection states
match info.connection_status() {
PeerConnectionStatus::Disconnected { .. } => {
// It should not be possible to ban a peer that is already disconnected.
error!(log_ref, "Banning a disconnected peer"; "peer_id" => %peer_id);
// It is possible to ban a peer that has a disconnected score, if there are many
// events that score it poorly and are processed after it has disconnected.
debug!(log_ref, "Banning a disconnected peer"; "peer_id" => %peer_id);
self.disconnected_peers = self.disconnected_peers.saturating_sub(1);
info.ban();
self.banned_peers_count
Expand Down
1 change: 1 addition & 0 deletions beacon_node/network/Cargo.toml
Expand Up @@ -13,6 +13,7 @@ tempfile = "3.1.0"
exit-future = "0.2.0"
slog-term = "2.6.0"
slog-async = "2.5.0"
logging = { path = "../../common/logging" }

[dependencies]
beacon_chain = { path = "../beacon_chain" }
Expand Down
36 changes: 19 additions & 17 deletions beacon_node/network/src/service/tests.rs
Expand Up @@ -5,35 +5,35 @@ mod tests {
use crate::{NetworkConfig, NetworkService};
use beacon_chain::test_utils::BeaconChainHarness;
use eth2_libp2p::Enr;
//use slog::{o, Drain, Level, Logger};
use slog::Logger;
use slog::{o, Drain, Level, Logger};
use sloggers::{null::NullLoggerBuilder, Build};
use std::str::FromStr;
use std::sync::Arc;
use store::config::StoreConfig;
use tokio::runtime::Runtime;
use types::{test_utils::generate_deterministic_keypairs, MinimalEthSpec};

fn get_logger() -> Logger {
/* For emitting logs during the tests
let drain = {
let decorator = slog_term::TermDecorator::new().build();
let decorator =
logging::AlignedTermDecorator::new(decorator, logging::MAX_MESSAGE_WIDTH);
let drain = slog_term::FullFormat::new(decorator).build().fuse();
let drain = slog_async::Async::new(drain).chan_size(2048).build();
drain.filter_level(Level::Debug)
};
fn get_logger(actual_log: bool) -> Logger {
if actual_log {
let drain = {
let decorator = slog_term::TermDecorator::new().build();
let decorator =
logging::AlignedTermDecorator::new(decorator, logging::MAX_MESSAGE_WIDTH);
let drain = slog_term::FullFormat::new(decorator).build().fuse();
let drain = slog_async::Async::new(drain).chan_size(2048).build();
drain.filter_level(Level::Debug)
};

Logger::root(drain.fuse(), o!())
*/
let builder = NullLoggerBuilder;
builder.build().expect("should build logger")
Logger::root(drain.fuse(), o!())
} else {
let builder = NullLoggerBuilder;
builder.build().expect("should build logger")
}
}

#[test]
fn test_dht_persistence() {
let log = get_logger();
let log = get_logger(false);

let beacon_chain = Arc::new(
BeaconChainHarness::new_with_store_config(
Expand Down Expand Up @@ -73,6 +73,8 @@ mod tests {
let _ = NetworkService::start(beacon_chain.clone(), &config, executor)
.await
.unwrap();
// Allow the network task to spawn on the executor before shutting down.
tokio::time::sleep(std::time::Duration::from_secs(1)).await;
drop(signal);
});

Expand Down

0 comments on commit 655b1b7

Please sign in to comment.