From a5d57ca10b6746b957bb81aea2ee4f17e8611477 Mon Sep 17 00:00:00 2001
From: anastasia <lubennikovaav@gmail.com>
Date: Thu, 5 Aug 2021 19:38:19 +0300
Subject: [PATCH] list_nonrels() returns elements in arbitrary order. Remove
 incorrect comments that say otherwise.

---
 pageserver/src/basebackup.rs         | 15 ++++-----------
 pageserver/src/object_repository.rs  |  2 +-
 pageserver/src/object_store.rs       |  1 -
 pageserver/src/relish.rs             |  2 +-
 pageserver/src/restore_local_repo.rs |  3 +++
 pageserver/src/rocksdb_storage.rs    |  1 +
 6 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/pageserver/src/basebackup.rs b/pageserver/src/basebackup.rs
index 08eea4459a..99ff58efae 100644
--- a/pageserver/src/basebackup.rs
+++ b/pageserver/src/basebackup.rs
@@ -70,9 +70,6 @@ impl<'a> Basebackup<'a> {
         }
 
         // Gather non-relational files from object storage pages.
-        // Iteration is sorted order: all objects of the same type are grouped and traversed
-        // in key ascending order. For example all pg_xact records precede pg_multixact records and are sorted by block number.
-        // It allows to easily construct SLRU segments.
         for obj in self.timeline.list_nonrels(self.lsn)? {
             match obj {
                 RelishTag::Slru { slru, segno } => {
@@ -96,7 +93,7 @@ impl<'a> Basebackup<'a> {
     }
 
     //
-    // Generate SLRU segment files from repository. Path identifies SLRU kind (pg_xact, pg_multixact/members, ...).
+    // Generate SLRU segment files from repository.
     //
     fn add_slru_segment(&mut self, slru: SlruKind, segno: u32) -> anyhow::Result<()> {
         let nblocks = self
@@ -116,15 +113,11 @@ impl<'a> Basebackup<'a> {
             slru_buf.extend_from_slice(&img);
         }
 
-        let dir = match slru {
-            SlruKind::Clog => "pg_xact",
-            SlruKind::MultiXactMembers => "pg_multixact/members",
-            SlruKind::MultiXactOffsets => "pg_multixact/offsets",
-        };
-
-        let segname = format!("{}/{:>04X}", dir, segno);
+        let segname = format!("{}/{:>04X}", slru.to_str(), segno);
         let header = new_tar_header(&segname, slru_buf.len() as u64)?;
         self.ar.append(&header, slru_buf.as_slice())?;
+
+        trace!("Added to basebackup slru {} relsize {}", segname, nblocks);
         Ok(())
     }
 
diff --git a/pageserver/src/object_repository.rs b/pageserver/src/object_repository.rs
index 4000973d6d..184c5509cf 100644
--- a/pageserver/src/object_repository.rs
+++ b/pageserver/src/object_repository.rs
@@ -843,7 +843,7 @@ impl ObjectTimeline {
                 }
             }
         } else {
-            info!("relation {} not found at {}", rel, lsn);
+            trace!("relation {} not found at {}", rel, lsn);
             Ok(None)
         }
     }
diff --git a/pageserver/src/object_store.rs b/pageserver/src/object_store.rs
index f8fcc07793..b3927ba325 100644
--- a/pageserver/src/object_store.rs
+++ b/pageserver/src/object_store.rs
@@ -72,7 +72,6 @@ pub trait ObjectStore: Send + Sync {
     /// Iterate through non-rel relishes
     ///
     /// This is used to prepare tarball for new node startup.
-    /// Returns objects in increasing key-version order.
     fn list_nonrels<'a>(&'a self, timelineid: ZTimelineId, lsn: Lsn) -> Result<HashSet<RelishTag>>;
 
     /// Iterate through objects tags. If nonrel_only, then only non-relationa data is iterated.
diff --git a/pageserver/src/relish.rs b/pageserver/src/relish.rs
index 7af086c35c..252a9122c0 100644
--- a/pageserver/src/relish.rs
+++ b/pageserver/src/relish.rs
@@ -195,7 +195,7 @@ pub enum SlruKind {
 }
 
 impl SlruKind {
-    fn to_str(&self) -> &'static str {
+    pub fn to_str(&self) -> &'static str {
         match self {
             Self::Clog => "pg_xact",
             Self::MultiXactMembers => "pg_multixact/members",
diff --git a/pageserver/src/restore_local_repo.rs b/pageserver/src/restore_local_repo.rs
index ad9eee38db..deb353a6c9 100644
--- a/pageserver/src/restore_local_repo.rs
+++ b/pageserver/src/restore_local_repo.rs
@@ -557,7 +557,10 @@ fn save_xlog_dbase_create(timeline: &dyn Timeline, lsn: Lsn, rec: &XlCreateDatab
 
         num_rels_copied += 1;
     }
+
     // Copy relfilemap
+    // TODO This implementation is very inefficient -
+    // it scans all non-rels only to find FileNodeMaps
     for tag in timeline.list_nonrels(req_lsn)? {
         match tag {
             RelishTag::FileNodeMap { spcnode, dbnode } => {
diff --git a/pageserver/src/rocksdb_storage.rs b/pageserver/src/rocksdb_storage.rs
index 4d2aaf029c..92c36f8567 100644
--- a/pageserver/src/rocksdb_storage.rs
+++ b/pageserver/src/rocksdb_storage.rs
@@ -212,6 +212,7 @@ impl ObjectStore for RocksObjectStore {
     }
 
     /// Get a list of all distinct NON-relations in timeline
+    /// that are visible at given lsn.
     ///
     /// TODO: This implementation is very inefficient, it scans
     /// through all non-rel page versions in the system. In practice, this