refactor: use immutable storage state in timeline

Signed-off-by: Alex Chi <iskyzh@gmail.com>

pageserver/src/tenant.rs

@@ -87,8 +87,8 @@ pub mod disk_btree;
 pub(crate) mod ephemeral_file;
 pub mod layer_cache;
 pub mod layer_map;
-pub mod manifest;
 pub mod layer_map_mgr;
+pub mod manifest;
 
 pub mod metadata;
 mod par_fsync;
@@ -557,17 +557,10 @@ impl Tenant {
                 .context("failed to reconcile with remote")?
         }
 
+        let layers = timeline.layer_mgr.read();
         // Sanity check: a timeline should have some content.
         anyhow::ensure!(
-            ancestor.is_some()
-                || timeline
-                    .layers
-                    .read()
-                    .await
-                    .0
-                    .iter_historic_layers()
-                    .next()
-                    .is_some(),
+            ancestor.is_some() || layers.iter_historic_layers().next().is_some(),
             "Timeline has no ancestor and no layer files"
         );
 

pageserver/src/tenant/layer_map_mgr.rs

@@ -63,6 +63,18 @@ impl LayerMapMgr {
         self.layer_map.store(Arc::new(new_state));
         Ok(())
     }
+
+    /// Update the layer map.
+    pub  fn update_sync<O>(&self, operation: O) -> Result<()>
+    where
+        O: FnOnce(LayerMap) -> Result<LayerMap>,
+    {
+        let state_lock = self.state_lock.blocking_lock();
+        let state = self.clone_for_write(&state_lock);
+        let new_state = operation(state)?;
+        self.layer_map.store(Arc::new(new_state));
+        Ok(())
+    }
 }
 
 #[cfg(test)]

pageserver/src/tenant/storage_layer.rs

@@ -41,8 +41,6 @@ pub use inmemory_layer::InMemoryLayer;
 pub use layer_desc::{PersistentLayerDesc, PersistentLayerKey};
 pub use remote_layer::RemoteLayer;
 
-use super::layer_map::BatchedUpdates;
-
 pub fn range_overlaps<T>(a: &Range<T>, b: &Range<T>) -> bool
 where
     T: PartialOrd<T>,
@@ -177,12 +175,10 @@ impl LayerAccessStats {
     ///
     /// See [`record_residence_event`] for why you need to do this while holding the layer map lock.
     pub(crate) fn for_loading_layer(
-        layer_map_lock_held_witness: &BatchedUpdates<'_>,
         status: LayerResidenceStatus,
     ) -> Self {
         let new = LayerAccessStats(Mutex::new(LayerAccessStatsLocked::default()));
         new.record_residence_event(
-            layer_map_lock_held_witness,
             status,
             LayerResidenceEventReason::LayerLoad,
         );
@@ -196,7 +192,6 @@ impl LayerAccessStats {
     /// See [`record_residence_event`] for why you need to do this while holding the layer map lock.
     pub(crate) fn clone_for_residence_change(
         &self,
-        layer_map_lock_held_witness: &BatchedUpdates<'_>,
         new_status: LayerResidenceStatus,
     ) -> LayerAccessStats {
         let clone = {
@@ -205,7 +200,6 @@ impl LayerAccessStats {
         };
         let new = LayerAccessStats(Mutex::new(clone));
         new.record_residence_event(
-            layer_map_lock_held_witness,
             new_status,
             LayerResidenceEventReason::ResidenceChange,
         );
@@ -228,7 +222,6 @@ impl LayerAccessStats {
     ///
     pub(crate) fn record_residence_event(
         &self,
-        _layer_map_lock_held_witness: &BatchedUpdates<'_>,
         status: LayerResidenceStatus,
         reason: LayerResidenceEventReason,
     ) {

pageserver/src/tenant/storage_layer/remote_layer.rs

@@ -4,7 +4,6 @@
 use crate::config::PageServerConf;
 use crate::context::RequestContext;
 use crate::repository::Key;
-use crate::tenant::layer_map::BatchedUpdates;
 use crate::tenant::remote_timeline_client::index::LayerFileMetadata;
 use crate::tenant::storage_layer::{Layer, ValueReconstructResult, ValueReconstructState};
 use anyhow::{bail, Result};
@@ -220,7 +219,6 @@ impl RemoteLayer {
     /// Create a Layer struct representing this layer, after it has been downloaded.
     pub fn create_downloaded_layer(
         &self,
-        layer_map_lock_held_witness: &BatchedUpdates<'_>,
         conf: &'static PageServerConf,
         file_size: u64,
     ) -> Arc<dyn PersistentLayer> {
@@ -232,10 +230,8 @@ impl RemoteLayer {
                 self.desc.tenant_id,
                 &fname,
                 file_size,
-                self.access_stats.clone_for_residence_change(
-                    layer_map_lock_held_witness,
-                    LayerResidenceStatus::Resident,
-                ),
+                self.access_stats
+                    .clone_for_residence_change(LayerResidenceStatus::Resident),
             ))
         } else {
             let fname = self.desc.image_file_name();
@@ -245,10 +241,8 @@ impl RemoteLayer {
                 self.desc.tenant_id,
                 &fname,
                 file_size,
-                self.access_stats.clone_for_residence_change(
-                    layer_map_lock_held_witness,
-                    LayerResidenceStatus::Resident,
-                ),
+                self.access_stats
+                    .clone_for_residence_change(LayerResidenceStatus::Resident),
             ))
         }
     }

pageserver/src/tenant/timeline/eviction_task.rs

@@ -197,8 +197,8 @@ impl Timeline {
         // We don't want to hold the layer map lock during eviction.
         // So, we just need to deal with this.
         let candidates: Vec<Arc<dyn PersistentLayer>> = {
-            let guard = self.layers.read().await;
-            let (layers, _) = &*guard;
+            let guard = self.lcache.layer_in_use_read().await;
+            let layers = self.layer_mgr.read();
             let mut candidates = Vec::new();
             for hist_layer in layers.iter_historic_layers() {
                 let hist_layer = self.lcache.get_from_desc(&hist_layer);