From 9090fc8490af5e850f55af1a81069da0ba3ba612 Mon Sep 17 00:00:00 2001 From: John Spray Date: Fri, 15 Nov 2024 16:53:15 +0000 Subject: [PATCH] pageserver: respect no_sync in VirtualFile --- pageserver/benches/bench_ingest.rs | 1 + pageserver/ctl/src/layer_map_analyzer.rs | 1 + pageserver/ctl/src/layers.rs | 3 +++ pageserver/ctl/src/main.rs | 1 + pageserver/src/bin/pageserver.rs | 5 ++++ pageserver/src/virtual_file.rs | 33 +++++++++++++++++++++++- 6 files changed, 43 insertions(+), 1 deletion(-) diff --git a/pageserver/benches/bench_ingest.rs b/pageserver/benches/bench_ingest.rs index f6b2a8e0318d..caacd365b306 100644 --- a/pageserver/benches/bench_ingest.rs +++ b/pageserver/benches/bench_ingest.rs @@ -167,6 +167,7 @@ fn criterion_benchmark(c: &mut Criterion) { 16384, virtual_file::io_engine_for_bench(), conf.virtual_file_io_mode, + virtual_file::SyncMode::Sync, ); page_cache::init(conf.page_cache_size); diff --git a/pageserver/ctl/src/layer_map_analyzer.rs b/pageserver/ctl/src/layer_map_analyzer.rs index 11b8e98f57d0..2c350d6d8642 100644 --- a/pageserver/ctl/src/layer_map_analyzer.rs +++ b/pageserver/ctl/src/layer_map_analyzer.rs @@ -138,6 +138,7 @@ pub(crate) async fn main(cmd: &AnalyzeLayerMapCmd) -> Result<()> { 10, virtual_file::api::IoEngineKind::StdFs, IoMode::preferred(), + virtual_file::SyncMode::Sync, ); pageserver::page_cache::init(100); diff --git a/pageserver/ctl/src/layers.rs b/pageserver/ctl/src/layers.rs index 6f543dcaa9ff..4c2c3ab30e7f 100644 --- a/pageserver/ctl/src/layers.rs +++ b/pageserver/ctl/src/layers.rs @@ -51,6 +51,7 @@ async fn read_delta_file(path: impl AsRef, ctx: &RequestContext) -> Result 10, virtual_file::api::IoEngineKind::StdFs, IoMode::preferred(), + virtual_file::SyncMode::Sync, ); page_cache::init(100); let path = Utf8Path::from_path(path.as_ref()).expect("non-Unicode path"); @@ -65,6 +66,7 @@ async fn read_image_file(path: impl AsRef, ctx: &RequestContext) -> Result 10, virtual_file::api::IoEngineKind::StdFs, IoMode::preferred(), + virtual_file::SyncMode::Sync, ); page_cache::init(100); let path = Utf8Path::from_path(path.as_ref()).expect("non-Unicode path"); @@ -171,6 +173,7 @@ pub(crate) async fn main(cmd: &LayerCmd) -> Result<()> { 10, virtual_file::api::IoEngineKind::StdFs, IoMode::preferred(), + virtual_file::SyncMode::Sync, ); pageserver::page_cache::init(100); diff --git a/pageserver/ctl/src/main.rs b/pageserver/ctl/src/main.rs index f506caec5b06..92e766d2fbbe 100644 --- a/pageserver/ctl/src/main.rs +++ b/pageserver/ctl/src/main.rs @@ -209,6 +209,7 @@ async fn print_layerfile(path: &Utf8Path) -> anyhow::Result<()> { 10, virtual_file::api::IoEngineKind::StdFs, IoMode::preferred(), + virtual_file::SyncMode::Sync, ); page_cache::init(100); let ctx = RequestContext::new(TaskKind::DebugTool, DownloadBehavior::Error); diff --git a/pageserver/src/bin/pageserver.rs b/pageserver/src/bin/pageserver.rs index 1e937fc85d9a..033a9a4619e3 100644 --- a/pageserver/src/bin/pageserver.rs +++ b/pageserver/src/bin/pageserver.rs @@ -176,6 +176,11 @@ fn main() -> anyhow::Result<()> { conf.max_file_descriptors, conf.virtual_file_io_engine, conf.virtual_file_io_mode, + if conf.no_sync { + virtual_file::SyncMode::UnsafeNoSync + } else { + virtual_file::SyncMode::Sync + }, ); tracing::info!("Initializing page_cache..."); page_cache::init(conf.page_cache_size); diff --git a/pageserver/src/virtual_file.rs b/pageserver/src/virtual_file.rs index daa8b99ab0f1..b9f8c7ea2024 100644 --- a/pageserver/src/virtual_file.rs +++ b/pageserver/src/virtual_file.rs @@ -175,10 +175,16 @@ impl VirtualFile { } pub async fn sync_all(&self) -> Result<(), Error> { + if SYNC_MODE.load(std::sync::atomic::Ordering::Relaxed) == SyncMode::UnsafeNoSync as u8 { + return Ok(()); + } self.inner.sync_all().await } pub async fn sync_data(&self) -> Result<(), Error> { + if SYNC_MODE.load(std::sync::atomic::Ordering::Relaxed) == SyncMode::UnsafeNoSync as u8 { + return Ok(()); + } self.inner.sync_data().await } @@ -233,6 +239,27 @@ impl VirtualFile { } } +/// Indicates whether to enable fsync, fdatasync, or O_SYNC/O_DSYNC when writing +/// files. Switching this off is unsafe and only used for testing on machines +/// with slow drives. +#[repr(u8)] +pub enum SyncMode { + Sync, + UnsafeNoSync, +} + +impl TryFrom for SyncMode { + type Error = u8; + + fn try_from(value: u8) -> Result { + Ok(match value { + v if v == (SyncMode::Sync as u8) => SyncMode::Sync, + v if v == (SyncMode::UnsafeNoSync as u8) => SyncMode::UnsafeNoSync, + x => return Err(x), + }) + } +} + /// /// 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, @@ -1332,12 +1359,13 @@ impl OpenFiles { /// server startup. /// #[cfg(not(test))] -pub fn init(num_slots: usize, engine: IoEngineKind, mode: IoMode) { +pub fn init(num_slots: usize, engine: IoEngineKind, mode: IoMode, sync_mode: SyncMode) { if OPEN_FILES.set(OpenFiles::new(num_slots)).is_err() { panic!("virtual_file::init called twice"); } set_io_mode(mode); io_engine::init(engine); + SYNC_MODE.store(sync_mode as u8, std::sync::atomic::Ordering::Relaxed); crate::metrics::virtual_file_descriptor_cache::SIZE_MAX.set(num_slots as u64); } @@ -1379,6 +1407,9 @@ pub(crate) fn set_io_mode(mode: IoMode) { pub(crate) fn get_io_mode() -> IoMode { IoMode::try_from(IO_MODE.load(Ordering::Relaxed)).unwrap() } + +static SYNC_MODE: AtomicU8 = AtomicU8::new(SyncMode::Sync as u8); + #[cfg(test)] mod tests { use crate::context::DownloadBehavior;