Skip to content

Commit

Permalink
drop memmap before file remaning or deletion
Browse files Browse the repository at this point in the history
  • Loading branch information
IvanPleshkov committed Jan 5, 2024
1 parent fad0e7c commit f641105
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 44 deletions.
20 changes: 16 additions & 4 deletions src/lib.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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,
})
}

Expand Down
43 changes: 3 additions & 40 deletions src/segment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand All @@ -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;
Expand Down

0 comments on commit f641105

Please sign in to comment.