From a10ba532dd0a64727305fa62b83613f5603f6f62 Mon Sep 17 00:00:00 2001
From: Konstantin Knizhnik <knizhnik@garret.ru>
Date: Wed, 17 May 2023 14:19:56 +0300
Subject: [PATCH] Reduce write amplification for wanted image layers

---
 pageserver/src/keyspace.rs        | 19 +++++++++++++++++++
 pageserver/src/tenant.rs          |  8 ++++----
 pageserver/src/tenant/timeline.rs | 11 +++++++++--
 3 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/pageserver/src/keyspace.rs b/pageserver/src/keyspace.rs
index 20e6df9c7b..818cf9e55b 100644
--- a/pageserver/src/keyspace.rs
+++ b/pageserver/src/keyspace.rs
@@ -62,6 +62,25 @@ impl KeySpace {
         KeyPartitioning { parts }
     }
 
+    pub fn get_logical_size(&self, range: &Range<Key>) -> u64 {
+        let start = match self.ranges.binary_search_by_key(&range.start, |r| r.start) {
+            Err(0) => 0,
+            Ok(index) => index,
+            Err(index) => index - 1,
+        };
+        let mut size = 0u64;
+        for i in start..self.ranges.len() {
+            if self.ranges[i].start >= range.end {
+                break;
+            }
+            let n_blocks = key_range_size(&self.ranges[i]);
+            if n_blocks != u32::MAX {
+                size += n_blocks as u64 * BLCKSZ as u64;
+            }
+        }
+        size
+    }
+
     ///
     /// Check if key space contains overlapping range
     ///
diff --git a/pageserver/src/tenant.rs b/pageserver/src/tenant.rs
index 9ab0262407..51fddff141 100644
--- a/pageserver/src/tenant.rs
+++ b/pageserver/src/tenant.rs
@@ -2195,7 +2195,7 @@ impl Tenant {
                 // made.
                 break;
             }
-            let result = timeline.gc().await?;
+            let result = timeline.gc(ctx).await?;
             totals += result;
         }
 
@@ -3754,7 +3754,7 @@ mod tests {
                 .await?;
             tline.freeze_and_flush().await?;
             tline.compact(&ctx).await?;
-            tline.gc().await?;
+            tline.gc(&ctx).await?;
         }
 
         Ok(())
@@ -3826,7 +3826,7 @@ mod tests {
                 .await?;
             tline.freeze_and_flush().await?;
             tline.compact(&ctx).await?;
-            tline.gc().await?;
+            tline.gc(&ctx).await?;
         }
 
         Ok(())
@@ -3910,7 +3910,7 @@ mod tests {
                 .await?;
             tline.freeze_and_flush().await?;
             tline.compact(&ctx).await?;
-            tline.gc().await?;
+            tline.gc(&ctx).await?;
         }
 
         Ok(())
diff --git a/pageserver/src/tenant/timeline.rs b/pageserver/src/tenant/timeline.rs
index 51a059951e..7d37524412 100644
--- a/pageserver/src/tenant/timeline.rs
+++ b/pageserver/src/tenant/timeline.rs
@@ -3667,7 +3667,7 @@ impl Timeline {
     /// within a layer file. We can only remove the whole file if it's fully
     /// obsolete.
     ///
-    pub(super) async fn gc(&self) -> anyhow::Result<GcResult> {
+    pub(super) async fn gc(&self, ctx: &RequestContext) -> anyhow::Result<GcResult> {
         let timer = self.metrics.garbage_collect_histo.start_timer();
 
         fail_point!("before-timeline-gc");
@@ -3697,6 +3697,7 @@ impl Timeline {
                 pitr_cutoff,
                 retain_lsns,
                 new_gc_cutoff,
+                ctx,
             )
             .instrument(
                 info_span!("gc_timeline", timeline = %self.timeline_id, cutoff = %new_gc_cutoff),
@@ -3716,6 +3717,7 @@ impl Timeline {
         pitr_cutoff: Lsn,
         retain_lsns: Vec<Lsn>,
         new_gc_cutoff: Lsn,
+        ctx: &RequestContext,
     ) -> anyhow::Result<GcResult> {
         let now = SystemTime::now();
         let mut result: GcResult = GcResult::default();
@@ -3762,6 +3764,7 @@ impl Timeline {
 
         let mut layers_to_remove = Vec::new();
         let mut wanted_image_layers = KeySpaceRandomAccum::default();
+        let keyspace = self.collect_keyspace(new_gc_cutoff, ctx).await?;
 
         // Scan all layers in the timeline (remote or on-disk).
         //
@@ -3852,7 +3855,11 @@ impl Timeline {
                 // But image layers are in any case less sparse than delta layers. Also we need some
                 // protection from replacing recent image layers with new one after each GC iteration.
                 if l.is_incremental() && !LayerMap::is_l0(&*l) {
-                    wanted_image_layers.add_range(l.get_key_range());
+                    let layer_logical_size = keyspace.get_logical_size(&l.get_key_range());
+                    let layer_age = new_gc_cutoff.0 - l.get_lsn_range().end.0;
+                    if layer_logical_size < layer_age {
+                        wanted_image_layers.add_range(l.get_key_range());
+                    }
                 }
                 result.layers_not_updated += 1;
                 continue 'outer;