From b45d5368b0f413671d29b025f4d8b711ecbdf259 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Wed, 1 Sep 2021 09:42:18 +0300
Subject: [PATCH] Don't create an image layer for dropped relations.

I noticed that the timeline directory contained files like this:

    pg_xact_0000_0_000000000169C3C2_00000000016BB399
    pg_xact_0000_0_00000000016BB399
    pg_xact_0000_0_00000000016BB399_00000000016BDD06
    pg_xact_0000_0_00000000016BDD06
    pg_xact_0000_0_00000000016BDD06_00000000016C63AA
    pg_xact_0000_0_00000000016C63AA
    pg_xact_0000_0_00000000016C63AA_0000000001765226_DROPPED
    pg_xact_0000_0_0000000001765226
    pg_xact_0001_0_00000000016BB77E_00000000016BDD06
    pg_xact_0001_0_00000000016BDD06
    pg_xact_0001_0_00000000016BDD06_0000000001765226_DROPPED
    pg_xact_0001_0_0000000001765226

Note how there is an image file after each DROPPED file. It's a waste of
time and space to materialize an image of the file at the point where it's
dropped, no one is going to request pages on a dropped relation. And it's
a correctness issue too: list_rels() and list_nonrels() will not consider
the relation as unlinked, unless the latest layer indicates so, and there
is no concept of a dropped image layer. That was causing test_clog_truncate
test to fail, when I adjusted the checkpointer to force a checkpoint more
aggressively.

There are a bunch more issues related to dropped rels and branching,
see https://github.com/zenithdb/zenith/issues/502. Hence this doesn't
completely fix the issue I saw with test_clog_truncate either. But it's
a start.
---
 .../src/layered_repository/inmemory_layer.rs  | 55 ++++++++++---------
 1 file changed, 28 insertions(+), 27 deletions(-)

diff --git a/pageserver/src/layered_repository/inmemory_layer.rs b/pageserver/src/layered_repository/inmemory_layer.rs
index 941b3ba3f3..e99b41bdb6 100644
--- a/pageserver/src/layered_repository/inmemory_layer.rs
+++ b/pageserver/src/layered_repository/inmemory_layer.rs
@@ -490,12 +490,6 @@ impl InMemoryLayer {
 
         let mut frozen_layers: Vec<Arc<dyn Layer>> = Vec::new();
 
-        // write a new base image layer at the cutoff point
-        let imgfile = ImageLayer::create_from_src(self.conf, timeline, self, end_lsn)?;
-        let imgfile_rc: Arc<dyn Layer> = Arc::new(imgfile);
-        frozen_layers.push(Arc::clone(&imgfile_rc));
-        trace!("freeze: created image layer {} at {}", self.seg, end_lsn);
-
         if self.start_lsn != end_lsn {
             // Write the page versions before the cutoff to disk.
             let delta_layer = DeltaLayer::create(
@@ -522,29 +516,36 @@ impl InMemoryLayer {
             assert!(before_page_versions.is_empty());
         }
 
-        // If there were any "new" page versions, initialize a new in-memory layer to hold
-        // them
-        let new_open = if !after_segsizes.is_empty() || !after_page_versions.is_empty() {
-            let new_open = Self::create_successor_layer(
-                self.conf,
-                imgfile_rc,
-                self.timelineid,
-                self.tenantid,
-                end_lsn,
-                end_lsn,
-            )?;
-            let mut new_inner = new_open.inner.lock().unwrap();
-            new_inner.page_versions.append(&mut after_page_versions);
-            new_inner.segsizes.append(&mut after_segsizes);
-            drop(new_inner);
-            trace!("freeze: created new in-mem layer {} {}-", self.seg, end_lsn);
+        let mut new_open_rc = None;
+        if !dropped {
+            // Write a new base image layer at the cutoff point
+            let imgfile = ImageLayer::create_from_src(self.conf, timeline, self, end_lsn)?;
+            let imgfile_rc: Arc<dyn Layer> = Arc::new(imgfile);
+            frozen_layers.push(Arc::clone(&imgfile_rc));
+            trace!("freeze: created image layer {} at {}", self.seg, end_lsn);
 
-            Some(Arc::new(new_open))
-        } else {
-            None
-        };
+            // If there were any page versions newer than the cutoff, initialize a new in-memory
+            // layer to hold them
+            if !after_segsizes.is_empty() || !after_page_versions.is_empty() {
+                let new_open = Self::create_successor_layer(
+                    self.conf,
+                    imgfile_rc,
+                    self.timelineid,
+                    self.tenantid,
+                    end_lsn,
+                    end_lsn,
+                )?;
+                let mut new_inner = new_open.inner.lock().unwrap();
+                new_inner.page_versions.append(&mut after_page_versions);
+                new_inner.segsizes.append(&mut after_segsizes);
+                drop(new_inner);
+                trace!("freeze: created new in-mem layer {} {}-", self.seg, end_lsn);
 
-        Ok((frozen_layers, new_open))
+                new_open_rc = Some(Arc::new(new_open))
+            }
+        }
+
+        Ok((frozen_layers, new_open_rc))
     }
 
     /// debugging function to print out the contents of the layer