std::fmt::Debug for Layer implementations (#3536)

Follow-up to #3513.

This removes the old blanket `std::fmt::Debug` impl on `dyn Layer` which
did not seem to be used from anywhere (no compilation errors after
removing).

Adds `std::fmt::Debug` requirement and implementations for `trait Layer`
implementors:
- LayerDescriptor (derived)
- RemoteLayer (manual)
- DeltaLayer (manual)
- ImageLayer (manual)

Manual implementations are used to skip PageserverConf, tenant and
timeline ids, large collections.

Adds and adjusts some doc comments to be more rustdoc alike.

pageserver/src/tenant/storage_layer.rs

@@ -85,7 +85,11 @@ pub enum ValueReconstructResult {
 
 /// Supertrait of the [`Layer`] trait that captures the bare minimum interface
 /// required by [`LayerMap`].
-pub trait Layer: Send + Sync {
+///
+/// All layers should implement a minimal `std::fmt::Debug` without tenant or
+/// timeline names, because those are known in the context of which the layers
+/// are used in (timeline).
+pub trait Layer: std::fmt::Debug + Send + Sync {
     /// Range of keys that this layer covers
     fn get_key_range(&self) -> Range<Key>;
 
@@ -148,8 +152,7 @@ pub type LayerKeyIter<'i> = Box<dyn Iterator<Item = (Key, Lsn, u64)> + 'i>;
 /// Furthermore, there are two kinds of on-disk layers: delta and image layers.
 /// A delta layer contains all modifications within a range of LSNs and keys.
 /// An image layer is a snapshot of all the data in a key-range, at a single
-/// LSN
-///
+/// LSN.
 pub trait PersistentLayer: Layer {
     fn get_tenant_id(&self) -> TenantId;
 
@@ -203,19 +206,11 @@ pub fn downcast_remote_layer(
     }
 }
 
-impl std::fmt::Debug for dyn Layer {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        f.debug_struct("Layer")
-            .field("short_id", &self.short_id())
-            .finish()
-    }
-}
-
 /// Holds metadata about a layer without any content. Used mostly for testing.
 ///
 /// To use filenames as fixtures, parse them as [`LayerFileName`] then convert from that to a
 /// LayerDescriptor.
-#[derive(Clone)]
+#[derive(Clone, Debug)]
 pub struct LayerDescriptor {
     pub key: Range<Key>,
     pub lsn: Range<Lsn>,

pageserver/src/tenant/storage_layer/delta_layer.rs

@@ -167,14 +167,13 @@ impl DeltaKey {
     }
 }
 
+/// DeltaLayer is the in-memory data structure associated with an on-disk delta
+/// file.
 ///
-/// DeltaLayer is the in-memory data structure associated with an
-/// on-disk delta file.  We keep a DeltaLayer in memory for each
-/// file, in the LayerMap. If a layer is in "loaded" state, we have a
-/// copy of the index in memory, in 'inner'. Otherwise the struct is
-/// just a placeholder for a file that exists on disk, and it needs to
-/// be loaded before using it in queries.
-///
+/// We keep a DeltaLayer in memory for each file, in the LayerMap. If a layer
+/// is in "loaded" state, we have a copy of the index in memory, in 'inner'.
+/// Otherwise the struct is just a placeholder for a file that exists on disk,
+/// and it needs to be loaded before using it in queries.
 pub struct DeltaLayer {
     path_or_conf: PathOrConf,
 
@@ -188,6 +187,17 @@ pub struct DeltaLayer {
     inner: RwLock<DeltaLayerInner>,
 }
 
+impl std::fmt::Debug for DeltaLayer {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("DeltaLayer")
+            .field("key_range", &self.key_range)
+            .field("lsn_range", &self.lsn_range)
+            .field("file_size", &self.file_size)
+            .field("inner", &self.inner)
+            .finish()
+    }
+}
+
 pub struct DeltaLayerInner {
     /// If false, the fields below have not been loaded into memory yet.
     loaded: bool,
@@ -200,6 +210,16 @@ pub struct DeltaLayerInner {
     file: Option<FileBlockReader<VirtualFile>>,
 }
 
+impl std::fmt::Debug for DeltaLayerInner {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("DeltaLayerInner")
+            .field("loaded", &self.loaded)
+            .field("index_start_blk", &self.index_start_blk)
+            .field("index_root_blk", &self.index_root_blk)
+            .finish()
+    }
+}
+
 impl Layer for DeltaLayer {
     fn get_key_range(&self) -> Range<Key> {
         self.key_range.clone()

pageserver/src/tenant/storage_layer/image_layer.rs

@@ -95,13 +95,13 @@ impl From<&ImageLayer> for Summary {
     }
 }
 
-///
 /// ImageLayer is the in-memory data structure associated with an on-disk image
-/// file.  We keep an ImageLayer in memory for each file, in the LayerMap. If a
-/// layer is in "loaded" state, we have a copy of the index in memory, in 'inner'.
+/// file.
+///
+/// We keep an ImageLayer in memory for each file, in the LayerMap. If a layer
+/// is in "loaded" state, we have a copy of the index in memory, in 'inner'.
 /// Otherwise the struct is just a placeholder for a file that exists on disk,
 /// and it needs to be loaded before using it in queries.
-///
 pub struct ImageLayer {
     path_or_conf: PathOrConf,
     pub tenant_id: TenantId,
@@ -115,6 +115,17 @@ pub struct ImageLayer {
     inner: RwLock<ImageLayerInner>,
 }
 
+impl std::fmt::Debug for ImageLayer {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("ImageLayer")
+            .field("key_range", &self.key_range)
+            .field("file_size", &self.file_size)
+            .field("lsn", &self.lsn)
+            .field("inner", &self.inner)
+            .finish()
+    }
+}
+
 pub struct ImageLayerInner {
     /// If false, the 'index' has not been loaded into memory yet.
     loaded: bool,
@@ -127,6 +138,16 @@ pub struct ImageLayerInner {
     file: Option<FileBlockReader<VirtualFile>>,
 }
 
+impl std::fmt::Debug for ImageLayerInner {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("ImageLayerInner")
+            .field("loaded", &self.loaded)
+            .field("index_start_blk", &self.index_start_blk)
+            .field("index_root_blk", &self.index_root_blk)
+            .finish()
+    }
+}
+
 impl Layer for ImageLayer {
     fn get_key_range(&self) -> Range<Key> {
         self.key_range.clone()

pageserver/src/tenant/storage_layer/inmemory_layer.rs

@@ -53,6 +53,15 @@ pub struct InMemoryLayer {
     inner: RwLock<InMemoryLayerInner>,
 }
 
+impl std::fmt::Debug for InMemoryLayer {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("InMemoryLayer")
+            .field("start_lsn", &self.start_lsn)
+            .field("inner", &self.inner)
+            .finish()
+    }
+}
+
 pub struct InMemoryLayerInner {
     /// Frozen layers have an exclusive end LSN.
     /// Writes are only allowed when this is None
@@ -71,6 +80,14 @@ pub struct InMemoryLayerInner {
     file: EphemeralFile,
 }
 
+impl std::fmt::Debug for InMemoryLayerInner {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("InMemoryLayerInner")
+            .field("end_lsn", &self.end_lsn)
+            .finish()
+    }
+}
+
 impl InMemoryLayerInner {
     fn assert_writeable(&self) {
         assert!(self.end_lsn.is_none());

pageserver/src/tenant/storage_layer/remote_layer.rs

@@ -21,7 +21,14 @@ use super::filename::{DeltaFileName, ImageFileName, LayerFileName};
 use super::image_layer::ImageLayer;
 use super::{DeltaLayer, LayerIter, LayerKeyIter, PersistentLayer};
 
-#[derive(Debug)]
+/// RemoteLayer is a not yet downloaded [`ImageLayer`] or
+/// [`crate::storage_layer::DeltaLayer`].
+///
+/// RemoteLayer might be downloaded on-demand during operations which are
+/// allowed download remote layers and during which, it gets replaced with a
+/// concrete `DeltaLayer` or `ImageLayer`.
+///
+/// See: [`crate::context::RequestContext`] for authorization to download
 pub struct RemoteLayer {
     tenantid: TenantId,
     timelineid: TimelineId,
@@ -39,6 +46,16 @@ pub struct RemoteLayer {
     pub(crate) ongoing_download: Arc<tokio::sync::Semaphore>,
 }
 
+impl std::fmt::Debug for RemoteLayer {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("RemoteLayer")
+            .field("file_name", &self.file_name)
+            .field("layer_metadata", &self.layer_metadata)
+            .field("is_incremental", &self.is_incremental)
+            .finish()
+    }
+}
+
 impl Layer for RemoteLayer {
     fn get_key_range(&self) -> Range<Key> {
         self.key_range.clone()