From 2e676fba96409150b143cd9be19ca59da99f76d2 Mon Sep 17 00:00:00 2001 From: Christian Schwarz Date: Thu, 2 Feb 2023 11:32:48 +0100 Subject: [PATCH] LayerResidenceStats: docs + distinct constructors for created=false,true --- libs/pageserver_api/src/models.rs | 2 +- pageserver/src/tenant/storage_layer.rs | 37 +++++++++++++------ .../src/tenant/storage_layer/delta_layer.rs | 4 +- .../src/tenant/storage_layer/image_layer.rs | 4 +- .../src/tenant/storage_layer/remote_layer.rs | 4 +- pageserver/src/tenant/timeline.rs | 4 +- 6 files changed, 33 insertions(+), 22 deletions(-) diff --git a/libs/pageserver_api/src/models.rs b/libs/pageserver_api/src/models.rs index 7788c86c50..4b6b01124a 100644 --- a/libs/pageserver_api/src/models.rs +++ b/libs/pageserver_api/src/models.rs @@ -258,7 +258,7 @@ pub struct LayerAccessStatFullDetails { pub enum LayerResidenceStatus { Resident { timestamp_millis_since_epoch: u128, - creating: bool, + created: bool, }, Evicted { timestamp_millis_since_epoch: u128, diff --git a/pageserver/src/tenant/storage_layer.rs b/pageserver/src/tenant/storage_layer.rs index fb64b2a199..fd9c66f054 100644 --- a/pageserver/src/tenant/storage_layer.rs +++ b/pageserver/src/tenant/storage_layer.rs @@ -111,7 +111,8 @@ struct LayerAccessStatsInner { pub enum LayerResidenceStatus { Resident { timestamp: SystemTime, - creating: bool, + /// If `true`, then this resident status marks the birth time of the layer. + created: bool, }, Evicted { timestamp: SystemTime, @@ -146,28 +147,42 @@ impl LayerAccessStatFullDetails { } impl LayerResidenceStatus { + /// Residence status for a layer file that only exists on the remote. pub fn evicted() -> Self { LayerResidenceStatus::Evicted { timestamp: SystemTime::now(), } } - pub fn resident(creating: bool) -> Self { + /// Residence status for a layer file that exists locally. + /// It may also exist on the remote, we don't care here. + /// NB: use this for existing layer files, e.g., during timeline load. + /// For newly written layer files, use [`created`]. + pub fn resident() -> Self { LayerResidenceStatus::Resident { timestamp: SystemTime::now(), - creating, + created: false, + } + } + + /// Residence status for a local layer file that we just wrote. + /// Example: a layer file created by compaction. + /// Private, because callers are supposed to use [`LayerAccessStats::new_for_new_layer_file`]. + fn created() -> Self { + LayerResidenceStatus::Resident { + timestamp: SystemTime::now(), + created: true, } } fn to_api_model(&self) -> pageserver_api::models::LayerResidenceStatus { match self { - LayerResidenceStatus::Resident { - timestamp, - creating, - } => pageserver_api::models::LayerResidenceStatus::Resident { - timestamp_millis_since_epoch: system_time_to_millis_since_epoch(timestamp), - creating: *creating, - }, + LayerResidenceStatus::Resident { timestamp, created } => { + pageserver_api::models::LayerResidenceStatus::Resident { + timestamp_millis_since_epoch: system_time_to_millis_since_epoch(timestamp), + created: *created, + } + } LayerResidenceStatus::Evicted { timestamp } => { pageserver_api::models::LayerResidenceStatus::Evicted { timestamp_millis_since_epoch: system_time_to_millis_since_epoch(timestamp), @@ -205,7 +220,7 @@ impl LayerAccessStats { pub(crate) fn new_for_new_layer_file() -> Self { let new = LayerAccessStats(Mutex::new(LayerAccessStatsInner::default())); - new.record_residence_change(LayerResidenceStatus::resident(true)); + new.record_residence_change(LayerResidenceStatus::created()); new } diff --git a/pageserver/src/tenant/storage_layer/delta_layer.rs b/pageserver/src/tenant/storage_layer/delta_layer.rs index f10f3b33c5..fdbbc16cef 100644 --- a/pageserver/src/tenant/storage_layer/delta_layer.rs +++ b/pageserver/src/tenant/storage_layer/delta_layer.rs @@ -621,9 +621,7 @@ impl DeltaLayer { key_range: summary.key_range, lsn_range: summary.lsn_range, file_size: metadata.len(), - access_stats: LayerAccessStats::new_for_loading_layer(LayerResidenceStatus::resident( - false, - )), + access_stats: LayerAccessStats::new_for_loading_layer(LayerResidenceStatus::resident()), inner: RwLock::new(DeltaLayerInner { loaded: false, file: None, diff --git a/pageserver/src/tenant/storage_layer/image_layer.rs b/pageserver/src/tenant/storage_layer/image_layer.rs index 40a8bd597a..4219b8fc06 100644 --- a/pageserver/src/tenant/storage_layer/image_layer.rs +++ b/pageserver/src/tenant/storage_layer/image_layer.rs @@ -422,9 +422,7 @@ impl ImageLayer { key_range: summary.key_range, lsn: summary.lsn, file_size: metadata.len(), - access_stats: LayerAccessStats::new_for_loading_layer(LayerResidenceStatus::resident( - false, - )), + access_stats: LayerAccessStats::new_for_loading_layer(LayerResidenceStatus::resident()), inner: RwLock::new(ImageLayerInner { file: None, loaded: false, diff --git a/pageserver/src/tenant/storage_layer/remote_layer.rs b/pageserver/src/tenant/storage_layer/remote_layer.rs index 9e2204d4d7..b04aef935e 100644 --- a/pageserver/src/tenant/storage_layer/remote_layer.rs +++ b/pageserver/src/tenant/storage_layer/remote_layer.rs @@ -235,7 +235,7 @@ impl RemoteLayer { &fname, file_size, self.access_stats - .clone_for_residence_change(LayerResidenceStatus::resident(false)), + .clone_for_residence_change(LayerResidenceStatus::resident()), )) } else { let fname = ImageFileName { @@ -249,7 +249,7 @@ impl RemoteLayer { &fname, file_size, self.access_stats - .clone_for_residence_change(LayerResidenceStatus::resident(false)), + .clone_for_residence_change(LayerResidenceStatus::resident()), )) } } diff --git a/pageserver/src/tenant/timeline.rs b/pageserver/src/tenant/timeline.rs index c578c436bc..662dd58a78 100644 --- a/pageserver/src/tenant/timeline.rs +++ b/pageserver/src/tenant/timeline.rs @@ -1165,7 +1165,7 @@ impl Timeline { self.tenant_id, &imgfilename, file_size, - LayerAccessStats::new_for_loading_layer(LayerResidenceStatus::resident(false)), + LayerAccessStats::new_for_loading_layer(LayerResidenceStatus::resident()), ); trace!("found layer {}", layer.path().display()); @@ -1197,7 +1197,7 @@ impl Timeline { self.tenant_id, &deltafilename, file_size, - LayerAccessStats::new_for_loading_layer(LayerResidenceStatus::resident(false)), + LayerAccessStats::new_for_loading_layer(LayerResidenceStatus::resident()), ); trace!("found layer {}", layer.path().display());