From 2a5b0d1b99186acb4e53a87c477eb6688870eed3 Mon Sep 17 00:00:00 2001 From: BodoBolero Date: Tue, 22 Apr 2025 16:02:23 +0200 Subject: [PATCH] remove more metrics, still compiles --- pageserver/src/metrics.rs | 47 --------------- pageserver/src/pgdatadir_mapping.rs | 18 ++---- pageserver/src/tenant/storage_layer/layer.rs | 1 - pageserver/src/tenant/timeline.rs | 59 +------------------ .../walreceiver/connection_manager.rs | 4 -- .../walreceiver/walreceiver_connection.rs | 7 --- 6 files changed, 8 insertions(+), 128 deletions(-) diff --git a/pageserver/src/metrics.rs b/pageserver/src/metrics.rs index 0c3e9ff0a4..51ecb36512 100644 --- a/pageserver/src/metrics.rs +++ b/pageserver/src/metrics.rs @@ -1669,13 +1669,7 @@ pub(crate) struct TimelineMetrics { pub imitate_logical_size_histo: StorageTimeMetrics, pub garbage_collect_histo: StorageTimeMetrics, pub find_gc_cutoffs_histo: StorageTimeMetrics, - pub standby_horizon_gauge: IntGauge, /// copy of LayeredTimeline.current_logical_size - pub current_logical_size_gauge: UIntGauge, - pub aux_file_size_gauge: IntGauge, - pub directory_entries_count_gauge: Lazy UIntGauge>>, - pub evictions: IntCounter, - pub wal_records_received: IntCounter, pub storage_io_size: StorageIoSizeMetrics, shutdown: std::sync::atomic::AtomicBool, } @@ -1732,41 +1726,6 @@ impl TimelineMetrics { &timeline_id, ); - - - let standby_horizon_gauge = STANDBY_HORIZON - .get_metric_with_label_values(&[&tenant_id, &shard_id, &timeline_id]) - .unwrap(); - // TODO: we shouldn't expose this metric - let current_logical_size_gauge = CURRENT_LOGICAL_SIZE - .get_metric_with_label_values(&[&tenant_id, &shard_id, &timeline_id]) - .unwrap(); - let aux_file_size_gauge = AUX_FILE_SIZE - .get_metric_with_label_values(&[&tenant_id, &shard_id, &timeline_id]) - .unwrap(); - // TODO use impl Trait syntax here once we have ability to use it: https://github.com/rust-lang/rust/issues/63065 - let directory_entries_count_gauge_closure = { - let tenant_shard_id = *tenant_shard_id; - let timeline_id_raw = *timeline_id_raw; - move || { - let tenant_id = tenant_shard_id.tenant_id.to_string(); - let shard_id = format!("{}", tenant_shard_id.shard_slug()); - let timeline_id = timeline_id_raw.to_string(); - let gauge: UIntGauge = DIRECTORY_ENTRIES_COUNT - .get_metric_with_label_values(&[&tenant_id, &shard_id, &timeline_id]) - .unwrap(); - gauge - } - }; - let directory_entries_count_gauge: Lazy UIntGauge>> = - Lazy::new(Box::new(directory_entries_count_gauge_closure)); - let evictions = EVICTIONS - .get_metric_with_label_values(&[&tenant_id, &shard_id, &timeline_id]) - .unwrap(); - - let wal_records_received = PAGESERVER_TIMELINE_WAL_RECORDS_RECEIVED - .get_metric_with_label_values(&[&tenant_id, &shard_id, &timeline_id]) - .unwrap(); let storage_io_size = StorageIoSizeMetrics::new(&tenant_id, &shard_id, &timeline_id); @@ -1781,13 +1740,7 @@ impl TimelineMetrics { imitate_logical_size_histo, garbage_collect_histo, find_gc_cutoffs_histo, - standby_horizon_gauge, - current_logical_size_gauge, - aux_file_size_gauge, - directory_entries_count_gauge, - evictions, storage_io_size, - wal_records_received, shutdown: std::sync::atomic::AtomicBool::default(), } } diff --git a/pageserver/src/pgdatadir_mapping.rs b/pageserver/src/pgdatadir_mapping.rs index 81e548a095..ef2a8b5e4d 100644 --- a/pageserver/src/pgdatadir_mapping.rs +++ b/pageserver/src/pgdatadir_mapping.rs @@ -1032,19 +1032,16 @@ impl Timeline { ) .await?; let mut result = HashMap::new(); - let mut sz = 0; + for (_, v) in kv { let v = v?; let v = aux_file::decode_file_value_bytes(&v) .context("value decode") .map_err(PageReconstructError::Other)?; for (fname, content) in v { - sz += fname.len(); - sz += content.len(); result.insert(fname, content); } } - self.aux_file_size_estimator.on_initial(sz); Ok(result) } @@ -2331,20 +2328,15 @@ impl DatadirModification<'_> { } let mut new_files = other_files; match (modifying_file, content.is_empty()) { - (Some(old_content), false) => { - self.tline - .aux_file_size_estimator - .on_update(old_content.len(), content.len()); + (Some(_old_content), false) => { + new_files.push((path, content)); } - (Some(old_content), true) => { - self.tline - .aux_file_size_estimator - .on_remove(old_content.len()); + (Some(_old_content), true) => { + // not adding the file key to the final `new_files` vec. } (None, false) => { - self.tline.aux_file_size_estimator.on_add(content.len()); new_files.push((path, content)); } // Compute may request delete of old version of pgstat AUX file if new one exceeds size limit. diff --git a/pageserver/src/tenant/storage_layer/layer.rs b/pageserver/src/tenant/storage_layer/layer.rs index 1b68fe2d0e..5d3d9a7002 100644 --- a/pageserver/src/tenant/storage_layer/layer.rs +++ b/pageserver/src/tenant/storage_layer/layer.rs @@ -1549,7 +1549,6 @@ impl LayerInner { tracing::info!("evicted layer after unknown residence period"); } } - timeline.metrics.evictions.inc(); timeline .metrics .resident_physical_size_sub(self.desc.file_size); diff --git a/pageserver/src/tenant/timeline.rs b/pageserver/src/tenant/timeline.rs index 835f35ca32..e92aaad976 100644 --- a/pageserver/src/tenant/timeline.rs +++ b/pageserver/src/tenant/timeline.rs @@ -96,7 +96,6 @@ use super::{ AttachedTenantConf, GcError, HeatMapTimeline, MaybeOffloaded, debug_assert_current_span_has_tenant_and_timeline_id, }; -use crate::aux_file::AuxFileSizeEstimator; use crate::config::PageServerConf; use crate::context::{ DownloadBehavior, PerfInstrumentFutureExt, RequestContext, RequestContextBuilder, @@ -414,8 +413,6 @@ pub struct Timeline { /// Cloned from [`super::Tenant::pagestream_throttle`] on construction. pub(crate) pagestream_throttle: Arc, - /// Size estimator for aux file v2 - pub(crate) aux_file_size_estimator: AuxFileSizeEstimator, /// Some test cases directly place keys into the timeline without actually modifying the directory /// keys (i.e., DB_DIR). The test cases creating such keys will put the keyspaces here, so that @@ -2813,7 +2810,6 @@ impl Timeline { &tenant_shard_id, &timeline_id, )); - let aux_file_metrics = metrics.aux_file_size_gauge.clone(); let mut result = Timeline { conf, @@ -2921,8 +2917,6 @@ impl Timeline { pagestream_throttle: resources.pagestream_throttle, - aux_file_size_estimator: AuxFileSizeEstimator::new(aux_file_metrics), - #[cfg(test)] extra_test_dense_keyspace: ArcSwap::new(Arc::new(KeySpace::default())), @@ -3510,18 +3504,6 @@ impl Timeline { ControlFlow::Break(()) => return, }; - // we cannot query current_logical_size.current_size() to know the current - // *negative* value, only truncated to u64. - let added = self - .current_logical_size - .size_added_after_initial - .load(AtomicOrdering::Relaxed); - - let sum = calculated_size.saturating_add_signed(added); - - // set the gauge value before it can be set in `update_current_logical_size`. - self.metrics.current_logical_size_gauge.set(sum); - self.current_logical_size .initial_logical_size .set((calculated_size,)) @@ -3630,21 +3612,6 @@ impl Timeline { let logical_size = &self.current_logical_size; logical_size.increment_size(delta); - // Also set the value in the prometheus gauge. Note that - // there is a race condition here: if this is is called by two - // threads concurrently, the prometheus gauge might be set to - // one value while current_logical_size is set to the - // other. - match logical_size.current_size() { - CurrentLogicalSize::Exact(ref new_current_size) => self - .metrics - .current_logical_size_gauge - .set(new_current_size.into()), - CurrentLogicalSize::Approximate(_) => { - // don't update the gauge yet, this allows us not to update the gauge back and - // forth between the initial size calculation task. - } - } } pub(crate) fn update_directory_entries_count(&self, kind: DirectoryKind, count: MetricsUpdate) { @@ -3682,26 +3649,8 @@ impl Timeline { } }; - // TODO: remove this, there's no place in the code that updates this aux metrics. - let aux_metric = - self.directory_metrics[DirectoryKind::AuxFiles.offset()].load(AtomicOrdering::Relaxed); - - let sum_of_entries = self - .directory_metrics - .iter() - .map(|v| v.load(AtomicOrdering::Relaxed)) - .sum(); - // Set a high general threshold and a lower threshold for the auxiliary files, - // as we can have large numbers of relations in the db directory. - const SUM_THRESHOLD: u64 = 5000; - const AUX_THRESHOLD: u64 = 1000; - if sum_of_entries >= SUM_THRESHOLD || aux_metric >= AUX_THRESHOLD { - self.metrics - .directory_entries_count_gauge - .set(sum_of_entries); - } else if let Some(metric) = Lazy::get(&self.metrics.directory_entries_count_gauge) { - metric.set(sum_of_entries); - } + + } async fn find_layer( @@ -6247,9 +6196,7 @@ impl Timeline { // It is an easy way to unset it when standby disappears without adding // more conf options. self.standby_horizon.store(Lsn::INVALID); - self.metrics - .standby_horizon_gauge - .set(Lsn::INVALID.0 as i64); + let res = self .gc_timeline( diff --git a/pageserver/src/tenant/timeline/walreceiver/connection_manager.rs b/pageserver/src/tenant/timeline/walreceiver/connection_manager.rs index 297dec80b1..b698514be1 100644 --- a/pageserver/src/tenant/timeline/walreceiver/connection_manager.rs +++ b/pageserver/src/tenant/timeline/walreceiver/connection_manager.rs @@ -739,10 +739,6 @@ impl ConnectionManagerState { self.timeline .standby_horizon .store(Lsn(timeline_update.standby_horizon)); - self.timeline - .metrics - .standby_horizon_gauge - .set(timeline_update.standby_horizon as i64); } let new_safekeeper_id = NodeId(timeline_update.safekeeper_id); diff --git a/pageserver/src/tenant/timeline/walreceiver/walreceiver_connection.rs b/pageserver/src/tenant/timeline/walreceiver/walreceiver_connection.rs index 5c9da7709e..357cce4ec2 100644 --- a/pageserver/src/tenant/timeline/walreceiver/walreceiver_connection.rs +++ b/pageserver/src/tenant/timeline/walreceiver/walreceiver_connection.rs @@ -421,13 +421,6 @@ pub(super) async fn handle_walreceiver_connection( Ok(()) } - if !records.is_empty() { - timeline - .metrics - .wal_records_received - .inc_by(records.len() as u64); - } - for interpreted in records { if matches!(interpreted.flush_uncommitted, FlushUncommittedRecords::Yes) && uncommitted_records > 0