From 4bec71ad19a5d718b20f0847fa42b11a5172a52d Mon Sep 17 00:00:00 2001 From: carneiro-cw <156914855+carneiro-cw@users.noreply.github.com> Date: Tue, 17 Dec 2024 16:52:48 -0300 Subject: [PATCH] chore: put rocks metrics collection behind a feature flag (#1922) --- Cargo.toml | 3 +++ src/eth/storage/permanent/rocks/rocks_cf.rs | 4 ++-- .../storage/permanent/rocks/rocks_config.rs | 2 +- .../storage/permanent/rocks/rocks_permanent.rs | 2 +- src/eth/storage/permanent/rocks/rocks_state.rs | 18 ++++++++++-------- 5 files changed, 17 insertions(+), 12 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index d0d38dfbd..741cbefb4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -201,6 +201,9 @@ dev = [] # Enable runtime metrics collection. metrics = ["dep:metrics-exporter-prometheus"] +# Enable runtime rocksdb metrics collection. +rocks_metrics = ["metrics"] + # Enable runtime tracing/spans collection. tracing = [] diff --git a/src/eth/storage/permanent/rocks/rocks_cf.rs b/src/eth/storage/permanent/rocks/rocks_cf.rs index 894ad3528..b40b2598c 100644 --- a/src/eth/storage/permanent/rocks/rocks_cf.rs +++ b/src/eth/storage/permanent/rocks/rocks_cf.rs @@ -18,7 +18,7 @@ use rocksdb::DB; use serde::Deserialize; use serde::Serialize; -#[cfg(feature = "metrics")] +#[cfg(feature = "rocks_metrics")] use crate::infra::metrics; /// A RocksDB Column Family (CF) reference. @@ -232,7 +232,7 @@ where } } - #[cfg(feature = "metrics")] + #[cfg(feature = "rocks_metrics")] pub fn export_metrics(&self) { let handle = self.handle(); let cur_size_active_mem_table = self diff --git a/src/eth/storage/permanent/rocks/rocks_config.rs b/src/eth/storage/permanent/rocks/rocks_config.rs index 25a6d5218..507e20f18 100644 --- a/src/eth/storage/permanent/rocks/rocks_config.rs +++ b/src/eth/storage/permanent/rocks/rocks_config.rs @@ -39,7 +39,7 @@ impl DbConfig { opts.increase_parallelism(16); // NOTE: As per the rocks db wiki: "The overhead of statistics is usually small but non-negligible. We usually observe an overhead of 5%-10%." - #[cfg(feature = "metrics")] + #[cfg(feature = "rocks_metrics")] { opts.enable_statistics(); opts.set_statistics_level(rocksdb::statistics::StatsLevel::ExceptTimeForMutex); diff --git a/src/eth/storage/permanent/rocks/rocks_permanent.rs b/src/eth/storage/permanent/rocks/rocks_permanent.rs index 4ab9fbeb2..b834f0f6d 100644 --- a/src/eth/storage/permanent/rocks/rocks_permanent.rs +++ b/src/eth/storage/permanent/rocks/rocks_permanent.rs @@ -125,7 +125,7 @@ impl PermanentStorage for RocksPermanentStorage { } fn save_block(&self, block: Block) -> anyhow::Result<()> { - #[cfg(feature = "metrics")] + #[cfg(feature = "rocks_metrics")] { self.state.export_metrics().inspect_err(|e| { tracing::error!(reason = ?e, "failed to export metrics in RocksPermanent"); diff --git a/src/eth/storage/permanent/rocks/rocks_state.rs b/src/eth/storage/permanent/rocks/rocks_state.rs index e2776aa4b..7404ab1ca 100644 --- a/src/eth/storage/permanent/rocks/rocks_state.rs +++ b/src/eth/storage/permanent/rocks/rocks_state.rs @@ -52,17 +52,19 @@ use crate::eth::primitives::Slot; use crate::eth::primitives::SlotIndex; use crate::eth::primitives::TransactionMined; use crate::ext::OptionExt; +#[cfg(feature = "metrics")] +use crate::infra::metrics; use crate::log_and_err; use crate::utils::GIGABYTE; cfg_if::cfg_if! { - if #[cfg(feature = "metrics")] { + if #[cfg(feature = "rocks_metrics")] { use parking_lot::Mutex; use rocksdb::statistics::Histogram; use rocksdb::statistics::Ticker; - use crate::infra::metrics::{self, Count, HistogramInt, Sum}; + use crate::infra::metrics::{Count, HistogramInt, Sum}; } } @@ -119,13 +121,13 @@ pub struct RocksStorageState { blocks_by_hash: RocksCfRef, logs: RocksCfRef<(HashRocksdb, IndexRocksdb), CfLogsValue>, /// Last collected stats for a histogram - #[cfg(feature = "metrics")] + #[cfg(feature = "rocks_metrics")] prev_stats: Mutex>, /// Options passed at DB creation, stored for metrics /// /// a newly created `rocksdb::Options` object is unique, with an underlying pointer identifier inside of it, and is used to access /// the DB metrics, `Options` can be cloned, but two equal `Options` might not retrieve the same metrics - #[cfg(feature = "metrics")] + #[cfg(feature = "rocks_metrics")] db_options: Options, shutdown_timeout: Duration, enable_sync_write: bool, @@ -137,7 +139,7 @@ impl RocksStorageState { let cf_options_map = generate_cf_options_map(cache_multiplier); - #[cfg_attr(not(feature = "metrics"), allow(unused_variables))] + #[cfg_attr(not(feature = "rocks_metrics"), allow(unused_variables))] let (db, db_options) = create_or_open_db(&path, &cf_options_map).context("when trying to create (or open) rocksdb")?; if db.path().to_str().is_none() { @@ -157,9 +159,9 @@ impl RocksStorageState { blocks_by_number: new_cf_ref(&db, "blocks_by_number", &cf_options_map)?, blocks_by_hash: new_cf_ref(&db, "blocks_by_hash", &cf_options_map)?, logs: new_cf_ref(&db, "logs", &cf_options_map)?, - #[cfg(feature = "metrics")] + #[cfg(feature = "rocks_metrics")] prev_stats: Mutex::default(), - #[cfg(feature = "metrics")] + #[cfg(feature = "rocks_metrics")] db_options, db, shutdown_timeout, @@ -525,7 +527,7 @@ impl RocksStorageState { } } -#[cfg(feature = "metrics")] +#[cfg(feature = "rocks_metrics")] impl RocksStorageState { pub fn export_metrics(&self) -> Result<()> { let db_get = self.db_options.get_histogram_data(Histogram::DbGet);