Misc cosmetic fixes in comments, messages.

Most of these were extracted from PR #2785.

pageserver/src/storage_sync/index.rs

@@ -335,7 +335,8 @@ impl LayerFileMetadata {
     }
 }
 
-/// Part of the remote index, corresponding to a certain timeline.
+/// In-memory representation of an `index_part.json` file
+///
 /// Contains the data about all files in the timeline, present remotely and its metadata.
 ///
 /// This type needs to be backwards and forwards compatible. When changing the fields,
@@ -366,8 +367,11 @@ pub struct IndexPart {
     #[serde(default)]
     layer_metadata: HashMap<RelativePath, IndexLayerMetadata>,
 
+    // 'disk_consistent_lsn' is a copy of the 'disk_consistent_lsn' in the metadata.
+    // It's duplicated here for convenience.
     #[serde_as(as = "DisplayFromStr")]
     disk_consistent_lsn: Lsn,
+
     metadata_bytes: Vec<u8>,
 }
 

pageserver/src/tenant.rs

@@ -157,12 +157,14 @@ struct TimelineUninitMark {
 }
 
 impl UninitializedTimeline<'_> {
-    /// Ensures timeline data is valid, loads it into pageserver's memory and removes uninit mark file on success.
+    /// Ensures timeline data is valid, loads it into pageserver's memory and removes
+    /// uninit mark file on success.
     pub fn initialize(self) -> anyhow::Result<Arc<Timeline>> {
         let mut timelines = self.owning_tenant.timelines.lock().unwrap();
         self.initialize_with_lock(&mut timelines, true)
     }
 
+    /// Like `initialize`, but the caller is already holding lock on Tenant::timelines.
     fn initialize_with_lock(
         mut self,
         timelines: &mut HashMap<TimelineId, Arc<Timeline>>,
@@ -350,8 +352,6 @@ impl Drop for TimelineUninitMark {
     }
 }
 
-/// A repository corresponds to one .neon directory. One repository holds multiple
-/// timelines, forked off from the same initial call to 'initdb'.
 impl Tenant {
     pub fn tenant_id(&self) -> TenantId {
         self.tenant_id
@@ -617,7 +617,7 @@ impl Tenant {
                 local_timeline_directory.display()
             )
         })?;
-        info!("detach removed files");
+        info!("finished deleting layer files, releasing layer_removal_cs.lock()");
 
         drop(layer_removal_guard);
         timeline_entry.remove();

pageserver/src/tenant/timeline.rs

@@ -14,7 +14,7 @@ use std::cmp::{max, min, Ordering};
 use std::collections::{HashMap, HashSet};
 use std::fs;
 use std::ops::{Deref, Range};
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 use std::sync::atomic::{self, AtomicBool, AtomicI64, Ordering as AtomicOrdering};
 use std::sync::{Arc, Mutex, MutexGuard, RwLock};
 use std::time::{Duration, Instant, SystemTime};
@@ -501,7 +501,7 @@ impl Timeline {
     pub fn compact(&self) -> anyhow::Result<()> {
         let last_record_lsn = self.get_last_record_lsn();
 
-        // Last record Lsn could be zero in case the timelie was just created
+        // Last record Lsn could be zero in case the timeline was just created
         if !last_record_lsn.is_valid() {
             warn!("Skipping compaction for potentially just initialized timeline, it has invalid last record lsn: {last_record_lsn}");
             return Ok(());
@@ -909,7 +909,7 @@ impl Timeline {
                         imgfilename, self.timeline_id, disk_consistent_lsn
                     );
 
-                    rename_to_backup(direntry.path())?;
+                    rename_to_backup(&direntry.path())?;
                     continue;
                 }
 
@@ -933,7 +933,7 @@ impl Timeline {
                         deltafilename, self.timeline_id, disk_consistent_lsn
                     );
 
-                    rename_to_backup(direntry.path())?;
+                    rename_to_backup(&direntry.path())?;
                     continue;
                 }
 
@@ -2463,12 +2463,12 @@ impl<'a> TimelineWriter<'a> {
 
 /// Add a suffix to a layer file's name: .{num}.old
 /// Uses the first available num (starts at 0)
-fn rename_to_backup(path: PathBuf) -> anyhow::Result<()> {
+fn rename_to_backup(path: &Path) -> anyhow::Result<()> {
     let filename = path
         .file_name()
         .ok_or_else(|| anyhow!("Path {} don't have a file name", path.display()))?
         .to_string_lossy();
-    let mut new_path = path.clone();
+    let mut new_path = path.to_owned();
 
     for i in 0u32.. {
         new_path.set_file_name(format!("{}.{}.old", filename, i));

pageserver/src/tenant_mgr.rs

@@ -420,7 +420,10 @@ pub fn get_tenant(tenant_id: TenantId, active_only: bool) -> anyhow::Result<Arc<
         .get(&tenant_id)
         .with_context(|| format!("Tenant {tenant_id} not found in the local state"))?;
     if active_only && !tenant.is_active() {
-        anyhow::bail!("Tenant {tenant_id} is not active")
+        anyhow::bail!(
+            "Tenant {tenant_id} is not active. Current state: {:?}",
+            tenant.current_state()
+        )
     } else {
         Ok(Arc::clone(tenant))
     }

test_runner/regress/test_remote_storage.py

@@ -93,8 +93,8 @@ def test_remote_storage_backup_and_restore(
         # run checkpoint manually to be sure that data landed in remote storage
         pageserver_http.timeline_checkpoint(tenant_id, timeline_id)
 
-        log.info(f"waiting for checkpoint {checkpoint_number} upload")
         # wait until pageserver successfully uploaded a checkpoint to remote storage
+        log.info(f"waiting for checkpoint {checkpoint_number} upload")
         wait_for_upload(client, tenant_id, timeline_id, current_lsn)
         log.info(f"upload of checkpoint {checkpoint_number} is done")