diff --git a/.github/workflows/build_and_test.yml b/.github/workflows/build_and_test.yml index 9021ac48d9..2b0b0ba2bf 100644 --- a/.github/workflows/build_and_test.yml +++ b/.github/workflows/build_and_test.yml @@ -183,13 +183,12 @@ jobs: # corresponding Cargo.toml files for their descriptions. - name: Set env variables run: | + CARGO_FEATURES="--features testing" if [[ $BUILD_TYPE == "debug" ]]; then cov_prefix="scripts/coverage --profraw-prefix=$GITHUB_JOB --dir=/tmp/coverage run" - CARGO_FEATURES="--features testing" CARGO_FLAGS="--locked $CARGO_FEATURES" elif [[ $BUILD_TYPE == "release" ]]; then cov_prefix="" - CARGO_FEATURES="--features testing,profiling" CARGO_FLAGS="--locked --release $CARGO_FEATURES" fi echo "cov_prefix=${cov_prefix}" >> $GITHUB_ENV diff --git a/Cargo.lock b/Cargo.lock index ad1fc67219..246d481ef9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -66,12 +66,6 @@ dependencies = [ "backtrace", ] -[[package]] -name = "arrayvec" -version = "0.7.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8da52d66c7071e2e3fa2a1e5c6d088fec47b593032b254f5e980de8ea54454d6" - [[package]] name = "asn1-rs" version = "0.5.1" @@ -633,12 +627,6 @@ version = "3.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "572f695136211188308f16ad2ca5c851a712c464060ae6974944458eb83880ba" -[[package]] -name = "bytemuck" -version = "1.12.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "aaa3a8d9a1ca92e282c96a32d6511b695d7d994d1d102ba85d279f9b2756947f" - [[package]] name = "byteorder" version = "1.4.3" @@ -899,7 +887,7 @@ dependencies = [ "clap 4.0.29", "comfy-table", "git-version", - "nix 0.25.1", + "nix", "once_cell", "pageserver_api", "postgres", @@ -934,15 +922,6 @@ version = "0.8.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5827cebf4670468b8772dd191856768aedcb1b0278a04f989f7766351917b9dc" -[[package]] -name = "cpp_demangle" -version = "0.3.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "eeaa953eaad386a53111e47172c2fedba671e5684c8dd601a5f474f4f118710f" -dependencies = [ - "cfg-if", -] - [[package]] name = "cpufeatures" version = "0.2.5" @@ -1066,7 +1045,7 @@ dependencies = [ "crossterm_winapi", "libc", "mio", - "parking_lot 0.12.1", + "parking_lot", "signal-hook", "signal-hook-mio", "winapi", @@ -1176,15 +1155,6 @@ version = "2.3.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "23d8666cb01533c39dde32bcbab8e227b4ed6679b2c925eba05feabea39508fb" -[[package]] -name = "debugid" -version = "0.7.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d6ee87af31d84ef885378aebca32be3d682b0e0dc119d5b4860a2c5bb5046730" -dependencies = [ - "uuid 0.8.2", -] - [[package]] name = "debugid" version = "0.8.0" @@ -1192,7 +1162,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bef552e6f588e446098f6ba40d89ac146c8c7b64aade83c051ee00bb5d2bc18d" dependencies = [ "serde", - "uuid 1.2.2", + "uuid", ] [[package]] @@ -1318,18 +1288,6 @@ dependencies = [ "windows-sys 0.42.0", ] -[[package]] -name = "findshlibs" -version = "0.10.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "40b9e59cd0f7e0806cca4be089683ecb6434e602038df21fe6bf6711b2f07f64" -dependencies = [ - "cc", - "lazy_static", - "libc", - "winapi", -] - [[package]] name = "fixedbitset" version = "0.4.2" @@ -1793,24 +1751,6 @@ dependencies = [ "serde", ] -[[package]] -name = "inferno" -version = "0.10.12" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "de3886428c6400486522cf44b8626e7b94ad794c14390290f2a274dcf728a58f" -dependencies = [ - "ahash", - "atty", - "indexmap", - "itoa", - "lazy_static", - "log", - "num-format", - "quick-xml", - "rgb", - "str_stack", -] - [[package]] name = "inotify" version = "0.9.6" @@ -2037,15 +1977,6 @@ version = "2.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2dffe52ecf27772e601905b7522cb4ef790d2cc203488bbd0e2fe85fcb74566d" -[[package]] -name = "memmap2" -version = "0.5.8" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4b182332558b18d807c4ce1ca8ca983b34c3ee32765e47b3f0f69b90355cc1dc" -dependencies = [ - "libc", -] - [[package]] name = "memoffset" version = "0.6.5" @@ -2113,19 +2044,6 @@ version = "0.8.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e5ce46fe64a9d73be07dcbe690a38ce1b293be448fd8ce1e6c1b8062c9f72c6a" -[[package]] -name = "nix" -version = "0.23.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8f3790c00a0150112de0f4cd161e3d7fc4b2d8a5542ffc35f099a2562aecb35c" -dependencies = [ - "bitflags", - "cc", - "cfg-if", - "libc", - "memoffset 0.6.5", -] - [[package]] name = "nix" version = "0.25.1" @@ -2189,16 +2107,6 @@ dependencies = [ "num-traits", ] -[[package]] -name = "num-format" -version = "0.4.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a652d9771a63711fd3c3deb670acfbe5c30a4072e664d7a3bf5a9e1056ac72c3" -dependencies = [ - "arrayvec", - "itoa", -] - [[package]] name = "num-integer" version = "0.1.45" @@ -2315,7 +2223,7 @@ dependencies = [ "hyper", "itertools", "metrics", - "nix 0.25.1", + "nix", "num-traits", "once_cell", "pageserver_api", @@ -2325,7 +2233,6 @@ dependencies = [ "postgres-types", "postgres_connection", "postgres_ffi", - "pprof", "pq_proto", "rand", "regex", @@ -2369,17 +2276,6 @@ dependencies = [ "workspace_hack", ] -[[package]] -name = "parking_lot" -version = "0.11.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7d17b78036a60663b797adeaee46f5c9dfebb86948d1255007a1d6be0271ff99" -dependencies = [ - "instant", - "lock_api", - "parking_lot_core 0.8.5", -] - [[package]] name = "parking_lot" version = "0.12.1" @@ -2387,21 +2283,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3742b2c103b9f06bc9fff0a37ff4912935851bee6d36f3c02bcc755bcfec228f" dependencies = [ "lock_api", - "parking_lot_core 0.9.5", -] - -[[package]] -name = "parking_lot_core" -version = "0.8.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d76e8e1493bcac0d2766c42737f34458f1c8c50c0d23bcb24ea953affb273216" -dependencies = [ - "cfg-if", - "instant", - "libc", - "redox_syscall", - "smallvec", - "winapi", + "parking_lot_core", ] [[package]] @@ -2604,25 +2486,6 @@ dependencies = [ "workspace_hack", ] -[[package]] -name = "pprof" -version = "0.6.1" -source = "git+https://github.com/neondatabase/pprof-rs.git?branch=wallclock-profiling#4e011a87d22fb4d21d15cc38bce81ff1c75e4bc9" -dependencies = [ - "backtrace", - "cfg-if", - "findshlibs", - "inferno", - "lazy_static", - "libc", - "log", - "nix 0.23.2", - "parking_lot 0.11.2", - "symbolic-demangle", - "tempfile", - "thiserror", -] - [[package]] name = "ppv-lite86" version = "0.2.17" @@ -2717,7 +2580,7 @@ dependencies = [ "lazy_static", "libc", "memchr", - "parking_lot 0.12.1", + "parking_lot", "procfs", "thiserror", ] @@ -2798,7 +2661,7 @@ dependencies = [ "md5", "metrics", "once_cell", - "parking_lot 0.12.1", + "parking_lot", "pin-project-lite", "pq_proto", "rand", @@ -2822,20 +2685,11 @@ dependencies = [ "tracing-subscriber", "url", "utils", - "uuid 1.2.2", + "uuid", "workspace_hack", "x509-parser", ] -[[package]] -name = "quick-xml" -version = "0.22.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8533f14c8382aaad0d592c812ac3b826162128b65662331e1127b45c3d18536b" -dependencies = [ - "memchr", -] - [[package]] name = "quote" version = "1.0.21" @@ -3027,15 +2881,6 @@ dependencies = [ "winreg", ] -[[package]] -name = "rgb" -version = "0.8.34" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3603b7d71ca82644f79b5a06d1220e9a58ede60bd32255f698cb1af8838b8db3" -dependencies = [ - "bytemuck", -] - [[package]] name = "ring" version = "0.16.20" @@ -3216,9 +3061,9 @@ dependencies = [ "humantime", "hyper", "metrics", - "nix 0.25.1", + "nix", "once_cell", - "parking_lot 0.12.1", + "parking_lot", "postgres", "postgres-protocol", "postgres_ffi", @@ -3396,7 +3241,7 @@ version = "0.29.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ccc95faa4078768a6bf8df45e2b894bbf372b3dbbfb364e9429c1c58ab7545c6" dependencies = [ - "debugid 0.8.0", + "debugid", "getrandom", "hex", "serde", @@ -3404,7 +3249,7 @@ dependencies = [ "thiserror", "time", "url", - "uuid 1.2.2", + "uuid", ] [[package]] @@ -3626,7 +3471,7 @@ dependencies = [ "hyper", "metrics", "once_cell", - "parking_lot 0.12.1", + "parking_lot", "prost", "tokio", "tokio-stream", @@ -3637,12 +3482,6 @@ dependencies = [ "workspace_hack", ] -[[package]] -name = "str_stack" -version = "0.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9091b6114800a5f2141aee1d1b9d6ca3592ac062dc5decb3764ec5895a47b4eb" - [[package]] name = "stringprep" version = "0.1.2" @@ -3690,29 +3529,6 @@ version = "0.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8fb1df15f412ee2e9dfc1c504260fa695c1c3f10fe9f4a6ee2d2184d7d6450e2" -[[package]] -name = "symbolic-common" -version = "8.8.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f551f902d5642e58039aee6a9021a61037926af96e071816361644983966f540" -dependencies = [ - "debugid 0.7.3", - "memmap2", - "stable_deref_trait", - "uuid 0.8.2", -] - -[[package]] -name = "symbolic-demangle" -version = "8.8.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4564ca7b4e6eb14105aa8bbbce26e080f6b5d9c4373e67167ab31f7b86443750" -dependencies = [ - "cpp_demangle", - "rustc-demangle", - "symbolic-common", -] - [[package]] name = "syn" version = "1.0.105" @@ -3923,7 +3739,7 @@ dependencies = [ "futures-channel", "futures-util", "log", - "parking_lot 0.12.1", + "parking_lot", "percent-encoding", "phf", "pin-project-lite", @@ -4314,7 +4130,7 @@ dependencies = [ "hyper", "jsonwebtoken", "metrics", - "nix 0.25.1", + "nix", "once_cell", "pq_proto", "rand", @@ -4338,12 +4154,6 @@ dependencies = [ "workspace_hack", ] -[[package]] -name = "uuid" -version = "0.8.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bc5cf98d8186244414c848017f0e2676b3fcb46807f6668a97dfe67359a3c4b7" - [[package]] name = "uuid" version = "1.2.2" @@ -4658,7 +4468,6 @@ dependencies = [ name = "workspace_hack" version = "0.1.0" dependencies = [ - "ahash", "anyhow", "bytes", "chrono", @@ -4686,7 +4495,6 @@ dependencies = [ "serde", "serde_json", "socket2", - "stable_deref_trait", "syn", "tokio", "tokio-util", diff --git a/pageserver/Cargo.toml b/pageserver/Cargo.toml index 8f112fa670..1854b6762f 100644 --- a/pageserver/Cargo.toml +++ b/pageserver/Cargo.toml @@ -10,8 +10,6 @@ default = [] # which adds some runtime cost to run tests on outage conditions testing = ["fail/failpoints"] -profiling = ["pprof"] - [dependencies] amplify_num = { git = "https://github.com/hlinnaka/rust-amplify.git", branch = "unsigned-int-perf" } anyhow = { version = "1.0", features = ["backtrace"] } @@ -40,7 +38,6 @@ pin-project-lite = "0.2.7" postgres = { git = "https://github.com/neondatabase/rust-postgres.git", rev="43e6db254a97fdecbce33d8bc0890accfd74495e" } postgres-protocol = { git = "https://github.com/neondatabase/rust-postgres.git", rev="43e6db254a97fdecbce33d8bc0890accfd74495e" } postgres-types = { git = "https://github.com/neondatabase/rust-postgres.git", rev="43e6db254a97fdecbce33d8bc0890accfd74495e" } -pprof = { git = "https://github.com/neondatabase/pprof-rs.git", branch = "wallclock-profiling", features = ["flamegraph"], optional = true } rand = "0.8.3" regex = "1.4.5" rstar = "0.9.3" diff --git a/pageserver/src/bin/pageserver.rs b/pageserver/src/bin/pageserver.rs index a124bf85c2..18ec1ac68b 100644 --- a/pageserver/src/bin/pageserver.rs +++ b/pageserver/src/bin/pageserver.rs @@ -13,7 +13,7 @@ use tracing::*; use metrics::set_build_info_metric; use pageserver::{ config::{defaults::*, PageServerConf}, - http, page_cache, page_service, profiling, task_mgr, + http, page_cache, page_service, task_mgr, task_mgr::TaskKind, task_mgr::{ BACKGROUND_RUNTIME, COMPUTE_REQUEST_RUNTIME, MGMT_REQUEST_RUNTIME, WALRECEIVER_RUNTIME, @@ -40,8 +40,6 @@ const FEATURES: &[&str] = &[ "testing", #[cfg(feature = "fail/failpoints")] "fail/failpoints", - #[cfg(feature = "profiling")] - "profiling", ]; fn version() -> String { @@ -247,9 +245,6 @@ fn start_pageserver(conf: &'static PageServerConf) -> anyhow::Result<()> { // Install signal handlers let signals = signals::install_shutdown_handlers()?; - // Start profiler (if enabled) - let profiler_guard = profiling::init_profiler(conf); - // Launch broker client WALRECEIVER_RUNTIME.block_on(pageserver::walreceiver::init_broker_client(conf))?; @@ -372,7 +367,6 @@ fn start_pageserver(conf: &'static PageServerConf) -> anyhow::Result<()> { "Got {}. Terminating in immediate shutdown mode", signal.name() ); - profiling::exit_profiler(conf, &profiler_guard); std::process::exit(111); } @@ -381,7 +375,6 @@ fn start_pageserver(conf: &'static PageServerConf) -> anyhow::Result<()> { "Got {}. Terminating gracefully in fast shutdown mode", signal.name() ); - profiling::exit_profiler(conf, &profiler_guard); BACKGROUND_RUNTIME.block_on(pageserver::shutdown_pageserver(0)); unreachable!() } diff --git a/pageserver/src/config.rs b/pageserver/src/config.rs index deb79531a4..7b99d98581 100644 --- a/pageserver/src/config.rs +++ b/pageserver/src/config.rs @@ -138,7 +138,6 @@ pub struct PageServerConf { pub auth_validation_public_key_path: Option, pub remote_storage_config: Option, - pub profiling: ProfilingConfig, pub default_tenant_conf: TenantConf, /// Storage broker endpoints to connect to. @@ -165,25 +164,6 @@ pub struct PageServerConf { /// startup code to the connection code through a dozen layers. pub static SAFEKEEPER_AUTH_TOKEN: OnceCell> = OnceCell::new(); -#[derive(Debug, Clone, PartialEq, Eq)] -pub enum ProfilingConfig { - Disabled, - PageRequests, -} - -impl FromStr for ProfilingConfig { - type Err = anyhow::Error; - - fn from_str(s: &str) -> Result { - let result = match s { - "disabled" => ProfilingConfig::Disabled, - "page_requests" => ProfilingConfig::PageRequests, - _ => bail!("invalid value \"{s}\" for profiling option, valid values are \"disabled\" and \"page_requests\""), - }; - Ok(result) - } -} - // use dedicated enum for builder to better indicate the intention // and avoid possible confusion with nested options pub enum BuilderValue { @@ -226,7 +206,6 @@ struct PageServerConfigBuilder { id: BuilderValue, - profiling: BuilderValue, broker_endpoint: BuilderValue, broker_keepalive_interval: BuilderValue, @@ -262,7 +241,6 @@ impl Default for PageServerConfigBuilder { auth_validation_public_key_path: Set(None), remote_storage_config: Set(None), id: NotSet, - profiling: Set(ProfilingConfig::Disabled), broker_endpoint: Set(storage_broker::DEFAULT_ENDPOINT .parse() .expect("failed to parse default broker endpoint")), @@ -348,10 +326,6 @@ impl PageServerConfigBuilder { self.id = BuilderValue::Set(node_id) } - pub fn profiling(&mut self, profiling: ProfilingConfig) { - self.profiling = BuilderValue::Set(profiling) - } - pub fn log_format(&mut self, log_format: LogFormat) { self.log_format = BuilderValue::Set(log_format) } @@ -405,7 +379,6 @@ impl PageServerConfigBuilder { .remote_storage_config .ok_or(anyhow!("missing remote_storage_config"))?, id: self.id.ok_or(anyhow!("missing id"))?, - profiling: self.profiling.ok_or(anyhow!("missing profiling"))?, // TenantConf is handled separately default_tenant_conf: TenantConf::default(), broker_endpoint: self @@ -588,7 +561,6 @@ impl PageServerConf { t_conf = Self::parse_toml_tenant_conf(item)?; } "id" => builder.id(NodeId(parse_toml_u64(key, item)?)), - "profiling" => builder.profiling(parse_toml_from_str(key, item)?), "broker_endpoint" => builder.broker_endpoint(parse_toml_string(key, item)?.parse().context("failed to parse broker endpoint")?), "broker_keepalive_interval" => builder.broker_keepalive_interval(parse_toml_duration(key, item)?), "log_format" => builder.log_format( @@ -722,7 +694,6 @@ impl PageServerConf { auth_type: AuthType::Trust, auth_validation_public_key_path: None, remote_storage_config: None, - profiling: ProfilingConfig::Disabled, default_tenant_conf: TenantConf::default(), broker_endpoint: storage_broker::DEFAULT_ENDPOINT.parse().unwrap(), broker_keepalive_interval: Duration::from_secs(5000), @@ -898,7 +869,6 @@ log_format = 'json' auth_type: AuthType::Trust, auth_validation_public_key_path: None, remote_storage_config: None, - profiling: ProfilingConfig::Disabled, default_tenant_conf: TenantConf::default(), broker_endpoint: storage_broker::DEFAULT_ENDPOINT.parse().unwrap(), broker_keepalive_interval: humantime::parse_duration( @@ -949,7 +919,6 @@ log_format = 'json' auth_type: AuthType::Trust, auth_validation_public_key_path: None, remote_storage_config: None, - profiling: ProfilingConfig::Disabled, default_tenant_conf: TenantConf::default(), broker_endpoint: storage_broker::DEFAULT_ENDPOINT.parse().unwrap(), broker_keepalive_interval: Duration::from_secs(5), diff --git a/pageserver/src/lib.rs b/pageserver/src/lib.rs index 2f78c199b9..91cde477ad 100644 --- a/pageserver/src/lib.rs +++ b/pageserver/src/lib.rs @@ -9,7 +9,6 @@ pub(crate) mod metrics; pub mod page_cache; pub mod page_service; pub mod pgdatadir_mapping; -pub mod profiling; pub mod repository; pub mod task_mgr; pub mod tenant; diff --git a/pageserver/src/page_service.rs b/pageserver/src/page_service.rs index 5393fca780..f123168211 100644 --- a/pageserver/src/page_service.rs +++ b/pageserver/src/page_service.rs @@ -39,10 +39,9 @@ use utils::{ use crate::auth::check_permission; use crate::basebackup; -use crate::config::{PageServerConf, ProfilingConfig}; +use crate::config::PageServerConf; use crate::import_datadir::import_wal_from_tar; use crate::metrics::{LIVE_CONNECTIONS_COUNT, SMGR_QUERY_TIME}; -use crate::profiling::profpoint_start; use crate::task_mgr; use crate::task_mgr::TaskKind; use crate::tenant::mgr; @@ -250,7 +249,7 @@ impl PageRequestMetrics { #[derive(Debug)] struct PageServerHandler { - conf: &'static PageServerConf, + _conf: &'static PageServerConf, auth: Option>, claims: Option, } @@ -258,7 +257,7 @@ struct PageServerHandler { impl PageServerHandler { pub fn new(conf: &'static PageServerConf, auth: Option>) -> Self { PageServerHandler { - conf, + _conf: conf, auth, claims: None, } @@ -604,10 +603,6 @@ impl PageServerHandler { */ let page = crate::tenant::with_ondemand_download(|| { - // FIXME: this profiling now happens at different place than it used to. The - // current profiling is based on a thread-local variable, so it doesn't work - // across awaits - let _profiling_guard = profpoint_start(self.conf, ProfilingConfig::PageRequests); timeline.get_rel_page_at_lsn(req.rel, req.blkno, lsn, req.latest) }) .await?; diff --git a/pageserver/src/profiling.rs b/pageserver/src/profiling.rs deleted file mode 100644 index ad896cfa30..0000000000 --- a/pageserver/src/profiling.rs +++ /dev/null @@ -1,107 +0,0 @@ -//! -//! Support for profiling -//! -//! This relies on a modified version of the 'pprof-rs' crate. That's not very -//! nice, so to avoid a hard dependency on that, this is an optional feature. -//! -use crate::config::{PageServerConf, ProfilingConfig}; - -/// The actual implementation is in the `profiling_impl` submodule. If the profiling -/// feature is not enabled, it's just a dummy implementation that panics if you -/// try to enabled profiling in the configuration. -pub use profiling_impl::*; - -#[cfg(feature = "profiling")] -mod profiling_impl { - use super::*; - use pprof; - use std::marker::PhantomData; - - /// Start profiling the current thread. Returns a guard object; - /// the profiling continues until the guard is dropped. - /// - /// Note: profiling is not re-entrant. If you call 'profpoint_start' while - /// profiling is already started, nothing happens, and the profiling will be - /// stopped when either guard object is dropped. - #[inline] - pub fn profpoint_start( - conf: &crate::config::PageServerConf, - point: ProfilingConfig, - ) -> Option { - if conf.profiling == point { - pprof::start_profiling(); - Some(ProfilingGuard(PhantomData)) - } else { - None - } - } - - /// A hack to remove Send and Sync from the ProfilingGuard. Because the - /// profiling is attached to current thread. - //// - /// See comments in https://github.com/rust-lang/rust/issues/68318 - type PhantomUnsend = std::marker::PhantomData<*mut u8>; - - pub struct ProfilingGuard(PhantomUnsend); - - impl Drop for ProfilingGuard { - fn drop(&mut self) { - pprof::stop_profiling(); - } - } - - /// Initialize the profiler. This must be called before any 'profpoint_start' calls. - pub fn init_profiler(conf: &PageServerConf) -> Option { - if conf.profiling != ProfilingConfig::Disabled { - Some(pprof::ProfilerGuardBuilder::default().build().unwrap()) - } else { - None - } - } - - /// Exit the profiler. Writes the flamegraph to current workdir. - pub fn exit_profiler(_conf: &PageServerConf, profiler_guard: &Option) { - // Write out the flamegraph - if let Some(profiler_guard) = profiler_guard { - if let Ok(report) = profiler_guard.report().build() { - // this gets written under the workdir - let file = std::fs::File::create("flamegraph.svg").unwrap(); - let mut options = pprof::flamegraph::Options::default(); - options.image_width = Some(2500); - report.flamegraph_with_options(file, &mut options).unwrap(); - } - } - } -} - -/// Dummy implementation when compiling without profiling feature or for non-linux OSes. -#[cfg(not(feature = "profiling"))] -mod profiling_impl { - use super::*; - - pub struct DummyProfilerGuard; - - impl Drop for DummyProfilerGuard { - fn drop(&mut self) { - // do nothing, this exists to calm Clippy down - } - } - - pub fn profpoint_start( - _conf: &PageServerConf, - _point: ProfilingConfig, - ) -> Option { - None - } - - pub fn init_profiler(conf: &PageServerConf) -> Option { - if conf.profiling != ProfilingConfig::Disabled { - // shouldn't happen, we don't allow profiling in the config if the support - // for it is disabled. - panic!("profiling enabled but the binary was compiled without profiling support"); - } - None - } - - pub fn exit_profiler(_conf: &PageServerConf, _guard: &Option) {} -} diff --git a/run_clippy.sh b/run_clippy.sh index bf770432d0..fe0e745d7d 100755 --- a/run_clippy.sh +++ b/run_clippy.sh @@ -9,8 +9,8 @@ # In vscode, this setting is Rust-analyzer>Check On Save:Command -# Not every feature is supported in macOS builds, e.g. `profiling`, -# avoid running regular linting script that checks every feature. +# Not every feature is supported in macOS builds. Avoid running regular linting +# script that checks every feature. if [[ "$OSTYPE" == "darwin"* ]]; then # no extra features to test currently, add more here when needed cargo clippy --locked --all --all-targets --features testing -- -A unknown_lints -D warnings diff --git a/test_runner/fixtures/neon_fixtures.py b/test_runner/fixtures/neon_fixtures.py index 5b00ebdea7..705ab70ab4 100644 --- a/test_runner/fixtures/neon_fixtures.py +++ b/test_runner/fixtures/neon_fixtures.py @@ -1980,10 +1980,6 @@ class NeonPageserver(PgProtocol): if '"testing"' not in self.version: pytest.skip("pageserver was built without 'testing' feature") - def is_profiling_enabled_or_skip(self): - if '"profiling"' not in self.version: - pytest.skip("pageserver was built without 'profiling' feature") - def http_client(self, auth_token: Optional[str] = None) -> PageserverHttpClient: return PageserverHttpClient( port=self.service_port.http, diff --git a/test_runner/fixtures/utils.py b/test_runner/fixtures/utils.py index 1fb9eb72e6..df83fc6377 100644 --- a/test_runner/fixtures/utils.py +++ b/test_runner/fixtures/utils.py @@ -148,7 +148,7 @@ def get_scale_for_db(size_mb: int) -> int: ATTACHMENT_NAME_REGEX: re.Pattern = re.compile( # type: ignore[type-arg] - r"flamegraph\.svg|regression\.diffs|.+\.(?:log|stderr|stdout|filediff|metrics|html)" + r"regression\.diffs|.+\.(?:log|stderr|stdout|filediff|metrics|html)" ) diff --git a/test_runner/performance/README.md b/test_runner/performance/README.md index a32ce87c33..c1a57fb28b 100644 --- a/test_runner/performance/README.md +++ b/test_runner/performance/README.md @@ -1,12 +1,8 @@ # Running locally -First make a release build. The profiling flag is optional, used only for tests that -generate flame graphs. The `-s` flag just silences a lot of output, and makes it +First make a release build. The `-s` flag silences a lot of output, and makes it easier to see if you have compile errors without scrolling up. -`BUILD_TYPE=release CARGO_BUILD_FLAGS="--features=testing,profiling" make -s -j8` - -NOTE: the `profiling` flag only works on linux because we use linux-specific -libc APIs like `libc::timer_t`. +`BUILD_TYPE=release CARGO_BUILD_FLAGS="--features=testing" make -s -j8` Then run the tests `NEON_BIN=./target/release poetry run pytest test_runner/performance"` diff --git a/test_runner/performance/test_perf_pgbench.py b/test_runner/performance/test_perf_pgbench.py index 50e5366c1e..2b8760dff2 100644 --- a/test_runner/performance/test_perf_pgbench.py +++ b/test_runner/performance/test_perf_pgbench.py @@ -8,7 +8,7 @@ from typing import Dict, List import pytest from fixtures.benchmark_fixture import MetricReport, PgBenchInitResult, PgBenchRunResult -from fixtures.compare_fixtures import NeonCompare, PgCompare +from fixtures.compare_fixtures import PgCompare from fixtures.utils import get_scale_for_db @@ -176,28 +176,6 @@ def test_pgbench(neon_with_baseline: PgCompare, scale: int, duration: int): run_test_pgbench(neon_with_baseline, scale, duration, PgBenchLoadType.SELECT_ONLY) -# Run the pgbench tests, and generate a flamegraph from it -# This requires that the pageserver was built with the 'profiling' feature. -# -# TODO: If the profiling is cheap enough, there's no need to run the same test -# twice, with and without profiling. But for now, run it separately, so that we -# can see how much overhead the profiling adds. -@pytest.mark.parametrize("scale", get_scales_matrix()) -@pytest.mark.parametrize("duration", get_durations_matrix()) -def test_pgbench_flamegraph(zenbenchmark, pg_bin, neon_env_builder, scale: int, duration: int): - neon_env_builder.pageserver_config_override = """ -profiling="page_requests" -""" - env = neon_env_builder.init_start() - env.pageserver.is_profiling_enabled_or_skip() - env.neon_cli.create_branch("empty", "main") - - neon_compare = NeonCompare(zenbenchmark, env, pg_bin, "pgbench") - run_test_pgbench(neon_compare, scale, duration, PgBenchLoadType.INIT) - run_test_pgbench(neon_compare, scale, duration, PgBenchLoadType.SIMPLE_UPDATE) - run_test_pgbench(neon_compare, scale, duration, PgBenchLoadType.SELECT_ONLY) - - # The following 3 tests run on an existing database as it was set up by previous tests, # and leaves the database in a state that would be used in the next tests. # Modifying the definition order of these functions or adding other remote tests in between will alter results. diff --git a/workspace_hack/Cargo.toml b/workspace_hack/Cargo.toml index 4c7fbd8333..989cc9202e 100644 --- a/workspace_hack/Cargo.toml +++ b/workspace_hack/Cargo.toml @@ -13,7 +13,6 @@ publish = false ### BEGIN HAKARI SECTION [dependencies] -ahash = { version = "0.7", features = ["std"] } anyhow = { version = "1", features = ["backtrace", "std"] } bytes = { version = "1", features = ["serde", "std"] } chrono = { version = "0.4", default-features = false, features = ["clock", "iana-time-zone", "serde", "std", "winapi"] } @@ -41,7 +40,6 @@ scopeguard = { version = "1", features = ["use_std"] } serde = { version = "1", features = ["alloc", "derive", "serde_derive", "std"] } serde_json = { version = "1", features = ["raw_value", "std"] } socket2 = { version = "0.4", default-features = false, features = ["all"] } -stable_deref_trait = { version = "1", features = ["alloc", "std"] } tokio = { version = "1", features = ["bytes", "fs", "io-std", "io-util", "libc", "macros", "memchr", "mio", "net", "num_cpus", "once_cell", "process", "rt", "rt-multi-thread", "signal-hook-registry", "socket2", "sync", "time", "tokio-macros"] } tokio-util = { version = "0.7", features = ["codec", "io", "io-util", "tracing"] } tower = { version = "0.4", features = ["__common", "balance", "buffer", "discover", "futures-core", "futures-util", "indexmap", "limit", "load", "log", "make", "pin-project", "pin-project-lite", "rand", "ready-cache", "retry", "slab", "timeout", "tokio", "tokio-util", "tracing", "util"] } @@ -50,7 +48,6 @@ tracing-core = { version = "0.1", features = ["once_cell", "std"] } url = { version = "2", features = ["serde"] } [build-dependencies] -ahash = { version = "0.7", features = ["std"] } anyhow = { version = "1", features = ["backtrace", "std"] } bytes = { version = "1", features = ["serde", "std"] } either = { version = "1", features = ["use_std"] }