From 55e06b0c72e2568f21732ea89bcc2154c625e8ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20L=C3=B3pez?= Date: Tue, 14 Jan 2025 01:15:17 +0100 Subject: [PATCH 1/2] core/io: make file locks non-blocking so they fail right away --- core/io/io_uring.rs | 81 ++++++++++++++++++++------------------------- core/io/unix.rs | 6 ++-- 2 files changed, 38 insertions(+), 49 deletions(-) diff --git a/core/io/io_uring.rs b/core/io/io_uring.rs index 4f6001cdf..1598debfa 100644 --- a/core/io/io_uring.rs +++ b/core/io/io_uring.rs @@ -1,11 +1,13 @@ use super::{common, Completion, File, OpenFlags, IO}; use crate::{LimboError, Result}; -use libc::{c_short, fcntl, flock, iovec, F_SETLK}; use log::{debug, trace}; -use nix::fcntl::{FcntlArg, OFlag}; +use rustix::fs::{self, FlockOperation, OFlags}; +use rustix::io_uring::iovec; use std::cell::RefCell; use std::collections::HashMap; use std::fmt; +use std::io::ErrorKind; +use std::os::fd::AsFd; use std::os::unix::io::AsRawFd; use std::rc::Rc; use thiserror::Error; @@ -136,12 +138,12 @@ impl IO for UringIO { .open(path)?; // Let's attempt to enable direct I/O. Not all filesystems support it // so ignore any errors. - let fd = file.as_raw_fd(); + let fd = file.as_fd(); if direct { - match nix::fcntl::fcntl(fd, FcntlArg::F_SETFL(OFlag::O_DIRECT)) { - Ok(_) => {}, + match fs::fcntl_setfl(fd, OFlags::DIRECT) { + Ok(_) => {} Err(error) => debug!("Error {error:?} returned when setting O_DIRECT flag to read file. The performance of the system may be affected"), - }; + } } let uring_file = Rc::new(UringFile { io: self.inner.clone(), @@ -199,52 +201,39 @@ pub struct UringFile { impl File for UringFile { fn lock_file(&self, exclusive: bool) -> Result<()> { - let fd = self.file.as_raw_fd(); - let flock = flock { - l_type: if exclusive { - libc::F_WRLCK as c_short - } else { - libc::F_RDLCK as c_short - }, - l_whence: libc::SEEK_SET as c_short, - l_start: 0, - l_len: 0, // Lock entire file - l_pid: 0, - }; - + let fd = self.file.as_fd(); // F_SETLK is a non-blocking lock. The lock will be released when the file is closed // or the process exits or after an explicit unlock. - let lock_result = unsafe { fcntl(fd, F_SETLK, &flock) }; - if lock_result == -1 { - let err = std::io::Error::last_os_error(); - if err.kind() == std::io::ErrorKind::WouldBlock { - return Err(LimboError::LockingError( - "File is locked by another process".into(), - )); + fs::fcntl_lock( + fd, + if exclusive { + FlockOperation::NonBlockingLockExclusive } else { - return Err(LimboError::IOError(err)); - } - } + FlockOperation::NonBlockingLockShared + }, + ) + .map_err(|e| { + let io_error = std::io::Error::from(e); + let message = match io_error.kind() { + ErrorKind::WouldBlock => { + "Failed locking file. File is locked by another process".to_string() + } + _ => format!("Failed locking file, {}", io_error), + }; + LimboError::LockingError(message) + })?; + Ok(()) } fn unlock_file(&self) -> Result<()> { - let fd = self.file.as_raw_fd(); - let flock = flock { - l_type: libc::F_UNLCK as c_short, - l_whence: libc::SEEK_SET as c_short, - l_start: 0, - l_len: 0, - l_pid: 0, - }; - - let unlock_result = unsafe { fcntl(fd, F_SETLK, &flock) }; - if unlock_result == -1 { - return Err(LimboError::LockingError(format!( + let fd = self.file.as_fd(); + fs::fcntl_lock(fd, FlockOperation::NonBlockingUnlock).map_err(|e| { + LimboError::LockingError(format!( "Failed to release file lock: {}", - std::io::Error::last_os_error() - ))); - } + std::io::Error::from(e) + )) + })?; Ok(()) } @@ -261,7 +250,7 @@ impl File for UringFile { let len = buf.len(); let buf = buf.as_mut_ptr(); let iovec = io.get_iovec(buf, len); - io_uring::opcode::Readv::new(fd, iovec, 1) + io_uring::opcode::Readv::new(fd, iovec as *const iovec as *const libc::iovec, 1) .offset(pos as u64) .build() .user_data(io.ring.get_key()) @@ -282,7 +271,7 @@ impl File for UringFile { let buf = buffer.borrow(); trace!("pwrite(pos = {}, length = {})", pos, buf.len()); let iovec = io.get_iovec(buf.as_ptr(), buf.len()); - io_uring::opcode::Writev::new(fd, iovec, 1) + io_uring::opcode::Writev::new(fd, iovec as *const iovec as *const libc::iovec, 1) .offset(pos as u64) .build() .user_data(io.ring.get_key()) diff --git a/core/io/unix.rs b/core/io/unix.rs index c021c4566..effd94bf5 100644 --- a/core/io/unix.rs +++ b/core/io/unix.rs @@ -142,9 +142,9 @@ impl File for UnixFile { fs::fcntl_lock( fd, if exclusive { - FlockOperation::LockExclusive + FlockOperation::NonBlockingLockExclusive } else { - FlockOperation::LockShared + FlockOperation::NonBlockingLockShared }, ) .map_err(|e| { @@ -164,7 +164,7 @@ impl File for UnixFile { fn unlock_file(&self) -> Result<()> { let fd = self.file.borrow(); let fd = fd.as_fd(); - fs::fcntl_lock(fd, FlockOperation::Unlock).map_err(|e| { + fs::fcntl_lock(fd, FlockOperation::NonBlockingUnlock).map_err(|e| { LimboError::LockingError(format!( "Failed to release file lock: {}", std::io::Error::from(e) From a16282ea62c6968924648506afee6d3c330f8e06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20L=C3=B3pez?= Date: Tue, 14 Jan 2025 11:06:13 +0100 Subject: [PATCH 2/2] core: remove nix as a dependency --- Cargo.lock | 1 - core/Cargo.toml | 1 - 2 files changed, 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5312501a3..bb69e5c0f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1217,7 +1217,6 @@ dependencies = [ "miette", "mimalloc", "mockall", - "nix 0.29.0", "pest", "pest_derive", "polling", diff --git a/core/Cargo.toml b/core/Cargo.toml index c0c12a0a3..0daa58c0d 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -40,7 +40,6 @@ fallible-iterator = "0.3.0" hex = "0.4.3" libc = "0.2.155" log = "0.4.20" -nix = { version = "0.29.0", features = ["fs"] } sieve-cache = "0.1.4" sqlite3-parser = { path = "../vendored/sqlite3-parser" } thiserror = "1.0.61"