preserve data in parent branch that might be referenced in child branch

pageserver/src/layered_repository.rs

@@ -1424,15 +1424,21 @@ impl LayeredTimeline {
             }
 
             // 2. Is it needed by a child branch?
+            // NOTE With that wee would keep data that
+            // might be referenced by child branches forever.
+            // We can track this in child timeline GC and delete parent layers when
+            // they are no longer needed. This might be complicated with long inheritance chains.
             for retain_lsn in &retain_lsns {
-                // start_lsn is inclusive and end_lsn is exclusive
-                if l.get_start_lsn() <= *retain_lsn && *retain_lsn < l.get_end_lsn() {
+                // start_lsn is inclusive
+                if &l.get_start_lsn() <= retain_lsn {
                     info!(
-                        "keeping {} {}-{} because it's needed by branch point {}",
+                        "keeping {} {}-{} because it's still might be referenced by child branch forked at {} is_dropped: {} is_incremental: {}",
                         seg,
                         l.get_start_lsn(),
                         l.get_end_lsn(),
-                        *retain_lsn
+                        retain_lsn,
+                        l.is_dropped(),
+                        l.is_incremental(),
                     );
                     if seg.rel.is_relation() {
                         result.ondisk_relfiles_needed_by_branches += 1;

pageserver/src/layered_repository/delta_layer.rs

@@ -203,7 +203,7 @@ impl Layer for DeltaLayer {
             let page_version_reader = inner
                 .book
                 .as_ref()
-                .unwrap()
+                .expect("should be loaded in load call above")
                 .chapter_reader(PAGE_VERSIONS_CHAPTER)?;
 
             // Scan the metadata BTreeMap backwards, starting from the given entry.
@@ -530,7 +530,7 @@ impl DeltaLayer {
 
         *inner = DeltaLayerInner {
             loaded: true,
-            book: None,
+            book: Some(book),
             page_version_metas,
             relsizes,
         };

pageserver/src/repository.rs

@@ -829,6 +829,23 @@ mod tests {
                 .to_string()
                 .contains("tried to request a page version that was garbage collected")),
         }
+        Ok(())
+    }
+
+    #[test]
+    fn test_retain_data_in_parent_which_is_needed_for_child() -> Result<()> {
+        let repo =
+            RepoHarness::create("test_retain_data_in_parent_which_is_needed_for_child")?.load();
+        let tline = repo.create_empty_timeline(TIMELINE_ID)?;
+
+        make_some_layers(&tline)?;
+
+        repo.branch_timeline(TIMELINE_ID, NEW_TIMELINE_ID, Lsn(0x40))?;
+        let newtline = repo.get_timeline(NEW_TIMELINE_ID)?;
+
+        // this removes layers before lsn 40 (50 minus 10), so there are two remaining layers, image and delta for 31-50
+        repo.gc_iteration(Some(TIMELINE_ID), 0x10, false)?;
+        assert!(newtline.get_page_at_lsn(TESTREL_A, 0, Lsn(0x25)).is_ok());
 
         Ok(())
     }