cleanup API + add flag to residence status to distinguish loading from creating

libs/pageserver_api/src/models.rs

@@ -256,8 +256,13 @@ pub struct LayerAccessStatFullDetails {
 #[derive(Debug, Copy, Clone, Serialize, Deserialize)]
 #[serde(tag = "kind")]
 pub enum LayerResidenceStatus {
-    Resident { timestamp_millis_since_epoch: u128 },
-    Evicted { timestamp_millis_since_epoch: u128 },
+    Resident {
+        timestamp_millis_since_epoch: u128,
+        creating: bool,
+    },
+    Evicted {
+        timestamp_millis_since_epoch: u128,
+    },
 }
 
 #[derive(Debug, Clone, Serialize)]
@@ -300,7 +305,7 @@ pub enum HistoricLayerInfo {
         #[serde_as(as = "DisplayFromStr")]
         lsn_end: Lsn,
         remote: bool,
-        access_stats: Option<LayerAccessStats>,
+        access_stats: LayerAccessStats,
     },
     Image {
         layer_file_name: String,
@@ -311,7 +316,7 @@ pub enum HistoricLayerInfo {
         #[serde_as(as = "DisplayFromStr")]
         lsn_start: Lsn,
         remote: bool,
-        access_stats: Option<LayerAccessStats>,
+        access_stats: LayerAccessStats,
     },
 }
 

pageserver/src/tenant/storage_layer.rs

@@ -88,7 +88,7 @@ pub enum ValueReconstructResult {
     Missing,
 }
 
-#[derive(Default, Debug)]
+#[derive(Debug)]
 pub struct LayerAccessStats(Mutex<LayerAccessStatsInner>);
 
 #[derive(Debug, Clone)]
@@ -109,8 +109,13 @@ struct LayerAccessStatsInner {
 
 #[derive(Debug, Clone)]
 pub enum LayerResidenceStatus {
-    Resident { timestamp: SystemTime },
-    Evicted { timestamp: SystemTime },
+    Resident {
+        timestamp: SystemTime,
+        creating: bool,
+    },
+    Evicted {
+        timestamp: SystemTime,
+    },
 }
 
 #[derive(Clone, Copy, strum_macros::EnumString)]
@@ -147,19 +152,22 @@ impl LayerResidenceStatus {
         }
     }
 
-    pub fn resident() -> Self {
+    pub fn resident(creating: bool) -> Self {
         LayerResidenceStatus::Resident {
             timestamp: SystemTime::now(),
+            creating,
         }
     }
 
     fn to_api_model(&self) -> pageserver_api::models::LayerResidenceStatus {
         match self {
-            LayerResidenceStatus::Resident { timestamp } => {
-                pageserver_api::models::LayerResidenceStatus::Resident {
-                    timestamp_millis_since_epoch: system_time_to_millis_since_epoch(timestamp),
-                }
-            }
+            LayerResidenceStatus::Resident {
+                timestamp,
+                creating,
+            } => pageserver_api::models::LayerResidenceStatus::Resident {
+                timestamp_millis_since_epoch: system_time_to_millis_since_epoch(timestamp),
+                creating: *creating,
+            },
             LayerResidenceStatus::Evicted { timestamp } => {
                 pageserver_api::models::LayerResidenceStatus::Evicted {
                     timestamp_millis_since_epoch: system_time_to_millis_since_epoch(timestamp),
@@ -189,24 +197,44 @@ impl LayerAccessStats {
     const LAST_ACCESSES_MAX_LEN: usize = 16;
     const LAST_RESIDENCE_CHANGES_MAX_LEN: usize = 16;
 
+    pub(crate) fn new_for_loading_layer(residence_status: LayerResidenceStatus) -> Self {
+        let new = LayerAccessStats(Mutex::new(LayerAccessStatsInner::default()));
+        new.record_residence_change(residence_status);
+        new
+    }
+
+    pub(crate) fn new_for_new_layer_file() -> Self {
+        let new = LayerAccessStats(Mutex::new(LayerAccessStatsInner::default()));
+        new.record_residence_change(LayerResidenceStatus::resident(true));
+        new
+    }
+
     /// Creates a clone of `self` and records `new_status` in the clone.
     /// The `new_status` is not recorded in `self`
     pub(crate) fn clone_for_residence_change(
         &self,
         new_status: LayerResidenceStatus,
     ) -> LayerAccessStats {
-        let mut clone = {
+        let clone = {
             let inner = self.0.lock().unwrap();
             inner.clone()
         };
+        let new = LayerAccessStats(Mutex::new(clone));
+        new.record_residence_change(new_status);
+        new
+    }
+
+    fn record_residence_change(&self, new_status: LayerResidenceStatus) {
+        if LAYER_ACCESS_STATS_KILLSWITCH.load(atomic::Ordering::SeqCst) {
+            return;
+        }
+        let mut inner = self.0.lock().unwrap();
 
         // make room first to avoid reallocs
-        while clone.last_residence_changes.len() >= Self::LAST_RESIDENCE_CHANGES_MAX_LEN {
-            clone.last_residence_changes.pop_back();
+        while inner.last_residence_changes.len() >= Self::LAST_RESIDENCE_CHANGES_MAX_LEN {
+            inner.last_residence_changes.pop_back();
         }
-        clone.last_residence_changes.push_front(new_status.clone());
-
-        LayerAccessStats(Mutex::new(clone))
+        inner.last_residence_changes.push_front(new_status);
     }
 
     fn record_access(&self, access_kind: LayerAccessKind, task_kind: TaskKind) {

pageserver/src/tenant/storage_layer/delta_layer.rs

@@ -57,7 +57,7 @@ use utils::{
 
 use super::{
     DeltaFileName, Layer, LayerAccessStats, LayerAccessStatsReset, LayerFileName, LayerIter,
-    LayerKeyIter, PathOrConf,
+    LayerKeyIter, LayerResidenceStatus, PathOrConf,
 };
 
 ///
@@ -438,7 +438,7 @@ impl PersistentLayer for DeltaLayer {
             lsn_start: lsn_range.start,
             lsn_end: lsn_range.end,
             remote: false,
-            access_stats: Some(self.access_stats.to_api_model()),
+            access_stats: self.access_stats.to_api_model(),
         };
 
         if let Some(reset) = reset {
@@ -582,7 +582,7 @@ impl DeltaLayer {
         tenant_id: TenantId,
         filename: &DeltaFileName,
         file_size: u64,
-        existing_access_stats: Option<LayerAccessStats>,
+        access_stats: LayerAccessStats,
     ) -> DeltaLayer {
         DeltaLayer {
             path_or_conf: PathOrConf::Conf(conf),
@@ -591,7 +591,7 @@ impl DeltaLayer {
             key_range: filename.key_range.clone(),
             lsn_range: filename.lsn_range.clone(),
             file_size,
-            access_stats: existing_access_stats.unwrap_or_default(),
+            access_stats,
             inner: RwLock::new(DeltaLayerInner {
                 loaded: false,
                 file: None,
@@ -621,7 +621,9 @@ impl DeltaLayer {
             key_range: summary.key_range,
             lsn_range: summary.lsn_range,
             file_size: metadata.len(),
-            access_stats: LayerAccessStats::default(),
+            access_stats: LayerAccessStats::new_for_loading_layer(LayerResidenceStatus::resident(
+                false,
+            )),
             inner: RwLock::new(DeltaLayerInner {
                 loaded: false,
                 file: None,
@@ -792,7 +794,7 @@ impl DeltaLayerWriterInner {
             key_range: self.key_start..key_end,
             lsn_range: self.lsn_range.clone(),
             file_size: metadata.len(),
-            access_stats: LayerAccessStats::default(),
+            access_stats: LayerAccessStats::new_for_new_layer_file(),
             inner: RwLock::new(DeltaLayerInner {
                 loaded: false,
                 file: None,

pageserver/src/tenant/storage_layer/image_layer.rs

@@ -52,7 +52,7 @@ use utils::{
 };
 
 use super::filename::{ImageFileName, LayerFileName, PathOrConf};
-use super::{Layer, LayerAccessStats, LayerAccessStatsReset, LayerIter};
+use super::{Layer, LayerAccessStats, LayerAccessStatsReset, LayerIter, LayerResidenceStatus};
 
 ///
 /// Header stored in the beginning of the file
@@ -249,7 +249,7 @@ impl PersistentLayer for ImageLayer {
             key_end: key_range.end,
             lsn_start: lsn_range.start,
             remote: false,
-            access_stats: Some(self.access_stats.to_api_model()),
+            access_stats: self.access_stats.to_api_model(),
         };
 
         if let Some(reset) = reset {
@@ -385,7 +385,7 @@ impl ImageLayer {
         tenant_id: TenantId,
         filename: &ImageFileName,
         file_size: u64,
-        existing_access_stats: Option<LayerAccessStats>,
+        access_stats: LayerAccessStats,
     ) -> ImageLayer {
         ImageLayer {
             path_or_conf: PathOrConf::Conf(conf),
@@ -394,7 +394,7 @@ impl ImageLayer {
             key_range: filename.key_range.clone(),
             lsn: filename.lsn,
             file_size,
-            access_stats: existing_access_stats.unwrap_or_default(),
+            access_stats,
             inner: RwLock::new(ImageLayerInner {
                 loaded: false,
                 file: None,
@@ -422,7 +422,9 @@ impl ImageLayer {
             key_range: summary.key_range,
             lsn: summary.lsn,
             file_size: metadata.len(),
-            access_stats: LayerAccessStats::default(),
+            access_stats: LayerAccessStats::new_for_loading_layer(LayerResidenceStatus::resident(
+                false,
+            )),
             inner: RwLock::new(ImageLayerInner {
                 file: None,
                 loaded: false,
@@ -582,7 +584,7 @@ impl ImageLayerWriterInner {
             key_range: self.key_range.clone(),
             lsn: self.lsn,
             file_size: metadata.len(),
-            access_stats: LayerAccessStats::default(),
+            access_stats: LayerAccessStats::new_for_new_layer_file(),
             inner: RwLock::new(ImageLayerInner {
                 loaded: false,
                 file: None,

pageserver/src/tenant/storage_layer/remote_layer.rs

@@ -155,7 +155,7 @@ impl PersistentLayer for RemoteLayer {
                 lsn_start: lsn_range.start,
                 lsn_end: lsn_range.end,
                 remote: true,
-                access_stats: Some(self.access_stats.to_api_model()),
+                access_stats: self.access_stats.to_api_model(),
             }
         } 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: Some(self.access_stats.to_api_model()),
+                access_stats: self.access_stats.to_api_model(),
             }
         }
     }
@@ -180,7 +180,7 @@ impl RemoteLayer {
         timelineid: TimelineId,
         fname: &ImageFileName,
         layer_metadata: &LayerFileMetadata,
-        existing_access_stats: Option<LayerAccessStats>,
+        access_stats: LayerAccessStats,
     ) -> RemoteLayer {
         RemoteLayer {
             tenantid,
@@ -192,7 +192,7 @@ impl RemoteLayer {
             file_name: fname.to_owned().into(),
             layer_metadata: layer_metadata.clone(),
             ongoing_download: Arc::new(tokio::sync::Semaphore::new(1)),
-            access_stats: existing_access_stats.unwrap_or_default(),
+            access_stats,
         }
     }
 
@@ -201,8 +201,7 @@ impl RemoteLayer {
         timelineid: TimelineId,
         fname: &DeltaFileName,
         layer_metadata: &LayerFileMetadata,
-
-        existing_access_stats: Option<LayerAccessStats>,
+        access_stats: LayerAccessStats,
     ) -> RemoteLayer {
         RemoteLayer {
             tenantid,
@@ -214,7 +213,7 @@ impl RemoteLayer {
             file_name: fname.to_owned().into(),
             layer_metadata: layer_metadata.clone(),
             ongoing_download: Arc::new(tokio::sync::Semaphore::new(1)),
-            access_stats: existing_access_stats.unwrap_or_default(),
+            access_stats,
         }
     }
 
@@ -235,10 +234,8 @@ impl RemoteLayer {
                 self.tenantid,
                 &fname,
                 file_size,
-                Some(
-                    self.access_stats
-                        .clone_for_residence_change(LayerResidenceStatus::resident()),
-                ),
+                self.access_stats
+                    .clone_for_residence_change(LayerResidenceStatus::resident(false)),
             ))
         } else {
             let fname = ImageFileName {
@@ -251,10 +248,8 @@ impl RemoteLayer {
                 self.tenantid,
                 &fname,
                 file_size,
-                Some(
-                    self.access_stats
-                        .clone_for_residence_change(LayerResidenceStatus::resident()),
-                ),
+                self.access_stats
+                    .clone_for_residence_change(LayerResidenceStatus::resident(false)),
             ))
         }
     }

pageserver/src/tenant/timeline.rs

@@ -29,8 +29,8 @@ use crate::broker_client::is_broker_client_initialized;
 use crate::context::{DownloadBehavior, RequestContext};
 use crate::tenant::remote_timeline_client::{self, index::LayerFileMetadata};
 use crate::tenant::storage_layer::{
-    DeltaFileName, DeltaLayerWriter, ImageFileName, ImageLayerWriter, InMemoryLayer, LayerFileName,
-    RemoteLayer,
+    DeltaFileName, DeltaLayerWriter, ImageFileName, ImageLayerWriter, InMemoryLayer,
+    LayerAccessStats, LayerFileName, LayerResidenceStatus, RemoteLayer,
 };
 use crate::tenant::{
     ephemeral_file::is_ephemeral_file,
@@ -71,9 +71,7 @@ use walreceiver::spawn_connection_manager_task;
 use super::layer_map::BatchedUpdates;
 use super::remote_timeline_client::index::IndexPart;
 use super::remote_timeline_client::RemoteTimelineClient;
-use super::storage_layer::{
-    DeltaLayer, ImageLayer, Layer, LayerAccessStatsReset, LayerResidenceStatus,
-};
+use super::storage_layer::{DeltaLayer, ImageLayer, Layer, LayerAccessStatsReset};
 
 #[derive(Debug, PartialEq, Eq, Clone, Copy)]
 enum FlushLoopState {
@@ -879,22 +877,18 @@ impl Timeline {
                 self.timeline_id,
                 &image_name,
                 &layer_metadata,
-                Some(
-                    local_layer
-                        .access_stats()
-                        .clone_for_residence_change(LayerResidenceStatus::evicted()),
-                ),
+                local_layer
+                    .access_stats()
+                    .clone_for_residence_change(LayerResidenceStatus::evicted()),
             ),
             LayerFileName::Delta(delta_name) => RemoteLayer::new_delta(
                 self.tenant_id,
                 self.timeline_id,
                 &delta_name,
                 &layer_metadata,
-                Some(
-                    local_layer
-                        .access_stats()
-                        .clone_for_residence_change(LayerResidenceStatus::evicted()),
-                ),
+                local_layer
+                    .access_stats()
+                    .clone_for_residence_change(LayerResidenceStatus::evicted()),
             ),
             #[cfg(test)]
             LayerFileName::Test(_) => unreachable!(),
@@ -1171,7 +1165,7 @@ impl Timeline {
                     self.tenant_id,
                     &imgfilename,
                     file_size,
-                    None,
+                    LayerAccessStats::new_for_loading_layer(LayerResidenceStatus::resident(false)),
                 );
 
                 trace!("found layer {}", layer.path().display());
@@ -1203,7 +1197,7 @@ impl Timeline {
                     self.tenant_id,
                     &deltafilename,
                     file_size,
-                    None,
+                    LayerAccessStats::new_for_loading_layer(LayerResidenceStatus::resident(false)),
                 );
 
                 trace!("found layer {}", layer.path().display());
@@ -1341,7 +1335,7 @@ impl Timeline {
                         self.timeline_id,
                         imgfilename,
                         &remote_layer_metadata,
-                        None,
+                        LayerAccessStats::new_for_loading_layer(LayerResidenceStatus::evicted()),
                     );
                     let remote_layer = Arc::new(remote_layer);
 
@@ -1366,7 +1360,7 @@ impl Timeline {
                         self.timeline_id,
                         deltafilename,
                         &remote_layer_metadata,
-                        None,
+                        LayerAccessStats::new_for_loading_layer(LayerResidenceStatus::evicted()),
                     );
                     let remote_layer = Arc::new(remote_layer);
                     updates.insert_historic(remote_layer);