Skip to content

Commit

Permalink
Add flag to disable warning logs for duplicate gossip messages (#5009)
Browse files Browse the repository at this point in the history
* Add flag to disable warning logs for duplicate gossip messages.

* Update Lighthouse book.
  • Loading branch information
jimmygchen authored Dec 14, 2023
1 parent ae4a296 commit 366f0d7
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 1 deletion.
5 changes: 5 additions & 0 deletions beacon_node/lighthouse_network/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,10 @@ pub struct Config {

/// Configuration for the inbound rate limiter (requests received by this node).
pub inbound_rate_limiter_config: Option<InboundRateLimiterConfig>,

/// 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 {
Expand Down Expand Up @@ -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,
}
}
}
Expand Down
19 changes: 18 additions & 1 deletion beacon_node/lighthouse_network/src/service/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ pub struct Network<AppReqId: ReqId, TSpec: EthSpec> {
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,
}
Expand Down Expand Up @@ -425,6 +427,7 @@ impl<AppReqId: ReqId, TSpec: EthSpec> Network<AppReqId, TSpec> {
update_gossipsub_scores,
gossip_cache,
local_peer_id,
disable_duplicate_warn_logs: config.disable_duplicate_warn_logs,
log,
};

Expand Down Expand Up @@ -743,7 +746,21 @@ impl<AppReqId: ReqId, TSpec: EthSpec> Network<AppReqId, TSpec> {
.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() {
Expand Down
10 changes: 10 additions & 0 deletions beacon_node/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
3 changes: 3 additions & 0 deletions beacon_node/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}

Expand Down
5 changes: 5 additions & 0 deletions book/src/help_bn.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 4 additions & 0 deletions book/src/redundancy.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.

Expand Down
19 changes: 19 additions & 0 deletions lighthouse/tests/beacon_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
}

0 comments on commit 366f0d7

Please sign in to comment.