From 6f7d248419a26c48d17434e405fe1673a71687f0 Mon Sep 17 00:00:00 2001
From: Christian Schwarz <christian@neon.tech>
Date: Thu, 7 Dec 2023 14:44:31 +0000
Subject: [PATCH] Revert "half-way done impl to track residence duration per
 fd"

This reverts commit a1e818a8865ff0800df41ee703e54188e82173b7.
---
 pageserver/src/metrics.rs      |  6 ----
 pageserver/src/virtual_file.rs | 53 +++++++++-------------------------
 2 files changed, 14 insertions(+), 45 deletions(-)

diff --git a/pageserver/src/metrics.rs b/pageserver/src/metrics.rs
index 170afe9e9f..b684c4fa90 100644
--- a/pageserver/src/metrics.rs
+++ b/pageserver/src/metrics.rs
@@ -841,12 +841,6 @@ pub(crate) static STORAGE_IO_SIZE: Lazy<IntGaugeVec> = Lazy::new(|| {
     .expect("failed to define a metric")
 });
 
-pub(crate) mod virtual_file_descriptor_cache {
-
-    pub(crate) static FD_RESIDENCE_DURATION
-
-}
-
 #[derive(Debug)]
 struct GlobalAndPerTimelineHistogram {
     global: Histogram,
diff --git a/pageserver/src/virtual_file.rs b/pageserver/src/virtual_file.rs
index cc1d04f395..92a6fe396f 100644
--- a/pageserver/src/virtual_file.rs
+++ b/pageserver/src/virtual_file.rs
@@ -104,12 +104,7 @@ struct SlotInner {
     tag: u64,
 
     /// the underlying file
-    file: Option<FileInSlot>,
-}
-
-struct FileInSlot {
-    file: File,
-    inserted_at: Instant,
+    file: Option<File>,
 }
 
 impl OpenFiles {
@@ -159,17 +154,12 @@ impl OpenFiles {
         // We now have the victim slot locked. If it was in use previously, close the
         // old file.
         //
-        if let Some(FileInSlot { file, inserted_at }) = slot_guard.file.take() {
+        if let Some(old_file) = slot_guard.file.take() {
             // the normal path of dropping VirtualFile uses `Close`, use `CloseByReplace` here to
             // distinguish the two.
-            let start = Instant::new();
-            drop(file);
-            let end = Instant::new();
             STORAGE_IO_TIME_METRIC
                 .get(StorageIoOperation::CloseByReplace)
-                .observe(end.duration_since(start));
-            crate::metrics::virtual_file_descriptor_cache::observe_fd_residence_duration(inserted_at.duration_since(end))
-
+                .observe_closure_duration(|| drop(old_file));
         }
 
         // Prepare the slot for reuse and return it
@@ -267,23 +257,20 @@ impl<T> MaybeFatalIo<T> for std::io::Result<T> {
 /// where "support" means that we measure wall clock time.
 macro_rules! observe_duration {
     ($op:expr, $($body:tt)*) => {{
-        let start = Instant::now();
+        let instant = Instant::now();
         let result = $($body)*;
-        let end = Instant::now();
-        let elapsed = end.duration_since(start).as_secs_f64();
+        let elapsed = instant.elapsed().as_secs_f64();
         STORAGE_IO_TIME_METRIC
             .get($op)
             .observe(elapsed);
-        (result, end)
+        result
     }}
 }
 
 macro_rules! with_file {
     ($this:expr, $op:expr, | $ident:ident | $($body:tt)*) => {{
         let $ident = $this.lock_file().await?;
-        let (res, _) = observe_duration!($op, $($body)*);
-        $ident.
-        res
+        observe_duration!($op, $($body)*)
     }};
 }
 
@@ -325,16 +312,7 @@ impl VirtualFile {
         }
         let (handle, mut slot_guard) = get_open_files().find_victim_slot().await;
 
-        let (res, done_ts) = observe_duration!(StorageIoOperation::Open, open_options.open(path));
-        let file = res?;
-
-        // TODO: Under pressure, it's likely the slot will get re-used and
-        // the underlying file closed before they get around to using it.
-        // => https://github.com/neondatabase/neon/issues/6065
-        slot_guard.file.replace(FileInSlot {
-            file,
-            inserted_at: done_ts,
-        });
+        let file = observe_duration!(StorageIoOperation::Open, open_options.open(path))?;
 
         // Strip all options other than read and write.
         //
@@ -355,6 +333,8 @@ impl VirtualFile {
             timeline_id,
         };
 
+        slot_guard.file.replace(file);
+
         Ok(vfile)
     }
 
@@ -462,16 +442,11 @@ impl VirtualFile {
         let (handle, mut slot_guard) = open_files.find_victim_slot().await;
 
         // Open the physical file
-        let (res, done_ts) =
-            observe_duration!(StorageIoOperation::Open, self.open_options.open(&self.path));
-        let file = res?;
+        let file = observe_duration!(StorageIoOperation::Open, self.open_options.open(&self.path))?;
 
         // Store the File in the slot and update the handle in the VirtualFile
         // to point to it.
-        slot_guard.file.replace(FileInSlot {
-            file,
-            inserted_at: done_ts,
-        });
+        slot_guard.file.replace(file);
 
         *handle_guard = handle;
 
@@ -614,7 +589,7 @@ impl<'a> AsRef<File> for FileGuard<'a> {
     fn as_ref(&self) -> &File {
         // This unwrap is safe because we only create `FileGuard`s
         // if we know that the file is Some.
-        &self.slot_guard.file.as_ref().unwrap().file
+        self.slot_guard.file.as_ref().unwrap()
     }
 }
 
@@ -657,7 +632,7 @@ impl Drop for VirtualFile {
                 slot.recently_used.store(false, Ordering::Relaxed);
                 // there is also the `CloseByReplace` operation for closes done on eviction for
                 // comparison.
-                if let Some(FileInSlot { file, inserted_at }) = slot_guard.file.take() {
+                if let Some(fd) = slot_guard.file.take() {
                     STORAGE_IO_TIME_METRIC
                         .get(StorageIoOperation::Close)
                         .observe_closure_duration(|| drop(fd));