From b7db62411b6376ecd9318d3dc74081e96839a523 Mon Sep 17 00:00:00 2001 From: Dmitry Rodionov Date: Tue, 16 May 2023 16:54:29 +0300 Subject: [PATCH] Make storage time operations an enum instead of an array (#4238) Use an enum instead of an array. Before that there was no connection between definition of the metric and point where it was used aside from matching string literals. Now its possible to use IDE features to check for references. Also this allows to avoid mismatch between set of metrics that was defined and set of metrics that was actually used What is interesting is that `init logical size` case is not used. I think `LogicalSize` is a duplicate of `InitLogicalSize`. So removed the latter. --- pageserver/src/http/routes.rs | 4 +-- pageserver/src/metrics.rs | 68 ++++++++++++++++++++++++----------- 2 files changed, 49 insertions(+), 23 deletions(-) diff --git a/pageserver/src/http/routes.rs b/pageserver/src/http/routes.rs index 361c7850d6..26cd02e5ed 100644 --- a/pageserver/src/http/routes.rs +++ b/pageserver/src/http/routes.rs @@ -19,7 +19,7 @@ use super::models::{ }; use crate::context::{DownloadBehavior, RequestContext}; use crate::disk_usage_eviction_task; -use crate::metrics::STORAGE_TIME_GLOBAL; +use crate::metrics::{StorageTimeOperation, STORAGE_TIME_GLOBAL}; use crate::pgdatadir_mapping::LsnForTimestamp; use crate::task_mgr::TaskKind; use crate::tenant::config::TenantConfOpt; @@ -710,7 +710,7 @@ async fn tenant_create_handler(mut request: Request) -> Result = Lazy::new(|| { register_counter_vec!( @@ -673,7 +690,9 @@ pub struct StorageTimeMetrics { } impl StorageTimeMetrics { - pub fn new(operation: &str, tenant_id: &str, timeline_id: &str) -> Self { + pub fn new(operation: StorageTimeOperation, tenant_id: &str, timeline_id: &str) -> Self { + let operation: &'static str = operation.into(); + let timeline_sum = STORAGE_TIME_SUM_PER_TIMELINE .get_metric_with_label_values(&[operation, tenant_id, timeline_id]) .unwrap(); @@ -737,16 +756,23 @@ impl TimelineMetrics { let materialized_page_cache_hit_counter = MATERIALIZED_PAGE_CACHE_HIT .get_metric_with_label_values(&[&tenant_id, &timeline_id]) .unwrap(); - let flush_time_histo = StorageTimeMetrics::new("layer flush", &tenant_id, &timeline_id); - let compact_time_histo = StorageTimeMetrics::new("compact", &tenant_id, &timeline_id); + let flush_time_histo = + StorageTimeMetrics::new(StorageTimeOperation::LayerFlush, &tenant_id, &timeline_id); + let compact_time_histo = + StorageTimeMetrics::new(StorageTimeOperation::Compact, &tenant_id, &timeline_id); let create_images_time_histo = - StorageTimeMetrics::new("create images", &tenant_id, &timeline_id); - let logical_size_histo = StorageTimeMetrics::new("logical size", &tenant_id, &timeline_id); - let imitate_logical_size_histo = - StorageTimeMetrics::new("imitate logical size", &tenant_id, &timeline_id); + StorageTimeMetrics::new(StorageTimeOperation::CreateImages, &tenant_id, &timeline_id); + let logical_size_histo = + StorageTimeMetrics::new(StorageTimeOperation::LogicalSize, &tenant_id, &timeline_id); + let imitate_logical_size_histo = StorageTimeMetrics::new( + StorageTimeOperation::ImitateLogicalSize, + &tenant_id, + &timeline_id, + ); let load_layer_map_histo = - StorageTimeMetrics::new("load layer map", &tenant_id, &timeline_id); - let garbage_collect_histo = StorageTimeMetrics::new("gc", &tenant_id, &timeline_id); + StorageTimeMetrics::new(StorageTimeOperation::LoadLayerMap, &tenant_id, &timeline_id); + let garbage_collect_histo = + StorageTimeMetrics::new(StorageTimeOperation::Gc, &tenant_id, &timeline_id); let last_record_gauge = LAST_RECORD_LSN .get_metric_with_label_values(&[&tenant_id, &timeline_id]) .unwrap(); @@ -814,7 +840,7 @@ impl Drop for TimelineMetrics { .write() .unwrap() .remove(tenant_id, timeline_id); - for op in STORAGE_TIME_OPERATIONS { + for op in StorageTimeOperation::VARIANTS { let _ = STORAGE_TIME_SUM_PER_TIMELINE.remove_label_values(&[op, tenant_id, timeline_id]); let _ =