refactor: remove is_incremental=true for ImageLayers footgun (#5061)

Accidentially giving is_incremental=true for ImageLayers costs a lot of
debugging time. Removes all API which would allow to do that. They can
easily be restored later *when needed*.

Split off from #4938.

pageserver/benches/bench_layer_map.rs

@@ -215,7 +215,6 @@ fn bench_sequential(c: &mut Criterion) {
             TimelineId::generate(),
             zero.add(10 * i32)..zero.add(10 * i32 + 1),
             Lsn(i),
-            false,
             0,
         );
         updates.insert_historic(layer);

pageserver/src/tenant/storage_layer.rs

@@ -447,7 +447,6 @@ pub mod tests {
                 TimelineId::from_array([0; 16]),
                 value.key_range,
                 value.lsn,
-                false,
                 233,
             )
         }

pageserver/src/tenant/storage_layer/image_layer.rs

@@ -222,7 +222,7 @@ impl ImageLayer {
             self.desc.key_range.start,
             self.desc.key_range.end,
             self.lsn,
-            self.desc.is_incremental,
+            self.desc.is_incremental(),
             self.desc.file_size
         );
 
@@ -382,7 +382,6 @@ impl ImageLayer {
                 timeline_id,
                 filename.key_range.clone(),
                 filename.lsn,
-                false,
                 file_size,
             ), // Now we assume image layer ALWAYS covers the full range. This may change in the future.
             lsn: filename.lsn,
@@ -409,7 +408,6 @@ impl ImageLayer {
                 summary.timeline_id,
                 summary.key_range,
                 summary.lsn,
-                false,
                 metadata.len(),
             ), // Now we assume image layer ALWAYS covers the full range. This may change in the future.
             lsn: summary.lsn,
@@ -511,7 +509,6 @@ struct ImageLayerWriterInner {
     tenant_id: TenantId,
     key_range: Range<Key>,
     lsn: Lsn,
-    is_incremental: bool,
 
     blob_writer: WriteBlobWriter<VirtualFile>,
     tree: DiskBtreeBuilder<BlockBuf, KEY_SIZE>,
@@ -527,7 +524,6 @@ impl ImageLayerWriterInner {
         tenant_id: TenantId,
         key_range: &Range<Key>,
         lsn: Lsn,
-        is_incremental: bool,
     ) -> anyhow::Result<Self> {
         // Create the file initially with a temporary filename.
         // We'll atomically rename it to the final name when we're done.
@@ -562,7 +558,6 @@ impl ImageLayerWriterInner {
             lsn,
             tree: tree_builder,
             blob_writer,
-            is_incremental,
         };
 
         Ok(writer)
@@ -623,7 +618,6 @@ impl ImageLayerWriterInner {
             self.timeline_id,
             self.key_range.clone(),
             self.lsn,
-            self.is_incremental, // for now, image layer ALWAYS covers the full range
             metadata.len(),
         );
 
@@ -698,7 +692,6 @@ impl ImageLayerWriter {
         tenant_id: TenantId,
         key_range: &Range<Key>,
         lsn: Lsn,
-        is_incremental: bool,
     ) -> anyhow::Result<ImageLayerWriter> {
         Ok(Self {
             inner: Some(ImageLayerWriterInner::new(
@@ -707,7 +700,6 @@ impl ImageLayerWriter {
                 tenant_id,
                 key_range,
                 lsn,
-                is_incremental,
             )?),
         })
     }

pageserver/src/tenant/storage_layer/layer_desc.rs

@@ -28,13 +28,8 @@ pub struct PersistentLayerDesc {
     /// range start
     /// - An image layer represents snapshot at one LSN, so end_lsn is always the snapshot LSN + 1
     pub lsn_range: Range<Lsn>,
-    /// Whether this is a delta layer.
+    /// Whether this is a delta layer, and also, is this incremental.
     pub is_delta: bool,
-    /// Does this layer only contain some data for the key-range (incremental),
-    /// or does it contain a version of every page? This is important to know
-    /// for garbage collecting old layers: an incremental layer depends on
-    /// the previous non-incremental layer.
-    pub is_incremental: bool,
     pub file_size: u64,
 }
 
@@ -67,7 +62,6 @@ impl PersistentLayerDesc {
             key_range,
             lsn_range: Lsn(0)..Lsn(1),
             is_delta: false,
-            is_incremental: false,
             file_size: 0,
         }
     }
@@ -77,7 +71,6 @@ impl PersistentLayerDesc {
         timeline_id: TimelineId,
         key_range: Range<Key>,
         lsn: Lsn,
-        is_incremental: bool,
         file_size: u64,
     ) -> Self {
         Self {
@@ -86,7 +79,6 @@ impl PersistentLayerDesc {
             key_range,
             lsn_range: Self::image_layer_lsn_range(lsn),
             is_delta: false,
-            is_incremental,
             file_size,
         }
     }
@@ -104,7 +96,6 @@ impl PersistentLayerDesc {
             key_range,
             lsn_range,
             is_delta: true,
-            is_incremental: true,
             file_size,
         }
     }
@@ -170,8 +161,12 @@ impl PersistentLayerDesc {
         self.tenant_id
     }
 
+    /// Does this layer only contain some data for the key-range (incremental),
+    /// or does it contain a version of every page? This is important to know
+    /// for garbage collecting old layers: an incremental layer depends on
+    /// the previous non-incremental layer.
     pub fn is_incremental(&self) -> bool {
-        self.is_incremental
+        self.is_delta
     }
 
     pub fn is_delta(&self) -> bool {
@@ -188,7 +183,7 @@ impl PersistentLayerDesc {
             self.lsn_range.start,
             self.lsn_range.end,
             self.is_delta,
-            self.is_incremental,
+            self.is_incremental(),
             self.file_size,
         );
 

pageserver/src/tenant/storage_layer/remote_layer.rs

@@ -60,7 +60,7 @@ impl std::fmt::Debug for RemoteLayer {
         f.debug_struct("RemoteLayer")
             .field("file_name", &self.desc.filename())
             .field("layer_metadata", &self.layer_metadata)
-            .field("is_incremental", &self.desc.is_incremental)
+            .field("is_incremental", &self.desc.is_incremental())
             .finish()
     }
 }
@@ -151,7 +151,6 @@ impl RemoteLayer {
                 timelineid,
                 fname.key_range.clone(),
                 fname.lsn,
-                false,
                 layer_metadata.file_size(),
             ),
             layer_metadata: layer_metadata.clone(),

pageserver/src/tenant/timeline.rs

@@ -3149,7 +3149,6 @@ impl Timeline {
                     self.tenant_id,
                     &img_range,
                     lsn,
-                    false, // image layer always covers the full range
                 )?;
 
                 fail_point!("image-layer-writer-fail-before-finish", |_| {