From e3481bfcae00697eafa078982bec8053a37e6fe3 Mon Sep 17 00:00:00 2001
From: Christian Schwarz <christian@neon.tech>
Date: Fri, 7 Feb 2025 03:02:07 +0100
Subject: [PATCH] undo that last piece

---
 pageserver/src/metrics.rs      | 31 ++++++++-----------------------
 pageserver/src/page_service.rs |  4 ++--
 2 files changed, 10 insertions(+), 25 deletions(-)

diff --git a/pageserver/src/metrics.rs b/pageserver/src/metrics.rs
index 58959f0e7a..779ebdcde7 100644
--- a/pageserver/src/metrics.rs
+++ b/pageserver/src/metrics.rs
@@ -1223,8 +1223,7 @@ pub(crate) mod virtual_file_io_engine {
     });
 }
 
-pub(crate) struct SmgrOpTimer(Option<SmgrOpTimerInner>);
-pub(crate) struct SmgrOpTimerInner {
+pub(crate) struct SmgrOpTimer {
     global_latency_histo: Histogram,
 
     // Optional because not all op types are tracked per-timeline
@@ -1240,19 +1239,13 @@ impl SmgrOpTimer {
         let Some(throttle) = throttle else {
             return;
         };
-        let inner = self.0.as_mut().expect("other public methods consume self");
-        inner.throttled += *throttle;
+        self.throttled += *throttle;
     }
+}
 
-    pub(crate) fn observe_smgr_op_completion_and_start_flushing(mut self) {
-        let (flush_start, inner) = self
-            .smgr_op_end()
-            .expect("this method consume self, and the only other caller is drop handler");
-    }
-
-    /// Returns `None`` if this method has already been called, `Some` otherwise.
-    fn smgr_op_end(&mut self) -> Option<(Instant, SmgrOpTimerInner)> {
-        let inner = self.0.take()?;
+impl Drop for SmgrOpTimer {
+    fn drop(&mut self) {
+        let inner = self;
 
         let now = Instant::now();
         let elapsed = now - inner.start;
@@ -1282,14 +1275,6 @@ impl SmgrOpTimer {
         if let Some(per_timeline_getpage_histo) = &inner.per_timeline_latency_histo {
             per_timeline_getpage_histo.observe(elapsed);
         }
-
-        Some((now, inner))
-    }
-}
-
-impl Drop for SmgrOpTimer {
-    fn drop(&mut self) {
-        self.smgr_op_end();
     }
 }
 
@@ -1571,13 +1556,13 @@ impl SmgrQueryTimePerTimeline {
             None
         };
 
-        SmgrOpTimer(Some(SmgrOpTimerInner {
+        SmgrOpTimer {
             global_latency_histo: self.global_latency[op as usize].clone(),
             per_timeline_latency_histo,
             start: started_at,
             op,
             throttled: Duration::ZERO,
-        }))
+        }
     }
 
     pub(crate) fn observe_getpage_batch_start(&self, batch_size: usize) {
diff --git a/pageserver/src/page_service.rs b/pageserver/src/page_service.rs
index e577b4221d..880d0f5692 100644
--- a/pageserver/src/page_service.rs
+++ b/pageserver/src/page_service.rs
@@ -1071,8 +1071,6 @@ impl PageServerHandler {
             // The timer's underlying metric is used for a storage-internal latency SLO and
             // we don't want to include latency in it that we can't control.
             // And as pointed out above, in this case, we don't control the time that flush will take.
-            let flushing_timer =
-                timer.map(|timer| timer.observe_smgr_op_completion_and_start_flushing());
 
             // what we want to do
             let flush_fut = pgb_writer.flush();
@@ -1094,6 +1092,8 @@ impl PageServerHandler {
             // and log the info! line inside the request span
             // .instrument(span.clone())
             .await?;
+
+            drop(timer);
         }
         Ok(())
     }