From f641105d0be33483de554b5e8d5d2d08ed15c31d Mon Sep 17 00:00:00 2001
From: Ivan Pleshkov <pleshkov.ivan@gmail.com>
Date: Fri, 5 Jan 2024 12:58:46 +0000
Subject: [PATCH] drop memmap before file remaning or deletion

---
 src/lib.rs     | 20 ++++++++++++++++----
 src/segment.rs | 43 +++----------------------------------------
 2 files changed, 19 insertions(+), 44 deletions(-)

diff --git a/src/lib.rs b/src/lib.rs
index fe79732..fe6d6e2 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -1,6 +1,6 @@
 use crossbeam_channel::{Receiver, Sender};
 use fs4::FileExt;
-use log::{debug, info, trace, warn};
+use log::{debug, error, info, trace, warn};
 use std::cmp::Ordering;
 use std::fmt;
 use std::fs::{self, File};
@@ -491,15 +491,27 @@ impl fmt::Debug for Wal {
     }
 }
 
-fn close_segment(mut segment: OpenSegment, start_index: u64) -> Result<ClosedSegment> {
+fn close_segment(segment: OpenSegment, start_index: u64) -> Result<ClosedSegment> {
+    let old_path = segment.segment.path().to_owned();
     let new_path = segment
         .segment
         .path()
         .with_file_name(format!("closed-{start_index}"));
-    segment.segment.rename(new_path)?;
+
+    debug!("{:?}: renaming file to {:?}", &segment, &new_path);
+
+    // close open segment before renaming
+    std::mem::drop(segment);
+
+    fs::rename(&old_path, &new_path).map_err(|e| {
+        error!("{:?}: failed to rename segment {}", &old_path, e);
+        e
+    })?;
+
+    let segment = Segment::open(&new_path)?;
     Ok(ClosedSegment {
         start_index,
-        segment: segment.segment,
+        segment,
     })
 }
 
diff --git a/src/segment.rs b/src/segment.rs
index 9590aa5..a9c3cd3 100644
--- a/src/segment.rs
+++ b/src/segment.rs
@@ -8,7 +8,6 @@ use std::ops::Deref;
 use std::path::{Path, PathBuf};
 use std::ptr;
 use std::thread;
-#[cfg(target_os = "windows")]
 use std::time::Duration;
 
 use crate::mmap_view_sync::MmapViewSync;
@@ -554,48 +553,10 @@ impl Segment {
         &self.path
     }
 
-    /// Renames the segment file.
-    ///
-    /// The caller is responsible for syncing the directory in order to
-    /// guarantee that the rename is durable in the event of a crash.
-    pub fn rename<P>(&mut self, path: P) -> Result<()>
-    where
-        P: AsRef<Path>,
-    {
-        debug!("{:?}: renaming file to {:?}", self, path.as_ref());
-        fs::rename(&self.path, &path).map_err(|e| {
-            error!("{:?}: failed to rename segment {}", self.path, e);
-            e
-        })?;
-        self.path = path.as_ref().to_path_buf();
-        Ok(())
-    }
-
     /// Deletes the segment file.
     pub fn delete(self) -> Result<()> {
         debug!("{:?}: deleting file", self);
 
-        #[cfg(not(target_os = "windows"))]
-        {
-            self.delete_unix()
-        }
-
-        #[cfg(target_os = "windows")]
-        {
-            self.delete_windows()
-        }
-    }
-
-    #[cfg(not(target_os = "windows"))]
-    fn delete_unix(self) -> Result<()> {
-        fs::remove_file(&self.path).map_err(|e| {
-            error!("{:?}: failed to delete segment {}", self, e);
-            e
-        })
-    }
-
-    #[cfg(target_os = "windows")]
-    fn delete_windows(self) -> Result<()> {
         const DELETE_TRIES: u32 = 3;
 
         let Segment {
@@ -607,10 +568,12 @@ impl Segment {
         } = self;
         let mmap_len = mmap.len();
 
-        // Unmaps the file before `fs::remove_file` else access will be denied
+        // Unmaps the file before `fs::remove_file`
+        // else access will be denied on Windows or Docker containers with Windows host
         mmap.flush()?;
         std::mem::drop(mmap);
 
+        // Do several attempts to delete the file because on Windows it may be denied
         let mut tries = 0;
         loop {
             tries += 1;