From e20976487797b70dc5e2bade74413c534737f235 Mon Sep 17 00:00:00 2001
From: Konstantin Knizhnik <knizhnik@garret.ru>
Date: Mon, 24 Jan 2022 10:42:40 +0300
Subject: [PATCH] Do not delete layers beyand cutoff LSN (#1128)

* Do not delete layers beyand cutoff LSN

* Update pageserver/src/layered_repository/layer_map.rs

Co-authored-by: Heikki Linnakangas <heikki.linnakangas@iki.fi>

Co-authored-by: Heikki Linnakangas <heikki.linnakangas@iki.fi>
---
 pageserver/src/layered_repository.rs           |  9 +++++++--
 pageserver/src/layered_repository/layer_map.rs | 14 ++++++++++----
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/pageserver/src/layered_repository.rs b/pageserver/src/layered_repository.rs
index 2871ae2db8..b76e9bc0ee 100644
--- a/pageserver/src/layered_repository.rs
+++ b/pageserver/src/layered_repository.rs
@@ -651,7 +651,6 @@ impl LayeredRepository {
                     timeline.checkpoint(CheckpointConfig::Forced)?;
                     info!("timeline {} checkpoint_before_gc done", timelineid);
                 }
-
                 let result = timeline.gc_timeline(branchpoints, cutoff)?;
 
                 totals += result;
@@ -1633,6 +1632,7 @@ impl LayeredTimeline {
     pub fn gc_timeline(&self, retain_lsns: Vec<Lsn>, cutoff: Lsn) -> Result<GcResult> {
         let now = Instant::now();
         let mut result: GcResult = Default::default();
+        let disk_consistent_lsn = self.get_disk_consistent_lsn();
 
         let _enter = info_span!("garbage collection", timeline = %self.timelineid, tenant = %self.tenantid, cutoff = %cutoff).entered();
 
@@ -1718,7 +1718,12 @@ impl LayeredTimeline {
             }
 
             // 3. Is there a later on-disk layer for this relation?
-            if !l.is_dropped() && !layers.newer_image_layer_exists(l.get_seg_tag(), l.get_end_lsn())
+            if !l.is_dropped()
+                && !layers.newer_image_layer_exists(
+                    l.get_seg_tag(),
+                    l.get_end_lsn(),
+                    disk_consistent_lsn,
+                )
             {
                 info!(
                     "keeping {} {}-{} because it is the latest layer",
diff --git a/pageserver/src/layered_repository/layer_map.rs b/pageserver/src/layered_repository/layer_map.rs
index 99a7319be3..dce2163388 100644
--- a/pageserver/src/layered_repository/layer_map.rs
+++ b/pageserver/src/layered_repository/layer_map.rs
@@ -191,9 +191,15 @@ impl LayerMap {
     ///
     /// This is used for garbage collection, to determine if an old layer can
     /// be deleted.
-    pub fn newer_image_layer_exists(&self, seg: SegmentTag, lsn: Lsn) -> bool {
+    /// We ignore segments newer than disk_consistent_lsn because they will be removed at restart
+    pub fn newer_image_layer_exists(
+        &self,
+        seg: SegmentTag,
+        lsn: Lsn,
+        disk_consistent_lsn: Lsn,
+    ) -> bool {
         if let Some(segentry) = self.segs.get(&seg) {
-            segentry.newer_image_layer_exists(lsn)
+            segentry.newer_image_layer_exists(lsn, disk_consistent_lsn)
         } else {
             false
         }
@@ -311,13 +317,13 @@ impl SegEntry {
         self.historic.search(lsn)
     }
 
-    pub fn newer_image_layer_exists(&self, lsn: Lsn) -> bool {
+    pub fn newer_image_layer_exists(&self, lsn: Lsn, disk_consistent_lsn: Lsn) -> bool {
         // We only check on-disk layers, because
         // in-memory layers are not durable
 
         self.historic
             .iter_newer(lsn)
-            .any(|layer| !layer.is_incremental())
+            .any(|layer| !layer.is_incremental() && layer.get_end_lsn() <= disk_consistent_lsn)
     }
 
     // Set new open layer for a SegEntry.