From 8faaa35b58599b64f07befeb8fd676b25518f2f9 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Mon, 20 Jun 2022 23:20:30 +0000 Subject: [PATCH] Enable malloc metrics for the VC (#3279) ## Issue Addressed Following up from https://github.com/sigp/lighthouse/pull/3223#issuecomment-1158718102, it has been observed that the validator client uses vastly more memory in some compilation configurations than others. Compiling with Cross and then putting the binary into an Ubuntu 22.04 image seems to use 3x more memory than compiling with Cargo directly on Debian bullseye. ## Proposed Changes Enable malloc metrics for the validator client. This will hopefully allow us to see the difference between the two compilation configs and compare heap fragmentation. This PR doesn't enable malloc tuning for the VC because it was found to perform significantly worse. The `--disable-malloc-tuning` flag is repurposed to just disable the metrics. --- Cargo.lock | 1 + lighthouse/tests/validator_client.rs | 11 ++++++++--- validator_client/Cargo.toml | 1 + validator_client/src/config.rs | 7 ++++++- validator_client/src/http_metrics/metrics.rs | 7 +++++++ validator_client/src/http_metrics/mod.rs | 2 ++ 6 files changed, 25 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c147af092..de385f22c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6827,6 +6827,7 @@ dependencies = [ "lighthouse_version", "lockfile", "logging", + "malloc_utils", "monitoring_api", "parking_lot 0.12.1", "rand 0.8.5", diff --git a/lighthouse/tests/validator_client.rs b/lighthouse/tests/validator_client.rs index c14f5d27b..61c239f86 100644 --- a/lighthouse/tests/validator_client.rs +++ b/lighthouse/tests/validator_client.rs @@ -426,9 +426,14 @@ fn metrics_allow_origin_all_flag() { pub fn malloc_tuning_flag() { CommandLineTest::new() .flag("disable-malloc-tuning", None) - // Simply ensure that the node can start with this flag, it's very difficult to observe the - // effects of it. - .run(); + .run() + .with_config(|config| assert_eq!(config.http_metrics.allocator_metrics_enabled, false)); +} +#[test] +pub fn malloc_tuning_default() { + CommandLineTest::new() + .run() + .with_config(|config| assert_eq!(config.http_metrics.allocator_metrics_enabled, true)); } #[test] fn doppelganger_protection_flag() { diff --git a/validator_client/Cargo.toml b/validator_client/Cargo.toml index 9833c046f..8a3c8303a 100644 --- a/validator_client/Cargo.toml +++ b/validator_client/Cargo.toml @@ -58,3 +58,4 @@ sensitive_url = { path = "../common/sensitive_url" } task_executor = { path = "../common/task_executor" } reqwest = { version = "0.11.0", features = ["json","stream"] } url = "2.2.2" +malloc_utils = { path = "../common/malloc_utils" } diff --git a/validator_client/src/config.rs b/validator_client/src/config.rs index 45e10e39e..e56e64f5a 100644 --- a/validator_client/src/config.rs +++ b/validator_client/src/config.rs @@ -2,7 +2,7 @@ use crate::fee_recipient_file::FeeRecipientFile; use crate::graffiti_file::GraffitiFile; use crate::{http_api, http_metrics}; use clap::ArgMatches; -use clap_utils::{parse_optional, parse_required}; +use clap_utils::{flags::DISABLE_MALLOC_TUNING_FLAG, parse_optional, parse_required}; use directory::{ get_network_dir, DEFAULT_HARDCODED_NETWORK, DEFAULT_ROOT_DIR, DEFAULT_SECRET_DIR, DEFAULT_VALIDATOR_DIR, @@ -293,6 +293,11 @@ impl Config { config.http_metrics.allow_origin = Some(allow_origin.to_string()); } + + if cli_args.is_present(DISABLE_MALLOC_TUNING_FLAG) { + config.http_metrics.allocator_metrics_enabled = false; + } + /* * Explorer metrics */ diff --git a/validator_client/src/http_metrics/metrics.rs b/validator_client/src/http_metrics/metrics.rs index 56c1299b3..f405f1a2b 100644 --- a/validator_client/src/http_metrics/metrics.rs +++ b/validator_client/src/http_metrics/metrics.rs @@ -1,4 +1,5 @@ use super::Context; +use malloc_utils::scrape_allocator_metrics; use slot_clock::SlotClock; use std::time::{SystemTime, UNIX_EPOCH}; use types::EthSpec; @@ -206,6 +207,12 @@ pub fn gather_prometheus_metrics( } } + // It's important to ensure these metrics are explicitly enabled in the case that users aren't + // using glibc and this function causes panics. + if ctx.config.allocator_metrics_enabled { + scrape_allocator_metrics(); + } + warp_utils::metrics::scrape_health_metrics(); encoder diff --git a/validator_client/src/http_metrics/mod.rs b/validator_client/src/http_metrics/mod.rs index 51a2d3f8a..c30d60344 100644 --- a/validator_client/src/http_metrics/mod.rs +++ b/validator_client/src/http_metrics/mod.rs @@ -56,6 +56,7 @@ pub struct Config { pub listen_addr: IpAddr, pub listen_port: u16, pub allow_origin: Option, + pub allocator_metrics_enabled: bool, } impl Default for Config { @@ -65,6 +66,7 @@ impl Default for Config { listen_addr: IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), listen_port: 5064, allow_origin: None, + allocator_metrics_enabled: true, } } }