From 805bb198c287a5a1ac3e28627165313335c69cc9 Mon Sep 17 00:00:00 2001
From: sharnoff <sharnoff@neon.tech>
Date: Fri, 23 Sep 2022 11:49:28 -0700
Subject: [PATCH] Miscellaneous small fixups (#2503)

Changes are:

 * Correct typo "firts" -> "first"
 * Change <empty panic with comment explaining> to <panic with message
   taken from the comment>
 * Fix weird indentation that rustfmt was failing to handle
 * Use existing `anyhow::{anyhow,bail}!` as `{anyhow,bail}!` if it's
   already in scope
 * Spell `Result<T, anyhow::Error>` as `anyhow::Result<T>`
   * In general, closer to matching the rest of the codebase
 * Change usages of `hash_map::Entry` to `Entry` when it's already in
   scope
   * A quick search shows our style on this one varies across the files
     it's used in
---
 pageserver/src/tenant.rs          | 23 +++++++++++------------
 pageserver/src/tenant/timeline.rs |  8 +++++---
 2 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/pageserver/src/tenant.rs b/pageserver/src/tenant.rs
index ed41641277..c9ad3bf232 100644
--- a/pageserver/src/tenant.rs
+++ b/pageserver/src/tenant.rs
@@ -17,7 +17,6 @@ use tracing::*;
 use utils::crashsafe_dir::path_with_suffix_extension;
 
 use std::cmp::min;
-use std::collections::hash_map;
 use std::collections::hash_map::Entry;
 use std::collections::BTreeSet;
 use std::collections::HashMap;
@@ -246,12 +245,12 @@ impl Tenant {
                     let ancestor_ancestor_lsn = ancestor_timeline.get_ancestor_lsn();
                     if ancestor_ancestor_lsn > *lsn {
                         // can we safely just branch from the ancestor instead?
-                        anyhow::bail!(
-                    "invalid start lsn {} for ancestor timeline {}: less than timeline ancestor lsn {}",
-                    lsn,
-                    ancestor_timeline_id,
-                    ancestor_ancestor_lsn,
-                );
+                        bail!(
+                            "invalid start lsn {} for ancestor timeline {}: less than timeline ancestor lsn {}",
+                            lsn,
+                            ancestor_timeline_id,
+                            ancestor_ancestor_lsn,
+                        );
                     }
                 }
 
@@ -406,11 +405,11 @@ impl Tenant {
                 .with_context(|| format!("Failed to initialize timeline {timeline_id}"))?;
 
             match timelines_accessor.entry(timeline.timeline_id) {
-                hash_map::Entry::Occupied(_) => anyhow::bail!(
+                Entry::Occupied(_) => bail!(
                     "Found freshly initialized timeline {} in the tenant map",
                     timeline.timeline_id
                 ),
-                hash_map::Entry::Vacant(v) => {
+                Entry::Vacant(v) => {
                     v.insert(timeline);
                 }
             }
@@ -768,7 +767,7 @@ impl Tenant {
                 })
                 .with_context(|| {
                     format!(
-                        "Failed to fsync on firts save for config {}",
+                        "Failed to fsync on first save for config {}",
                         target_config_path.display()
                     )
                 })?;
@@ -1091,11 +1090,11 @@ impl Tenant {
             })?;
 
         match timelines.entry(new_timeline_id) {
-            hash_map::Entry::Occupied(_) => anyhow::bail!(
+            Entry::Occupied(_) => bail!(
                 "Found freshly initialized timeline {} in the tenant map",
                 new_timeline_id
             ),
-            hash_map::Entry::Vacant(v) => {
+            Entry::Vacant(v) => {
                 v.insert(Arc::clone(&new_timeline));
             }
         }
diff --git a/pageserver/src/tenant/timeline.rs b/pageserver/src/tenant/timeline.rs
index 019de81d64..74e873e632 100644
--- a/pageserver/src/tenant/timeline.rs
+++ b/pageserver/src/tenant/timeline.rs
@@ -343,7 +343,9 @@ impl Timeline {
                 match cached_lsn.cmp(&lsn) {
                     Ordering::Less => {} // there might be WAL between cached_lsn and lsn, we need to check
                     Ordering::Equal => return Ok(cached_img), // exact LSN match, return the image
-                    Ordering::Greater => panic!(), // the returned lsn should never be after the requested lsn
+                    Ordering::Greater => {
+                        unreachable!("the returned lsn should never be after the requested lsn")
+                    }
                 }
                 Some((cached_lsn, cached_img))
             }
@@ -726,10 +728,10 @@ impl Timeline {
         Ok(())
     }
 
-    pub fn layer_removal_guard(&self) -> Result<MutexGuard<()>, anyhow::Error> {
+    pub fn layer_removal_guard(&self) -> anyhow::Result<MutexGuard<()>> {
         self.layer_removal_cs
             .try_lock()
-            .map_err(|e| anyhow::anyhow!("cannot lock compaction critical section {e}"))
+            .map_err(|e| anyhow!("cannot lock compaction critical section {e}"))
     }
 
     /// Retrieve current logical size of the timeline.