From 33b15240ac354479580757192e4a777062f368b4 Mon Sep 17 00:00:00 2001 From: HaoranYi Date: Tue, 6 Dec 2022 17:08:56 -0600 Subject: [PATCH] Revert #28945 (#29127) revert #28945 --- Cargo.lock | 1 - bucket_map/src/bucket_map.rs | 91 -------------------------- bucket_map/src/bucket_storage.rs | 67 +------------------ core/Cargo.toml | 1 - core/src/system_monitor_service.rs | 64 +++++++----------- core/src/validator.rs | 17 ++--- ledger-tool/src/main.rs | 14 ++-- local-cluster/src/validator_configs.rs | 1 - programs/sbf/Cargo.lock | 1 - validator/src/cli.rs | 5 -- validator/src/main.rs | 1 - 11 files changed, 38 insertions(+), 225 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 06a91d7b8a063e..45f74ff6242e71 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5197,7 +5197,6 @@ dependencies = [ "serial_test", "solana-address-lookup-table-program", "solana-bloom", - "solana-bucket-map", "solana-client", "solana-entry", "solana-frozen-abi 1.15.0", diff --git a/bucket_map/src/bucket_map.rs b/bucket_map/src/bucket_map.rs index 460f47091859c3..55f669c1b037a8 100644 --- a/bucket_map/src/bucket_map.rs +++ b/bucket_map/src/bucket_map.rs @@ -181,97 +181,6 @@ fn read_be_u64(input: &[u8]) -> u64 { u64::from_be_bytes(input[0..std::mem::size_of::()].try_into().unwrap()) } -/// Utility function to get number of Mmap files for current process -#[cfg(target_os = "linux")] -pub fn get_mmap_count() -> Option { - let pid = std::process::id(); - let map_path = format!("/proc/{}/maps", pid); - let output = std::process::Command::new("wc") - .args(["-l", &map_path]) - .output() - .unwrap(); - if output.status.success() { - let n: usize = std::str::from_utf8(&output.stdout) - .unwrap() - .split_whitespace() - .next() - .unwrap() - .parse() - .unwrap(); - - Some(n) - } else { - None - } -} - -#[cfg(not(target_os = "linux"))] -pub fn get_mmap_count() -> Option { - None -} - -/// Utility function to get open files limits on the system -#[cfg(target_os = "linux")] -pub fn get_open_fd_limits() -> Option<(usize, usize)> { - let run = |cmd, args| -> Option { - let output = std::process::Command::new(cmd).args(args).output().unwrap(); - if output.status.success() { - let n: usize = std::str::from_utf8(&output.stdout) - .unwrap() - .split_whitespace() - .next() - .unwrap() - .parse() - .unwrap(); - - Some(n) - } else { - None - } - }; - - let soft_limit = run("sh", ["-c", "ulimit -Sn"])?; - let hard_limit = run("sh", ["-c", "ulimit -Hn"])?; - - Some((soft_limit, hard_limit)) -} - -#[cfg(not(target_os = "linux"))] -pub fn get_open_fd_limits() -> Option<(usize, usize)> { - None -} - -/// Utility function to get number of open file descriptors for current process -#[cfg(target_os = "linux")] -pub fn get_num_open_fd() -> Option { - let pid = std::process::id(); - let fd_path = format!("/proc/{}/fd", pid); - let cmd = format!("ls -l {} | wc -l", fd_path); - - let output = std::process::Command::new("sh") - .args(["-c", &cmd]) - .output() - .unwrap(); - if output.status.success() { - let n: usize = std::str::from_utf8(&output.stdout) - .unwrap() - .split_whitespace() - .next() - .unwrap() - .parse() - .unwrap(); - - Some(n) - } else { - None - } -} - -#[cfg(not(target_os = "linux"))] -pub fn get_num_open_fd() -> Option { - None -} - #[cfg(test)] mod tests { use { diff --git a/bucket_map/src/bucket_storage.rs b/bucket_map/src/bucket_storage.rs index 2090eefe0df182..49bc2963caae63 100644 --- a/bucket_map/src/bucket_storage.rs +++ b/bucket_map/src/bucket_storage.rs @@ -1,9 +1,5 @@ use { - crate::{ - bucket_map::{get_mmap_count, get_num_open_fd, get_open_fd_limits}, - bucket_stats::BucketStats, - MaxSearch, - }, + crate::{bucket_stats::BucketStats, MaxSearch}, memmap2::MmapMut, rand::{thread_rng, Rng}, solana_measure::measure::Measure, @@ -284,28 +280,11 @@ impl BucketStorage { .create(true) .open(file.clone()) .map_err(|e| { - let mmap_msg = get_mmap_count() - .map(|mmap_count| format!("current mmap_count: {}", mmap_count)) - .unwrap_or_default(); - - let open_fd_msg = get_num_open_fd() - .map(|open_fd| format!("current open_fd: {}", open_fd)) - .unwrap_or_default(); - - let limit_msg = get_open_fd_limits() - .map(|(soft_limit, hard_limit)| { - format!("soft_limit: {}, hard_limit: {}", soft_limit, hard_limit,) - }) - .unwrap_or_default(); - panic!( - "Unable to create data file {} in current dir({:?}): {:?}. {}, {}, {}", + "Unable to create data file {} in current dir({:?}): {:?}", file.display(), std::env::current_dir(), - e, - open_fd_msg, - mmap_msg, - limit_msg, + e ); }) .unwrap(); @@ -404,7 +383,6 @@ impl BucketStorage { mod test { use {super::*, tempfile::tempdir}; - #[cfg(target_os = "linux")] #[test] fn test_bucket_storage() { let tmpdir = tempdir().unwrap(); @@ -434,44 +412,5 @@ mod test { storage.free(ix, uid); assert!(storage.is_free(ix)); assert_eq!(storage.uid(ix), None); - - // test get_open_fd stats - let mmap_count = get_mmap_count().unwrap(); - let open_fd = get_num_open_fd().unwrap(); - let (soft_limit, hard_limit) = get_open_fd_limits().unwrap(); - - assert!(mmap_count > 0); - assert!(open_fd > 0); - assert!(soft_limit > 0); - assert!(hard_limit > 0); - } - - /// bench get_mmap_count - /// 2M mmaps takes 1.5s - #[cfg(target_os = "linux")] - #[ignore] - #[test] - fn test_time_mmap() { - use std::time::Instant; - - let mut v = vec![]; - for i in 1..1900000 { - if i % 100 == 0 { - println!("{}", i); - } - - let tmpdir = tempdir().unwrap(); - let paths: Vec = vec![tmpdir.path().to_path_buf()]; - assert!(!paths.is_empty()); - let s = BucketStorage::new(Arc::new(paths), 1, 1, 1, Arc::default(), Arc::default()); - v.push(s); - } - - let start = Instant::now(); - let mmap_count = get_mmap_count().unwrap(); - let duration = start.elapsed(); - - println!("{}", mmap_count); - println!("Time elapsed is: {:?}", duration); } } diff --git a/core/Cargo.toml b/core/Cargo.toml index b9034a513641d1..8829d4b9509f8b 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -38,7 +38,6 @@ serde = "1.0.144" serde_derive = "1.0.103" solana-address-lookup-table-program = { path = "../programs/address-lookup-table", version = "=1.15.0" } solana-bloom = { path = "../bloom", version = "=1.15.0" } -solana-bucket-map = { path = "../bucket_map", version = "=1.15.0" } solana-client = { path = "../client", version = "=1.15.0" } solana-entry = { path = "../entry", version = "=1.15.0" } solana-frozen-abi = { path = "../frozen-abi", version = "=1.15.0" } diff --git a/core/src/system_monitor_service.rs b/core/src/system_monitor_service.rs index b90e36acf68e09..8806b62bc85e06 100644 --- a/core/src/system_monitor_service.rs +++ b/core/src/system_monitor_service.rs @@ -7,7 +7,6 @@ use num_enum::{IntoPrimitive, TryFromPrimitive}; #[cfg(target_os = "linux")] use std::{fs::File, io::BufReader}; use { - solana_bucket_map::bucket_map::{get_mmap_count, get_num_open_fd}, solana_sdk::timing::AtomicInterval, std::{ collections::HashMap, @@ -30,7 +29,6 @@ const SAMPLE_INTERVAL_OS_NETWORK_LIMITS_MS: u64 = MS_PER_H; const SAMPLE_INTERVAL_MEM_MS: u64 = 5 * MS_PER_S; const SAMPLE_INTERVAL_CPU_MS: u64 = 10 * MS_PER_S; const SAMPLE_INTERVAL_DISK_MS: u64 = 5 * MS_PER_S; -const SAMPLE_INTERVAL_OPEN_FD_MS: u64 = 60 * MS_PER_S; const SLEEP_INTERVAL: Duration = Duration::from_millis(500); #[cfg(target_os = "linux")] @@ -387,21 +385,25 @@ fn parse_disk_stats(reader_diskstats: &mut impl BufRead) -> Result, config: SystemMonitorStatsReportConfig) -> Self { + pub fn new( + exit: Arc, + report_os_memory_stats: bool, + report_os_network_stats: bool, + report_os_cpu_stats: bool, + report_os_disk_stats: bool, + ) -> Self { info!("Starting SystemMonitorService"); let thread_hdl = Builder::new() .name("solSystemMonitr".to_string()) .spawn(move || { - Self::run(exit, config); + Self::run( + exit, + report_os_memory_stats, + report_os_network_stats, + report_os_cpu_stats, + report_os_disk_stats, + ); }) .unwrap(); @@ -830,22 +832,6 @@ impl SystemMonitorService { Self::report_cpuid_values(); } - #[cfg(target_os = "linux")] - fn report_open_fd_stats() { - if let Some(curr_mmap_count) = get_mmap_count() { - if let Some(curr_open_fd) = get_num_open_fd() { - datapoint_info!( - "open-mmap-stats", - ("number_mmap_files", curr_mmap_count, i64), - ("number_open_fd", curr_open_fd, i64), - ); - } - } - } - - #[cfg(not(target_os = "linux"))] - fn report_open_fd_stats() {} - #[cfg(target_os = "linux")] fn process_disk_stats(disk_stats: &mut Option) { match read_disk_stats() { @@ -981,7 +967,13 @@ impl SystemMonitorService { ) } - pub fn run(exit: Arc, config: SystemMonitorStatsReportConfig) { + pub fn run( + exit: Arc, + report_os_memory_stats: bool, + report_os_network_stats: bool, + report_os_cpu_stats: bool, + report_os_disk_stats: bool, + ) { let mut udp_stats = None; let mut disk_stats = None; let network_limits_timer = AtomicInterval::default(); @@ -989,13 +981,12 @@ impl SystemMonitorService { let mem_timer = AtomicInterval::default(); let cpu_timer = AtomicInterval::default(); let disk_timer = AtomicInterval::default(); - let open_fd_timer = AtomicInterval::default(); loop { if exit.load(Ordering::Relaxed) { break; } - if config.report_os_network_stats { + if report_os_network_stats { if network_limits_timer.should_update(SAMPLE_INTERVAL_OS_NETWORK_LIMITS_MS) { Self::check_os_network_limits(); } @@ -1003,20 +994,15 @@ impl SystemMonitorService { Self::process_net_stats(&mut udp_stats); } } - if config.report_os_memory_stats && mem_timer.should_update(SAMPLE_INTERVAL_MEM_MS) { + if report_os_memory_stats && mem_timer.should_update(SAMPLE_INTERVAL_MEM_MS) { Self::report_mem_stats(); } - if config.report_os_cpu_stats && cpu_timer.should_update(SAMPLE_INTERVAL_CPU_MS) { + if report_os_cpu_stats && cpu_timer.should_update(SAMPLE_INTERVAL_CPU_MS) { Self::report_cpu_stats(); } - if config.report_os_disk_stats && disk_timer.should_update(SAMPLE_INTERVAL_DISK_MS) { + if report_os_disk_stats && disk_timer.should_update(SAMPLE_INTERVAL_DISK_MS) { Self::process_disk_stats(&mut disk_stats); } - if config.report_os_open_fd_stats - && open_fd_timer.should_update(SAMPLE_INTERVAL_OPEN_FD_MS) - { - Self::report_open_fd_stats(); - } sleep(SLEEP_INTERVAL); } } diff --git a/core/src/validator.rs b/core/src/validator.rs index 21763a32e3d913..f8b7dc8a7659d2 100644 --- a/core/src/validator.rs +++ b/core/src/validator.rs @@ -18,9 +18,7 @@ use { sigverify, snapshot_packager_service::SnapshotPackagerService, stats_reporter_service::StatsReporterService, - system_monitor_service::{ - verify_net_stats_access, SystemMonitorService, SystemMonitorStatsReportConfig, - }, + system_monitor_service::{verify_net_stats_access, SystemMonitorService}, tower_storage::TowerStorage, tpu::{Tpu, TpuSockets, DEFAULT_TPU_COALESCE_MS}, tvu::{Tvu, TvuConfig, TvuSockets}, @@ -158,7 +156,6 @@ pub struct ValidatorConfig { pub no_os_network_stats_reporting: bool, pub no_os_cpu_stats_reporting: bool, pub no_os_disk_stats_reporting: bool, - pub no_os_open_fd_stats_reporting: bool, pub poh_pinned_cpu_core: usize, pub poh_hashes_per_batch: u64, pub process_ledger_before_services: bool, @@ -221,7 +218,6 @@ impl Default for ValidatorConfig { no_os_network_stats_reporting: true, no_os_cpu_stats_reporting: true, no_os_disk_stats_reporting: true, - no_os_open_fd_stats_reporting: true, poh_pinned_cpu_core: poh_service::DEFAULT_PINNED_CPU_CORE, poh_hashes_per_batch: poh_service::DEFAULT_HASHES_PER_BATCH, process_ledger_before_services: false, @@ -498,13 +494,10 @@ impl Validator { let system_monitor_service = Some(SystemMonitorService::new( Arc::clone(&exit), - SystemMonitorStatsReportConfig { - report_os_memory_stats: !config.no_os_memory_stats_reporting, - report_os_network_stats: !config.no_os_network_stats_reporting, - report_os_cpu_stats: !config.no_os_cpu_stats_reporting, - report_os_disk_stats: !config.no_os_disk_stats_reporting, - report_os_open_fd_stats: !config.no_os_open_fd_stats_reporting, - }, + !config.no_os_memory_stats_reporting, + !config.no_os_network_stats_reporting, + !config.no_os_cpu_stats_reporting, + !config.no_os_disk_stats_reporting, )); let (poh_timing_point_sender, poh_timing_point_receiver) = unbounded(); diff --git a/ledger-tool/src/main.rs b/ledger-tool/src/main.rs index 7c7889450ebfc6..a8675b5019db4e 100644 --- a/ledger-tool/src/main.rs +++ b/ledger-tool/src/main.rs @@ -25,8 +25,7 @@ use { }, solana_cli_output::{CliAccount, CliAccountNewConfig, OutputFormat}, solana_core::{ - system_monitor_service::{SystemMonitorService, SystemMonitorStatsReportConfig}, - validator::move_and_async_delete_path, + system_monitor_service::SystemMonitorService, validator::move_and_async_delete_path, }, solana_entry::entry::Entry, solana_geyser_plugin_manager::geyser_plugin_service::GeyserPluginService, @@ -2691,13 +2690,10 @@ fn main() { arg_matches.is_present("no_os_memory_stats_reporting"); let system_monitor_service = SystemMonitorService::new( Arc::clone(&exit_signal), - SystemMonitorStatsReportConfig { - report_os_memory_stats: !no_os_memory_stats_reporting, - report_os_network_stats: false, - report_os_cpu_stats: false, - report_os_disk_stats: false, - report_os_open_fd_stats: false, - }, + !no_os_memory_stats_reporting, + false, + false, + false, ); accounts_index_config.index_limit_mb = if let Some(limit) = diff --git a/local-cluster/src/validator_configs.rs b/local-cluster/src/validator_configs.rs index eda2952f1ffd7e..88a6977a48e9d5 100644 --- a/local-cluster/src/validator_configs.rs +++ b/local-cluster/src/validator_configs.rs @@ -46,7 +46,6 @@ pub fn safe_clone_config(config: &ValidatorConfig) -> ValidatorConfig { no_os_network_stats_reporting: config.no_os_network_stats_reporting, no_os_cpu_stats_reporting: config.no_os_cpu_stats_reporting, no_os_disk_stats_reporting: config.no_os_disk_stats_reporting, - no_os_open_fd_stats_reporting: config.no_os_open_fd_stats_reporting, poh_pinned_cpu_core: config.poh_pinned_cpu_core, account_indexes: config.account_indexes.clone(), accounts_db_caching_enabled: config.accounts_db_caching_enabled, diff --git a/programs/sbf/Cargo.lock b/programs/sbf/Cargo.lock index 270f3e417bf602..42b5e1a4767728 100644 --- a/programs/sbf/Cargo.lock +++ b/programs/sbf/Cargo.lock @@ -4477,7 +4477,6 @@ dependencies = [ "serde_derive", "solana-address-lookup-table-program", "solana-bloom", - "solana-bucket-map", "solana-client", "solana-entry", "solana-frozen-abi 1.15.0", diff --git a/validator/src/cli.rs b/validator/src/cli.rs index 4b9c260a9b1285..31f9ef878a4a5d 100644 --- a/validator/src/cli.rs +++ b/validator/src/cli.rs @@ -513,11 +513,6 @@ pub fn app<'a>(version: &'a str, default_args: &'a DefaultArgs) -> App<'a, 'a> { .long("no-os-disk-stats-reporting") .help("Disable reporting of OS disk statistics.") ) - .arg( - Arg::with_name("no_os_open_fd_stats_reporting") - .long("no-os-open-fd-stats-reporting") - .help("Disable reporting of open file descriptor statistics.") - ) .arg( Arg::with_name("accounts-hash-interval-slots") .long("accounts-hash-interval-slots") diff --git a/validator/src/main.rs b/validator/src/main.rs index cb7e6eb69ee222..4b88ec1931ce62 100644 --- a/validator/src/main.rs +++ b/validator/src/main.rs @@ -1125,7 +1125,6 @@ pub fn main() { no_os_network_stats_reporting: matches.is_present("no_os_network_stats_reporting"), no_os_cpu_stats_reporting: matches.is_present("no_os_cpu_stats_reporting"), no_os_disk_stats_reporting: matches.is_present("no_os_disk_stats_reporting"), - no_os_open_fd_stats_reporting: matches.is_present("no_os_open_fd_stats_reporting"), poh_pinned_cpu_core: value_of(&matches, "poh_pinned_cpu_core") .unwrap_or(poh_service::DEFAULT_PINNED_CPU_CORE), poh_hashes_per_batch: value_of(&matches, "poh_hashes_per_batch")