From 5b2199f4774c8bd37398f7a49b4b5c47d060eefc Mon Sep 17 00:00:00 2001 From: Christian Schwarz Date: Thu, 11 Jan 2024 18:43:35 +0000 Subject: [PATCH] refactor(virtual_file): add its own OpenOptions wrapper This is prep work for integrating support for runtime-configurable io engines (=> tokio-epoll-uring). --- pageserver/src/config.rs | 1 + pageserver/src/tenant/ephemeral_file.rs | 9 ++- .../src/tenant/storage_layer/delta_layer.rs | 4 +- .../src/tenant/storage_layer/image_layer.rs | 18 +++--- pageserver/src/virtual_file.rs | 18 ++++-- pageserver/src/virtual_file/open_options.rs | 62 +++++++++++++++++++ 6 files changed, 96 insertions(+), 16 deletions(-) create mode 100644 pageserver/src/virtual_file/open_options.rs diff --git a/pageserver/src/config.rs b/pageserver/src/config.rs index 7c03dc1bdd..c1a0b1aeb5 100644 --- a/pageserver/src/config.rs +++ b/pageserver/src/config.rs @@ -36,6 +36,7 @@ use crate::tenant::config::TenantConfOpt; use crate::tenant::{ TENANTS_SEGMENT_NAME, TENANT_DELETED_MARKER_FILE_NAME, TIMELINES_SEGMENT_NAME, }; + use crate::{ IGNORED_TENANT_FILE_NAME, METADATA_FILE_NAME, TENANT_CONFIG_NAME, TENANT_HEATMAP_BASENAME, TENANT_LOCATION_CONFIG_NAME, TIMELINE_DELETE_MARK_SUFFIX, TIMELINE_UNINIT_MARK_SUFFIX, diff --git a/pageserver/src/tenant/ephemeral_file.rs b/pageserver/src/tenant/ephemeral_file.rs index 591eacd104..9f7b0a7b78 100644 --- a/pageserver/src/tenant/ephemeral_file.rs +++ b/pageserver/src/tenant/ephemeral_file.rs @@ -5,11 +5,11 @@ use crate::config::PageServerConf; use crate::context::RequestContext; use crate::page_cache::{self, PAGE_SZ}; use crate::tenant::block_io::{BlockCursor, BlockLease, BlockReader}; -use crate::virtual_file::VirtualFile; +use crate::virtual_file::{self, VirtualFile}; use camino::Utf8PathBuf; use pageserver_api::shard::TenantShardId; use std::cmp::min; -use std::fs::OpenOptions; + use std::io::{self, ErrorKind}; use std::ops::DerefMut; use std::sync::atomic::AtomicU64; @@ -47,7 +47,10 @@ impl EphemeralFile { let file = VirtualFile::open_with_options( &filename, - OpenOptions::new().read(true).write(true).create(true), + virtual_file::OpenOptions::new() + .read(true) + .write(true) + .create(true), ) .await?; diff --git a/pageserver/src/tenant/storage_layer/delta_layer.rs b/pageserver/src/tenant/storage_layer/delta_layer.rs index 4ded6d6a8d..3a445ef71e 100644 --- a/pageserver/src/tenant/storage_layer/delta_layer.rs +++ b/pageserver/src/tenant/storage_layer/delta_layer.rs @@ -36,7 +36,7 @@ use crate::tenant::block_io::{BlockBuf, BlockCursor, BlockLease, BlockReader, Fi use crate::tenant::disk_btree::{DiskBtreeBuilder, DiskBtreeReader, VisitDirection}; use crate::tenant::storage_layer::{Layer, ValueReconstructResult, ValueReconstructState}; use crate::tenant::Timeline; -use crate::virtual_file::VirtualFile; +use crate::virtual_file::{self, VirtualFile}; use crate::{walrecord, TEMP_FILE_SUFFIX}; use crate::{DELTA_FILE_MAGIC, STORAGE_FORMAT_VERSION}; use anyhow::{bail, ensure, Context, Result}; @@ -649,7 +649,7 @@ impl DeltaLayer { { let file = VirtualFile::open_with_options( path, - &*std::fs::OpenOptions::new().read(true).write(true), + virtual_file::OpenOptions::new().read(true).write(true), ) .await .with_context(|| format!("Failed to open file '{}'", path))?; diff --git a/pageserver/src/tenant/storage_layer/image_layer.rs b/pageserver/src/tenant/storage_layer/image_layer.rs index f03c7642eb..c62e6aed51 100644 --- a/pageserver/src/tenant/storage_layer/image_layer.rs +++ b/pageserver/src/tenant/storage_layer/image_layer.rs @@ -34,7 +34,7 @@ use crate::tenant::storage_layer::{ LayerAccessStats, ValueReconstructResult, ValueReconstructState, }; use crate::tenant::Timeline; -use crate::virtual_file::VirtualFile; +use crate::virtual_file::{self, VirtualFile}; use crate::{IMAGE_FILE_MAGIC, STORAGE_FORMAT_VERSION, TEMP_FILE_SUFFIX}; use anyhow::{bail, ensure, Context, Result}; use bytes::Bytes; @@ -327,7 +327,7 @@ impl ImageLayer { { let file = VirtualFile::open_with_options( path, - &*std::fs::OpenOptions::new().read(true).write(true), + virtual_file::OpenOptions::new().read(true).write(true), ) .await .with_context(|| format!("Failed to open file '{}'", path))?; @@ -492,11 +492,15 @@ impl ImageLayerWriterInner { }, ); info!("new image layer {path}"); - let mut file = VirtualFile::open_with_options( - &path, - std::fs::OpenOptions::new().write(true).create_new(true), - ) - .await?; + let mut file = { + VirtualFile::open_with_options( + &path, + virtual_file::OpenOptions::new() + .write(true) + .create_new(true), + ) + .await? + }; // make room for the header block file.seek(SeekFrom::Start(PAGE_SZ as u64)).await?; let blob_writer = BlobWriter::new(file, PAGE_SZ as u64); diff --git a/pageserver/src/virtual_file.rs b/pageserver/src/virtual_file.rs index e549d208b2..5d55810ede 100644 --- a/pageserver/src/virtual_file.rs +++ b/pageserver/src/virtual_file.rs @@ -11,17 +11,22 @@ //! src/backend/storage/file/fd.c //! use crate::metrics::{StorageIoOperation, STORAGE_IO_SIZE, STORAGE_IO_TIME_METRIC}; + use crate::tenant::TENANTS_SEGMENT_NAME; use camino::{Utf8Path, Utf8PathBuf}; use once_cell::sync::OnceCell; -use std::fs::{self, File, OpenOptions}; +use std::fs::{self, File}; use std::io::{Error, ErrorKind, Seek, SeekFrom}; + use std::os::unix::fs::FileExt; use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use tokio::sync::{RwLock, RwLockReadGuard, RwLockWriteGuard}; use tokio::time::Instant; use utils::fs_ext; +mod open_options; +pub(crate) use open_options::*; + /// /// A virtual file descriptor. You can use this just like std::fs::File, but internally /// the underlying file is closed if the system is low on file descriptors, @@ -315,7 +320,10 @@ impl VirtualFile { // NB: there is also StorageIoOperation::OpenAfterReplace which is for the case // where our caller doesn't get to use the returned VirtualFile before its // slot gets re-used by someone else. - let file = observe_duration!(StorageIoOperation::Open, open_options.open(path))?; + let file = observe_duration!( + StorageIoOperation::Open, + open_options.open(path.as_std_path()).await + )?; // Strip all options other than read and write. // @@ -453,7 +461,7 @@ impl VirtualFile { // of the virtual file descriptor cache. let file = observe_duration!( StorageIoOperation::OpenAfterReplace, - self.open_options.open(&self.path) + self.open_options.open(self.path.as_std_path()).await )?; // Store the File in the slot and update the handle in the VirtualFile @@ -815,7 +823,9 @@ mod tests { #[tokio::test] async fn test_physical_files() -> Result<(), Error> { test_files("physical_files", |path, open_options| async move { - Ok(MaybeVirtualFile::File(open_options.open(path)?)) + Ok(MaybeVirtualFile::File( + open_options.open(path.as_std_path()).await?, + )) }) .await } diff --git a/pageserver/src/virtual_file/open_options.rs b/pageserver/src/virtual_file/open_options.rs new file mode 100644 index 0000000000..e2d17445a7 --- /dev/null +++ b/pageserver/src/virtual_file/open_options.rs @@ -0,0 +1,62 @@ +//! Enum-dispatch to the `OpenOptions` type of the respective [`super::IoEngineKind`]; + +use std::path::Path; + +#[derive(Debug, Clone)] +pub struct OpenOptions(std::fs::OpenOptions); + +impl Default for OpenOptions { + fn default() -> Self { + Self(std::fs::OpenOptions::new()) + } +} + +impl OpenOptions { + pub fn new() -> OpenOptions { + Self::default() + } + + pub fn read(&mut self, read: bool) -> &mut OpenOptions { + let _ = self.0.read(read); + self + } + + pub fn write(&mut self, write: bool) -> &mut OpenOptions { + let _ = self.0.write(write); + self + } + + pub fn create(&mut self, create: bool) -> &mut OpenOptions { + let _ = self.0.create(create); + self + } + + pub fn create_new(&mut self, create_new: bool) -> &mut OpenOptions { + let _ = self.0.create_new(create_new); + self + } + + pub fn truncate(&mut self, truncate: bool) -> &mut OpenOptions { + let _ = self.0.truncate(truncate); + self + } + + pub(in crate::virtual_file) async fn open( + &self, + path: &Path, + ) -> std::io::Result { + self.0.open(path) + } +} + +impl std::os::unix::prelude::OpenOptionsExt for OpenOptions { + fn mode(&mut self, mode: u32) -> &mut OpenOptions { + let _ = self.0.mode(mode); + self + } + + fn custom_flags(&mut self, flags: i32) -> &mut OpenOptions { + let _ = self.0.custom_flags(flags); + self + } +}