diff --git a/Cargo.lock b/Cargo.lock index c112c05188..87f8a4226f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2140,6 +2140,7 @@ dependencies = [ "num-traits", "once_cell", "pageserver_api", + "pin-project-lite", "postgres", "postgres-protocol", "postgres-types", diff --git a/pageserver/Cargo.toml b/pageserver/Cargo.toml index 71ea68071f..e81a631a85 100644 --- a/pageserver/Cargo.toml +++ b/pageserver/Cargo.toml @@ -35,6 +35,7 @@ itertools = "0.10.3" nix = "0.25" num-traits = "0.2.15" once_cell = "1.13.0" +pin-project-lite = "0.2.7" postgres = { git = "https://github.com/neondatabase/rust-postgres.git", rev="d052ee8b86fff9897c77b0fe89ea9daba0e1fa38" } postgres-protocol = { git = "https://github.com/neondatabase/rust-postgres.git", rev="d052ee8b86fff9897c77b0fe89ea9daba0e1fa38" } postgres-types = { git = "https://github.com/neondatabase/rust-postgres.git", rev="d052ee8b86fff9897c77b0fe89ea9daba0e1fa38" } diff --git a/pageserver/src/lib.rs b/pageserver/src/lib.rs index 49725deff6..2d5b66f575 100644 --- a/pageserver/src/lib.rs +++ b/pageserver/src/lib.rs @@ -3,7 +3,7 @@ pub mod config; pub mod http; pub mod import_datadir; pub mod keyspace; -pub mod metrics; +pub(crate) mod metrics; pub mod page_cache; pub mod page_service; pub mod pgdatadir_mapping; diff --git a/pageserver/src/metrics.rs b/pageserver/src/metrics.rs index 43586b926d..c5415a0b55 100644 --- a/pageserver/src/metrics.rs +++ b/pageserver/src/metrics.rs @@ -1,9 +1,8 @@ use metrics::core::{AtomicU64, GenericCounter}; use metrics::{ - register_gauge_vec, register_histogram, register_histogram_vec, register_int_counter, - register_int_counter_vec, register_int_gauge, register_int_gauge_vec, register_uint_gauge_vec, - GaugeVec, Histogram, HistogramVec, IntCounter, IntCounterVec, IntGauge, IntGaugeVec, UIntGauge, - UIntGaugeVec, + register_histogram, register_histogram_vec, register_int_counter, register_int_counter_vec, + register_int_gauge, register_int_gauge_vec, register_uint_gauge_vec, Histogram, HistogramVec, + IntCounter, IntCounterVec, IntGauge, IntGaugeVec, UIntGauge, UIntGaugeVec, }; use once_cell::sync::Lazy; use utils::id::{TenantId, TimelineId}; @@ -200,63 +199,56 @@ pub static NUM_ONDISK_LAYERS: Lazy = Lazy::new(|| { .expect("failed to define a metric") }); -pub static REMAINING_SYNC_ITEMS: Lazy = Lazy::new(|| { - register_int_gauge!( - "pageserver_remote_storage_remaining_sync_items", - "Number of storage sync items left in the queue" - ) - .expect("failed to register pageserver remote storage remaining sync items int gauge") -}); - -pub static IMAGE_SYNC_TIME: Lazy = Lazy::new(|| { - register_gauge_vec!( - "pageserver_remote_storage_image_sync_duration", - "Time spent to synchronize (up/download) a whole pageserver image", +// remote storage metrics +pub static REMOTE_UPLOAD_QUEUE_ITEMS: Lazy = Lazy::new(|| { + register_int_gauge_vec!( + "pageserver_remote_upload_queue_items", + "Number of items left in the upload queue", &["tenant_id", "timeline_id"], ) - .expect("failed to register per-timeline pageserver image sync time vec") + .expect("failed to define a metric") }); -pub static IMAGE_SYNC_OPERATION_KINDS: &[&str] = &["upload", "download", "delete"]; -pub static IMAGE_SYNC_STATUS: &[&str] = &["success", "failure", "abort"]; +pub enum RemoteOpKind { + Upload, + Download, + Delete, +} +impl RemoteOpKind { + fn as_str(&self) -> &str { + match self { + Self::Upload => "upload", + Self::Download => "download", + Self::Delete => "delete", + } + } +} -pub static IMAGE_SYNC_COUNT: Lazy = Lazy::new(|| { - register_int_counter_vec!( - "pageserver_remote_storage_image_sync_count", - "Number of synchronization operations executed for pageserver images. \ - Grouped by tenant, timeline, operation_kind and status", - &["tenant_id", "timeline_id", "operation_kind", "status"] - ) - .expect("failed to register pageserver image sync count vec") -}); +pub enum RemoteOpFileKind { + Layer, + Index, +} +impl RemoteOpFileKind { + fn as_str(&self) -> &str { + match self { + Self::Layer => "layer", + Self::Index => "index", + } + } +} -pub static IMAGE_SYNC_TIME_HISTOGRAM: Lazy = Lazy::new(|| { +pub static REMOTE_OPERATION_KINDS: &[&str] = &["upload", "download", "delete"]; +pub static REMOTE_OPERATION_FILE_KINDS: &[&str] = &["layer", "index"]; +pub static REMOTE_OPERATION_STATUSES: &[&str] = &["success", "failure"]; + +pub static REMOTE_OPERATION_TIME: Lazy = Lazy::new(|| { register_histogram_vec!( - "pageserver_remote_storage_image_sync_seconds", - "Time took to synchronize (download or upload) a whole pageserver image. \ - Grouped by operation_kind and status", - &["operation_kind", "status"], - vec![0.01, 0.025, 0.05, 0.1, 0.25, 0.5, 1.0, 3.0, 10.0, 20.0] + "pageserver_remote_operation_seconds", + "Time spent on remote storage operations. \ + Grouped by tenant, timeline, operation_kind and status", + &["tenant_id", "timeline_id", "file_kind", "op_kind", "status"] ) - .expect("failed to register pageserver image sync time histogram vec") -}); - -pub static REMOTE_INDEX_UPLOAD: Lazy = Lazy::new(|| { - register_int_counter_vec!( - "pageserver_remote_storage_remote_index_uploads_total", - "Number of remote index uploads", - &["tenant_id", "timeline_id"], - ) - .expect("failed to register pageserver remote index upload vec") -}); - -pub static NO_LAYERS_UPLOAD: Lazy = Lazy::new(|| { - register_int_counter_vec!( - "pageserver_remote_storage_no_layers_uploads_total", - "Number of skipped uploads due to no layers", - &["tenant_id", "timeline_id"], - ) - .expect("failed to register pageserver no layers upload vec") + .expect("failed to define a metric") }); pub static TENANT_TASK_EVENTS: Lazy = Lazy::new(|| { @@ -473,16 +465,90 @@ impl Drop for TimelineMetrics { let _ = SMGR_QUERY_TIME.remove_label_values(&[op, tenant_id, timeline_id]); } - for op in IMAGE_SYNC_OPERATION_KINDS { - for status in IMAGE_SYNC_STATUS { - let _ = IMAGE_SYNC_COUNT.remove_label_values(&[tenant_id, timeline_id, op, status]); + let _ = REMOTE_UPLOAD_QUEUE_ITEMS.remove_label_values(&[tenant_id, timeline_id]); + for file_kind in REMOTE_OPERATION_FILE_KINDS { + for op in REMOTE_OPERATION_KINDS { + for status in REMOTE_OPERATION_STATUSES { + let _ = REMOTE_OPERATION_TIME.remove_label_values(&[ + tenant_id, + timeline_id, + file_kind, + op, + status, + ]); + } } } - - let _ = IMAGE_SYNC_TIME.remove_label_values(&[tenant_id, timeline_id]); } } pub fn remove_tenant_metrics(tenant_id: &TenantId) { let _ = STORAGE_TIME.remove_label_values(&["gc", &tenant_id.to_string(), "-"]); } + +use futures::Future; +use pin_project_lite::pin_project; +use std::pin::Pin; +use std::task::{Context, Poll}; +use std::time::Instant; + +/// Wrapper future that measures the time spent by a remote storage operation, +/// and records the time and success/failure as a prometheus metric. +pub trait MeasureRemoteOp: Sized { + fn measure_remote_op( + self, + tenant_id: TenantId, + timeline_id: TimelineId, + file_kind: RemoteOpFileKind, + op: RemoteOpKind, + ) -> MeasuredRemoteOp { + let start = Instant::now(); + MeasuredRemoteOp { + inner: self, + tenant_id, + timeline_id, + file_kind, + op, + start, + } + } +} + +impl MeasureRemoteOp for T {} + +pin_project! { + pub struct MeasuredRemoteOp + { + #[pin] + inner: F, + tenant_id: TenantId, + timeline_id: TimelineId, + file_kind: RemoteOpFileKind, + op: RemoteOpKind, + start: Instant, + } +} + +impl>, O, E> Future for MeasuredRemoteOp { + type Output = Result; + + fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { + let this = self.project(); + let poll_result = this.inner.poll(cx); + if let Poll::Ready(ref res) = poll_result { + let duration = this.start.elapsed(); + let status = if res.is_ok() { &"success" } else { &"failure" }; + REMOTE_OPERATION_TIME + .get_metric_with_label_values(&[ + &this.tenant_id.to_string(), + &this.timeline_id.to_string(), + this.file_kind.as_str(), + this.op.as_str(), + status, + ]) + .unwrap() + .observe(duration.as_secs_f64()); + } + poll_result + } +} diff --git a/pageserver/src/storage_sync.rs b/pageserver/src/storage_sync.rs index 14f366042b..7cd41c8440 100644 --- a/pageserver/src/storage_sync.rs +++ b/pageserver/src/storage_sync.rs @@ -147,6 +147,9 @@ use utils::lsn::Lsn; use self::index::IndexPart; +use crate::metrics::MeasureRemoteOp; +use crate::metrics::RemoteOpFileKind; +use crate::metrics::RemoteOpKind; use crate::{ config::PageServerConf, storage_sync::index::{LayerFileMetadata, RelativePath}, @@ -708,7 +711,14 @@ impl RemoteTimelineClient { loop { let upload_result: anyhow::Result<()> = match task.op { UploadOp::UploadLayer(ref path, ref layer_metadata) => { - upload::upload_timeline_layer(&self.storage_impl, path, layer_metadata).await + upload::upload_timeline_layer(&self.storage_impl, path, layer_metadata) + .measure_remote_op( + self.tenant_id, + self.timeline_id, + RemoteOpFileKind::Layer, + RemoteOpKind::Upload, + ) + .await } UploadOp::UploadMetadata(ref index_part, _lsn) => { upload::upload_index_part( @@ -718,9 +728,24 @@ impl RemoteTimelineClient { self.timeline_id, index_part, ) + .measure_remote_op( + self.tenant_id, + self.timeline_id, + RemoteOpFileKind::Index, + RemoteOpKind::Upload, + ) .await } - UploadOp::Delete(ref path) => delete::delete_layer(&self.storage_impl, path).await, + UploadOp::Delete(ref path) => { + delete::delete_layer(&self.storage_impl, path) + .measure_remote_op( + self.tenant_id, + self.timeline_id, + RemoteOpFileKind::Layer, + RemoteOpKind::Delete, + ) + .await + } UploadOp::Barrier(_) => { // unreachable. Barrier operations are handled synchronously in // launch_queued_tasks diff --git a/pageserver/src/storage_sync/download.rs b/pageserver/src/storage_sync/download.rs index 32b515a141..fcd3f7dbef 100644 --- a/pageserver/src/storage_sync/download.rs +++ b/pageserver/src/storage_sync/download.rs @@ -9,6 +9,7 @@ use tokio::io::AsyncWriteExt; use tracing::debug; use crate::config::PageServerConf; +use crate::metrics::{MeasureRemoteOp, RemoteOpFileKind, RemoteOpKind}; use crate::storage_sync::index::LayerFileMetadata; use remote_storage::{DownloadError, GenericRemoteStorage}; use utils::crashsafe::path_with_suffix_extension; @@ -34,7 +35,14 @@ pub async fn download_layer_file<'a>( path: &'a RelativePath, layer_metadata: &'a LayerFileMetadata, ) -> anyhow::Result { - download_layer_file_guts(conf, storage, tenant_id, timeline_id, path, layer_metadata).await + download_layer_file_guts(conf, storage, tenant_id, timeline_id, path, layer_metadata) + .measure_remote_op( + tenant_id, + timeline_id, + RemoteOpFileKind::Layer, + RemoteOpKind::Download, + ) + .await } const TEMP_DOWNLOAD_EXTENSION: &str = "temp_download"; @@ -267,5 +275,11 @@ pub async fn download_index_part( Ok(index_part) } + .measure_remote_op( + tenant_id, + timeline_id, + RemoteOpFileKind::Index, + RemoteOpKind::Download, + ) .await }