simplify SmgrOpTimerState variant names, and add some doc comments; https://github.com/neondatabase/neon/pull/10353#discussion_r1913676569 and https://github.com/neondatabase/neon/pull/10353#discussion_r1913676824

pageserver/src/metrics.rs

@@ -1238,6 +1238,16 @@ pub(crate) struct SmgrOpTimerInner {
     timings: SmgrOpTimerState,
 }
 
+/// The stages of request processing are represented by the enum variants.
+/// Used as part of [`SmgrOpTimerInner::timings`].
+///
+/// Request processing calls into the `SmgrOpTimer::observe_*` methods at the
+/// transition points.
+/// These methods bump relevant counters and then update [`SmgrOpTimerInner::timings`]
+/// to the next state.
+///
+/// Each request goes through every stage, in all configurations.
+///
 #[derive(Debug)]
 enum SmgrOpTimerState {
     Received {
@@ -1247,13 +1257,13 @@ enum SmgrOpTimerState {
         #[allow(dead_code)]
         received_at: Instant,
     },
-    ParsedRoutedNowThrottling {
+    Throttling {
         throttle_started_at: Instant,
     },
-    ParsedRoutedThrottledNowBatching {
+    Batching {
         throttle_done_at: Instant,
     },
-    BatchedNowExecuting {
+    Executing {
         execution_started_at: Instant,
     },
     Flushing,
@@ -1262,6 +1272,7 @@ enum SmgrOpTimerState {
 
 // NB: when adding observation points, remember to update the Drop impl.
 impl SmgrOpTimer {
+    /// See [`SmgrOpTimerState`] for more context.
     pub(crate) fn observe_throttle_start(&mut self, at: Instant) {
         let Some(inner) = self.0.as_mut() else {
             return;
@@ -1270,15 +1281,17 @@ impl SmgrOpTimer {
             return;
         };
         inner.throttling.count_accounted_start.inc();
-        inner.timings = SmgrOpTimerState::ParsedRoutedNowThrottling {
+        inner.timings = SmgrOpTimerState::Throttling {
             throttle_started_at: at,
         };
     }
+
+    /// See [`SmgrOpTimerState`] for more context.
     pub(crate) fn observe_throttle_done(&mut self, throttle: ThrottleResult) {
         let Some(inner) = self.0.as_mut() else {
             return;
         };
-        let SmgrOpTimerState::ParsedRoutedNowThrottling {
+        let SmgrOpTimerState::Throttling {
             throttle_started_at,
         } = &mut inner.timings
         else {
@@ -1287,7 +1300,7 @@ impl SmgrOpTimer {
         inner.throttling.count_accounted_finish.inc();
         match throttle {
             ThrottleResult::NotThrottled { end } => {
-                inner.timings = SmgrOpTimerState::ParsedRoutedThrottledNowBatching {
+                inner.timings = SmgrOpTimerState::Batching {
                     throttle_done_at: end,
                 };
             }
@@ -1299,20 +1312,19 @@ impl SmgrOpTimer {
                     .wait_time
                     .inc_by((end - *throttle_started_at).as_micros().try_into().unwrap());
                 // state transition
-                inner.timings = SmgrOpTimerState::ParsedRoutedThrottledNowBatching {
+                inner.timings = SmgrOpTimerState::Batching {
                     throttle_done_at: end,
                 };
             }
         }
     }
 
+    /// See [`SmgrOpTimerState`] for more context.
     pub(crate) fn observe_execution_start(&mut self, at: Instant) {
         let Some(inner) = self.0.as_mut() else {
             return;
         };
-        let SmgrOpTimerState::ParsedRoutedThrottledNowBatching { throttle_done_at } =
-            &mut inner.timings
-        else {
+        let SmgrOpTimerState::Batching { throttle_done_at } = &mut inner.timings else {
             return;
         };
         // update metrics
@@ -1322,13 +1334,15 @@ impl SmgrOpTimer {
             .per_timeline_batch_wait_time
             .observe(batch.as_secs_f64());
         // state transition
-        inner.timings = SmgrOpTimerState::BatchedNowExecuting {
+        inner.timings = SmgrOpTimerState::Executing {
             execution_started_at: at,
         }
     }
 
     /// For all but the first caller, this is a no-op.
     /// The first callers receives Some, subsequent ones None.
+    ///
+    /// See [`SmgrOpTimerState`] for more context.
     pub(crate) fn observe_execution_end_flush_start(
         &mut self,
         at: Instant,
@@ -1338,7 +1352,7 @@ impl SmgrOpTimer {
         let Some(mut inner) = self.0.take() else {
             return None;
         };
-        let SmgrOpTimerState::BatchedNowExecuting {
+        let SmgrOpTimerState::Executing {
             execution_started_at,
         } = &mut inner.timings
         else {
@@ -1373,6 +1387,14 @@ impl SmgrOpTimer {
     }
 }
 
+/// The last stage of request processing is serializing and flushing the request
+/// into the TCP connection. We want to make slow flushes observable
+/// _while they are occuring_, so this struct provides a wrapper method [`Self::measure`]
+/// to periodically bump the metric.
+///
+/// If in the future we decide that we're not interested in live updates, we can
+/// add another `observe_*` method to [`SmgrOpTimer`], follow the existing pattern there,
+/// and remove this struct from the code base.
 pub(crate) struct SmgrOpFlushInProgress {
     flush_started_at: Instant,
     global_micros: IntCounter,