From 00dfb2702a00fe67830269298e1c71478daf2f64 Mon Sep 17 00:00:00 2001 From: Christian Schwarz Date: Thu, 2 Feb 2023 14:32:53 +0100 Subject: [PATCH] replace Option::None with LayerAccessStatsReset::NoReset --- pageserver/src/http/routes.rs | 3 +- pageserver/src/tenant/storage_layer.rs | 31 ++++++++++--------- .../src/tenant/storage_layer/delta_layer.rs | 14 +++------ .../src/tenant/storage_layer/image_layer.rs | 12 ++----- .../src/tenant/storage_layer/remote_layer.rs | 6 ++-- pageserver/src/tenant/timeline.rs | 2 +- 6 files changed, 31 insertions(+), 37 deletions(-) diff --git a/pageserver/src/http/routes.rs b/pageserver/src/http/routes.rs index e0e71e8ba3..935ce8cf36 100644 --- a/pageserver/src/http/routes.rs +++ b/pageserver/src/http/routes.rs @@ -536,7 +536,8 @@ async fn layer_map_info_handler(request: Request) -> Result let timeline_id: TimelineId = parse_request_param(&request, "timeline_id")?; check_permission(&request, Some(tenant_id))?; - let reset: Option = parse_query_param(&request, "reset")?; + let reset: LayerAccessStatsReset = + parse_query_param(&request, "reset")?.unwrap_or(LayerAccessStatsReset::NoReset); let tenant = mgr::get_tenant(tenant_id, true) .await diff --git a/pageserver/src/tenant/storage_layer.rs b/pageserver/src/tenant/storage_layer.rs index c244c2c96a..15bdc54fcd 100644 --- a/pageserver/src/tenant/storage_layer.rs +++ b/pageserver/src/tenant/storage_layer.rs @@ -121,6 +121,7 @@ pub enum LayerResidenceStatus { #[derive(Clone, Copy, strum_macros::EnumString)] pub enum LayerAccessStatsReset { + NoReset, JustTaskKindFlags, AllStats, } @@ -249,19 +250,11 @@ impl LayerAccessStats { inner.task_kind_flag |= task_kind; inner.last_accesses.write(this_access); } - fn reset(&self, what: LayerAccessStatsReset) { + fn to_api_model( + &self, + reset: LayerAccessStatsReset, + ) -> pageserver_api::models::LayerAccessStats { let mut inner = self.0.lock().unwrap(); - match what { - LayerAccessStatsReset::JustTaskKindFlags => { - inner.task_kind_flag.clear(); - } - LayerAccessStatsReset::AllStats => { - *inner = LayerAccessStatsInner::default(); - } - } - } - fn to_api_model(&self) -> pageserver_api::models::LayerAccessStats { - let inner = self.0.lock().unwrap(); let LayerAccessStatsInner { first_access, count_by_access_kind, @@ -269,7 +262,7 @@ impl LayerAccessStats { last_accesses, last_residence_changes, } = &*inner; - pageserver_api::models::LayerAccessStats { + let ret = pageserver_api::models::LayerAccessStats { access_count_by_access_kind: count_by_access_kind .iter() .map(|(kind, count)| (kind, *count)) @@ -284,7 +277,17 @@ impl LayerAccessStats { .iter() .map(|s| s.to_api_model()) .collect(), + }; + match reset { + LayerAccessStatsReset::NoReset => (), + LayerAccessStatsReset::JustTaskKindFlags => { + inner.task_kind_flag.clear(); + } + LayerAccessStatsReset::AllStats => { + *inner = LayerAccessStatsInner::default(); + } } + ret } } @@ -395,7 +398,7 @@ pub trait PersistentLayer: Layer { /// current_physical_size is computed as the som of this value. fn file_size(&self) -> Option; - fn info(&self, reset: Option) -> HistoricLayerInfo; + fn info(&self, reset: LayerAccessStatsReset) -> HistoricLayerInfo; fn access_stats(&self) -> &LayerAccessStats; } diff --git a/pageserver/src/tenant/storage_layer/delta_layer.rs b/pageserver/src/tenant/storage_layer/delta_layer.rs index f23bb3a3ff..4876aa30e8 100644 --- a/pageserver/src/tenant/storage_layer/delta_layer.rs +++ b/pageserver/src/tenant/storage_layer/delta_layer.rs @@ -426,26 +426,22 @@ impl PersistentLayer for DeltaLayer { Some(self.file_size) } - fn info(&self, reset: Option) -> HistoricLayerInfo { + fn info(&self, reset: LayerAccessStatsReset) -> HistoricLayerInfo { let layer_file_name = self.filename().file_name(); let key_range = self.get_key_range(); let lsn_range = self.get_lsn_range(); - let ret = HistoricLayerInfo::Delta { + let access_stats = self.access_stats.to_api_model(reset); + + HistoricLayerInfo::Delta { layer_file_name, key_start: key_range.start, key_end: key_range.end, lsn_start: lsn_range.start, lsn_end: lsn_range.end, remote: false, - access_stats: self.access_stats.to_api_model(), - }; - - if let Some(reset) = reset { - self.access_stats.reset(reset); + access_stats, } - - ret } fn access_stats(&self) -> &LayerAccessStats { diff --git a/pageserver/src/tenant/storage_layer/image_layer.rs b/pageserver/src/tenant/storage_layer/image_layer.rs index e834d31cb0..c94237e8fd 100644 --- a/pageserver/src/tenant/storage_layer/image_layer.rs +++ b/pageserver/src/tenant/storage_layer/image_layer.rs @@ -238,25 +238,19 @@ impl PersistentLayer for ImageLayer { Some(self.file_size) } - fn info(&self, reset: Option) -> HistoricLayerInfo { + fn info(&self, reset: LayerAccessStatsReset) -> HistoricLayerInfo { let layer_file_name = self.filename().file_name(); let key_range = self.get_key_range(); let lsn_range = self.get_lsn_range(); - let ret = HistoricLayerInfo::Image { + HistoricLayerInfo::Image { layer_file_name, key_start: key_range.start, key_end: key_range.end, lsn_start: lsn_range.start, remote: false, - access_stats: self.access_stats.to_api_model(), - }; - - if let Some(reset) = reset { - self.access_stats.reset(reset); + access_stats: self.access_stats.to_api_model(reset), } - - ret } fn access_stats(&self) -> &LayerAccessStats { diff --git a/pageserver/src/tenant/storage_layer/remote_layer.rs b/pageserver/src/tenant/storage_layer/remote_layer.rs index b04aef935e..db3cba6f3d 100644 --- a/pageserver/src/tenant/storage_layer/remote_layer.rs +++ b/pageserver/src/tenant/storage_layer/remote_layer.rs @@ -142,7 +142,7 @@ impl PersistentLayer for RemoteLayer { self.layer_metadata.file_size() } - fn info(&self, _reset: Option) -> HistoricLayerInfo { + fn info(&self, reset: LayerAccessStatsReset) -> HistoricLayerInfo { let layer_file_name = self.filename().file_name(); let key_range = self.get_key_range(); let lsn_range = self.get_lsn_range(); @@ -155,7 +155,7 @@ impl PersistentLayer for RemoteLayer { lsn_start: lsn_range.start, lsn_end: lsn_range.end, remote: true, - access_stats: self.access_stats.to_api_model(), + access_stats: self.access_stats.to_api_model(reset), } } else { HistoricLayerInfo::Image { @@ -164,7 +164,7 @@ impl PersistentLayer for RemoteLayer { key_end: key_range.end, lsn_start: lsn_range.start, remote: true, - access_stats: self.access_stats.to_api_model(), + access_stats: self.access_stats.to_api_model(reset), } } } diff --git a/pageserver/src/tenant/timeline.rs b/pageserver/src/tenant/timeline.rs index a59ee6ae2f..20a2f143a8 100644 --- a/pageserver/src/tenant/timeline.rs +++ b/pageserver/src/tenant/timeline.rs @@ -820,7 +820,7 @@ impl Timeline { self.state.subscribe() } - pub fn layer_map_info(&self, reset: Option) -> LayerMapInfo { + pub fn layer_map_info(&self, reset: LayerAccessStatsReset) -> LayerMapInfo { let mut in_memory_layers = Vec::new(); let layer_map = self.layers.read().unwrap(); if let Some(open_layer) = &layer_map.open_layer {