From 03dff207db65712c6b8cf6cb843f705fd4ab269e Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki@zenith.tech>
Date: Fri, 10 Sep 2021 10:24:35 +0300
Subject: [PATCH] Remove start_lsn arg from `create_empty_repository`.

Always use lsn(0) as the initial last_record_lsn. It is updated soon after
creating the timeline anyway, after loading the bootstrap data, so it
doesn't stay long in that state. I was a bit worried about using a special
value like 0, but it's actually nice that you can distinguish it from any
real LSN value. The unit tests have been using Lsn(0) as the initial start
LSN all along.
---
 pageserver/src/branches.rs           |  6 ++++--
 pageserver/src/layered_repository.rs | 12 +++---------
 pageserver/src/repository.rs         | 14 +++++---------
 3 files changed, 12 insertions(+), 20 deletions(-)

diff --git a/pageserver/src/branches.rs b/pageserver/src/branches.rs
index 163dffc827..dd01615eb9 100644
--- a/pageserver/src/branches.rs
+++ b/pageserver/src/branches.rs
@@ -221,11 +221,13 @@ fn bootstrap_timeline(
 
     info!("bootstrap_timeline {:?} at lsn {}", pgdata_path, lsn);
 
-    let timeline = repo.create_empty_timeline(tli, lsn)?;
+    // Import the contents of the data directory at the initial checkpoint
+    // LSN, and any WAL after that.
+    let timeline = repo.create_empty_timeline(tli)?;
     restore_local_repo::import_timeline_from_postgres_datadir(&pgdata_path, &*timeline, lsn)?;
 
     let wal_dir = pgdata_path.join("pg_wal");
-    restore_local_repo::import_timeline_wal(&wal_dir, &*timeline, timeline.get_last_record_lsn())?;
+    restore_local_repo::import_timeline_wal(&wal_dir, &*timeline, lsn)?;
 
     println!(
         "created initial timeline {} timeline.lsn {}",
diff --git a/pageserver/src/layered_repository.rs b/pageserver/src/layered_repository.rs
index 184fd06f66..afd955b11e 100644
--- a/pageserver/src/layered_repository.rs
+++ b/pageserver/src/layered_repository.rs
@@ -124,23 +124,17 @@ impl Repository for LayeredRepository {
         Ok(self.get_timeline_locked(timelineid, &mut timelines)?)
     }
 
-    fn create_empty_timeline(
-        &self,
-        timelineid: ZTimelineId,
-        start_lsn: Lsn,
-    ) -> Result<Arc<dyn Timeline>> {
+    fn create_empty_timeline(&self, timelineid: ZTimelineId) -> Result<Arc<dyn Timeline>> {
         let mut timelines = self.timelines.lock().unwrap();
 
-        assert!(start_lsn.is_aligned());
-
         // Create the timeline directory, and write initial metadata to file.
         std::fs::create_dir_all(self.conf.timeline_path(&timelineid, &self.tenantid))?;
 
         let metadata = TimelineMetadata {
-            disk_consistent_lsn: start_lsn,
+            disk_consistent_lsn: Lsn(0),
             prev_record_lsn: None,
             ancestor_timeline: None,
-            ancestor_lsn: start_lsn,
+            ancestor_lsn: Lsn(0),
         };
         Self::save_metadata(self.conf, timelineid, self.tenantid, &metadata)?;
 
diff --git a/pageserver/src/repository.rs b/pageserver/src/repository.rs
index 2e61ff98be..044eb5e721 100644
--- a/pageserver/src/repository.rs
+++ b/pageserver/src/repository.rs
@@ -17,11 +17,7 @@ pub trait Repository: Send + Sync {
     fn get_timeline(&self, timelineid: ZTimelineId) -> Result<Arc<dyn Timeline>>;
 
     /// Create a new, empty timeline. The caller is responsible for loading data into it
-    fn create_empty_timeline(
-        &self,
-        timelineid: ZTimelineId,
-        start_lsn: Lsn,
-    ) -> Result<Arc<dyn Timeline>>;
+    fn create_empty_timeline(&self, timelineid: ZTimelineId) -> Result<Arc<dyn Timeline>>;
 
     /// Branch a timeline
     fn branch_timeline(&self, src: ZTimelineId, dst: ZTimelineId, start_lsn: Lsn) -> Result<()>;
@@ -300,7 +296,7 @@ mod tests {
 
         // Create timeline to work on
         let timelineid = ZTimelineId::from_str("11223344556677881122334455667788").unwrap();
-        let tline = repo.create_empty_timeline(timelineid, Lsn(0x00))?;
+        let tline = repo.create_empty_timeline(timelineid)?;
 
         tline.put_page_image(TESTREL_A, 0, Lsn(0x20), TEST_IMG("foo blk 0 at 2"))?;
         tline.put_page_image(TESTREL_A, 0, Lsn(0x20), TEST_IMG("foo blk 0 at 2"))?;
@@ -416,7 +412,7 @@ mod tests {
     fn test_large_rel() -> Result<()> {
         let repo = get_test_repo("test_large_rel")?;
         let timelineid = ZTimelineId::from_str("11223344556677881122334455667788").unwrap();
-        let tline = repo.create_empty_timeline(timelineid, Lsn(0x00))?;
+        let tline = repo.create_empty_timeline(timelineid)?;
 
         let mut lsn = 0x10;
         for blknum in 0..pg_constants::RELSEG_SIZE + 1 {
@@ -482,7 +478,7 @@ mod tests {
     fn test_list_rels_drop() -> Result<()> {
         let repo = get_test_repo("test_list_rels_drop")?;
         let timelineid = ZTimelineId::from_str("11223344556677881122334455667788").unwrap();
-        let tline = repo.create_empty_timeline(timelineid, Lsn(0x00))?;
+        let tline = repo.create_empty_timeline(timelineid)?;
         const TESTDB: u32 = 111;
 
         // Import initial dummy checkpoint record, otherwise the get_timeline() call
@@ -539,7 +535,7 @@ mod tests {
     fn test_branch() -> Result<()> {
         let repo = get_test_repo("test_branch")?;
         let timelineid = ZTimelineId::from_str("11223344556677881122334455667788").unwrap();
-        let tline = repo.create_empty_timeline(timelineid, Lsn(0x00))?;
+        let tline = repo.create_empty_timeline(timelineid)?;
 
         // Import initial dummy checkpoint record, otherwise the get_timeline() call
         // after branching fails below