From 366f0d7ac27ad67892a731ccb4dfd82ee647a522 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Fri, 15 Dec 2023 09:26:51 +1100 Subject: [PATCH] Add flag to disable warning logs for duplicate gossip messages (#5009) * Add flag to disable warning logs for duplicate gossip messages. * Update Lighthouse book. --- beacon_node/lighthouse_network/src/config.rs | 5 +++++ .../lighthouse_network/src/service/mod.rs | 19 ++++++++++++++++++- beacon_node/src/cli.rs | 10 ++++++++++ beacon_node/src/config.rs | 3 +++ book/src/help_bn.md | 5 +++++ book/src/redundancy.md | 4 ++++ lighthouse/tests/beacon_node.rs | 19 +++++++++++++++++++ 7 files changed, 64 insertions(+), 1 deletion(-) diff --git a/beacon_node/lighthouse_network/src/config.rs b/beacon_node/lighthouse_network/src/config.rs index 0eb3f7bc8..3c0b4c723 100644 --- a/beacon_node/lighthouse_network/src/config.rs +++ b/beacon_node/lighthouse_network/src/config.rs @@ -158,6 +158,10 @@ pub struct Config { /// Configuration for the inbound rate limiter (requests received by this node). pub inbound_rate_limiter_config: Option, + + /// Whether to disable logging duplicate gossip messages as WARN. If set to true, duplicate + /// errors will be logged at DEBUG level. + pub disable_duplicate_warn_logs: bool, } impl Config { @@ -375,6 +379,7 @@ impl Default for Config { outbound_rate_limiter_config: None, invalid_block_storage: None, inbound_rate_limiter_config: None, + disable_duplicate_warn_logs: false, } } } diff --git a/beacon_node/lighthouse_network/src/service/mod.rs b/beacon_node/lighthouse_network/src/service/mod.rs index 3c2a3f5a9..d15bde37d 100644 --- a/beacon_node/lighthouse_network/src/service/mod.rs +++ b/beacon_node/lighthouse_network/src/service/mod.rs @@ -128,6 +128,8 @@ pub struct Network { gossip_cache: GossipCache, /// This node's PeerId. pub local_peer_id: PeerId, + /// Flag to disable warning logs for duplicate gossip messages and log at DEBUG level instead. + pub disable_duplicate_warn_logs: bool, /// Logger for behaviour actions. log: slog::Logger, } @@ -425,6 +427,7 @@ impl Network { update_gossipsub_scores, gossip_cache, local_peer_id, + disable_duplicate_warn_logs: config.disable_duplicate_warn_logs, log, }; @@ -743,7 +746,21 @@ impl Network { .gossipsub_mut() .publish(Topic::from(topic.clone()), message_data.clone()) { - slog::warn!(self.log, "Could not publish message"; "error" => ?e); + if self.disable_duplicate_warn_logs && matches!(e, PublishError::Duplicate) { + debug!( + self.log, + "Could not publish message"; + "error" => ?e, + "kind" => %topic.kind(), + ); + } else { + warn!( + self.log, + "Could not publish message"; + "error" => ?e, + "kind" => %topic.kind(), + ); + }; // add to metrics match topic.kind() { diff --git a/beacon_node/src/cli.rs b/beacon_node/src/cli.rs index 214accd3f..0fc485b15 100644 --- a/beacon_node/src/cli.rs +++ b/beacon_node/src/cli.rs @@ -1288,5 +1288,15 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { .default_value("64") .takes_value(true) ) + .arg( + Arg::with_name("disable-duplicate-warn-logs") + .long("disable-duplicate-warn-logs") + .help("Disable warning logs for duplicate gossip messages. The WARN level log is \ + useful for detecting a duplicate validator key running elsewhere. However, this may \ + result in excessive warning logs if the validator is broadcasting messages to \ + multiple beacon nodes via the validator client --broadcast flag. In this case, \ + disabling these warn logs may be useful.") + .takes_value(false) + ) .group(ArgGroup::with_name("enable_http").args(&["http", "gui", "staking"]).multiple(true)) } diff --git a/beacon_node/src/config.rs b/beacon_node/src/config.rs index 9ceab47f3..7c48ae760 100644 --- a/beacon_node/src/config.rs +++ b/beacon_node/src/config.rs @@ -1395,6 +1395,9 @@ pub fn set_network_config( Some(config_str.parse()?) } }; + + config.disable_duplicate_warn_logs = cli_args.is_present("disable-duplicate-warn-logs"); + Ok(()) } diff --git a/book/src/help_bn.md b/book/src/help_bn.md index 2a08f9017..5ab7c3849 100644 --- a/book/src/help_bn.md +++ b/book/src/help_bn.md @@ -29,6 +29,11 @@ FLAGS: --disable-deposit-contract-sync Explictly disables syncing of deposit logs from the execution node. This overrides any previous option that depends on it. Useful if you intend to run a non-validating beacon node. + --disable-duplicate-warn-logs Disable warning logs for duplicate gossip messages. The WARN level log is + useful for detecting a duplicate validator key running elsewhere. + However, this may result in excessive warning logs if the validator is + broadcasting messages to multiple beacon nodes via the validator client + --broadcast flag. In this case, disabling these warn logs may be useful. -x, --disable-enr-auto-update Discovery automatically updates the nodes local ENR with an external IP address and port as seen by other peers on the network. This disables this feature, fixing the ENR's IP/PORT to those specified on boot. diff --git a/book/src/redundancy.md b/book/src/redundancy.md index 93529295a..8318aea21 100644 --- a/book/src/redundancy.md +++ b/book/src/redundancy.md @@ -101,6 +101,10 @@ from this list: - `none`: Disable all broadcasting. This option only has an effect when provided alone, otherwise it is ignored. Not recommended except for expert tweakers. +Broadcasting attestation, blocks and sync committee messages may result in excessive warning logs in the beacon node +due to duplicate gossip messages. In this case, it may be desirable to disable warning logs for duplicates using the +beacon node `--disable-duplicate-warn-logs` flag. + The default is `--broadcast subscriptions`. To also broadcast blocks for example, use `--broadcast subscriptions,blocks`. diff --git a/lighthouse/tests/beacon_node.rs b/lighthouse/tests/beacon_node.rs index fd74b1b5b..2965297e3 100644 --- a/lighthouse/tests/beacon_node.rs +++ b/lighthouse/tests/beacon_node.rs @@ -2558,3 +2558,22 @@ fn genesis_state_url_value() { assert_eq!(config.genesis_state_url_timeout, Duration::from_secs(42)); }); } + +#[test] +fn disable_duplicate_warn_logs_default() { + CommandLineTest::new() + .run_with_zero_port() + .with_config(|config| { + assert_eq!(config.network.disable_duplicate_warn_logs, false); + }); +} + +#[test] +fn disable_duplicate_warn_logs() { + CommandLineTest::new() + .flag("disable-duplicate-warn-logs", None) + .run_with_zero_port() + .with_config(|config| { + assert_eq!(config.network.disable_duplicate_warn_logs, true); + }); +}