From 6146a82771f109c7826bcc9f1ae181e1eb7d5473 Mon Sep 17 00:00:00 2001 From: Christian Schwarz Date: Tue, 29 Apr 2025 13:27:39 +0200 Subject: [PATCH] incorporate findings from research --- pageserver/src/tenant/ephemeral_file.rs | 2 +- .../src/tenant/remote_timeline_client/download.rs | 2 +- pageserver/src/tenant/storage_layer/delta_layer.rs | 2 +- pageserver/src/tenant/storage_layer/image_layer.rs | 2 +- pageserver/src/virtual_file.rs | 10 ++++------ pageserver/src/virtual_file/io_engine.rs | 12 +++++++++--- 6 files changed, 17 insertions(+), 13 deletions(-) diff --git a/pageserver/src/tenant/ephemeral_file.rs b/pageserver/src/tenant/ephemeral_file.rs index 3bbaeb8946..40d06988ad 100644 --- a/pageserver/src/tenant/ephemeral_file.rs +++ b/pageserver/src/tenant/ephemeral_file.rs @@ -88,7 +88,7 @@ impl EphemeralFile { gate.enter()?, ); - file.fallocate_keep_size(0, 1 * 1024 * 1024 * 1024, ctx) + file.fallocate(0, 1 * 1024 * 1024 * 1024, ctx) .await .unwrap(); diff --git a/pageserver/src/tenant/remote_timeline_client/download.rs b/pageserver/src/tenant/remote_timeline_client/download.rs index 0cee73afba..9fcc059b50 100644 --- a/pageserver/src/tenant/remote_timeline_client/download.rs +++ b/pageserver/src/tenant/remote_timeline_client/download.rs @@ -109,7 +109,7 @@ pub async fn download_layer_file<'a>( 64 * 1024 /* TODO this is the max roundtup size by the buffered writer set_len_then_truncate */ )) { - temp_file.fallocate_keep_size( + temp_file.fallocate( 0, file_size, ctx, diff --git a/pageserver/src/tenant/storage_layer/delta_layer.rs b/pageserver/src/tenant/storage_layer/delta_layer.rs index 79bb7b3092..b2e2774446 100644 --- a/pageserver/src/tenant/storage_layer/delta_layer.rs +++ b/pageserver/src/tenant/storage_layer/delta_layer.rs @@ -440,7 +440,7 @@ impl DeltaLayerWriterInner { gate.enter()?, ); - file.fallocate_keep_size(0, 1 * 1024 * 1024 * 1024, ctx) + file.fallocate(0, 1 * 1024 * 1024 * 1024, ctx) .await .unwrap(); diff --git a/pageserver/src/tenant/storage_layer/image_layer.rs b/pageserver/src/tenant/storage_layer/image_layer.rs index 9e8e923414..3456a92cc5 100644 --- a/pageserver/src/tenant/storage_layer/image_layer.rs +++ b/pageserver/src/tenant/storage_layer/image_layer.rs @@ -806,7 +806,7 @@ impl ImageLayerWriterInner { gate.enter()?, ); - file.fallocate_keep_size(0, 1 * 1024 * 1024 * 1024, ctx) + file.fallocate(0, 1 * 1024 * 1024 * 1024, ctx) .await .unwrap(); diff --git a/pageserver/src/virtual_file.rs b/pageserver/src/virtual_file.rs index 7df045310d..e663d13400 100644 --- a/pageserver/src/virtual_file.rs +++ b/pageserver/src/virtual_file.rs @@ -161,13 +161,13 @@ impl VirtualFile { self.inner.set_len(len, ctx).await } - pub async fn fallocate_keep_size( + pub async fn fallocate( &self, offset: i64, size: i64, ctx: &RequestContext, ) -> Result<(), Error> { - self.inner.fallocate_keep_size(offset, size, ctx).await + self.inner.fallocate(offset, size, ctx).await } pub async fn metadata(&self) -> Result { @@ -647,16 +647,14 @@ impl VirtualFileInner { }) } - pub async fn fallocate_keep_size( + pub async fn fallocate( &self, offset: i64, size: i64, _ctx: &RequestContext, ) -> Result<(), Error> { with_file!(self, StorageIoOperation::Fallocate, |file_guard| { - let (_file_guard, res) = io_engine::get() - .fallocate_keep_size(file_guard, offset, size) - .await; + let (_file_guard, res) = io_engine::get().fallocate(file_guard, offset, size).await; res.maybe_fatal_err("fallocate") // TODO haven't thought about this }) } diff --git a/pageserver/src/virtual_file/io_engine.rs b/pageserver/src/virtual_file/io_engine.rs index 7128f4b59a..416240e36c 100644 --- a/pageserver/src/virtual_file/io_engine.rs +++ b/pageserver/src/virtual_file/io_engine.rs @@ -232,7 +232,7 @@ impl IoEngine { } } - pub(super) async fn fallocate_keep_size( + pub(super) async fn fallocate( &self, file_guard: FileGuard, offset: i64, @@ -250,11 +250,17 @@ impl IoEngine { file_guard.with_std_file(|std_file| { fallocate( std_file.as_raw_fd(), - FallocateFlags::FALLOC_FL_KEEP_SIZE, + // NB: if you ever think of using FALLOC_FL_KEEP_SIZE, keep + // in mind that I have found it to be punting to io_uring worker threads + // on Debian Bookworm Linux 6.1.0-32-amd64 and 6.12.25 mainline. + // => https://gist.github.com/problame/ed876bea40b915ba53267b8265e99352 + FallocateFlags::empty(), offset, len, ) - .expect("TODO") + .expect("TODO"); + std_file.sync_all().unwrap(); + () }); (file_guard, Ok(())) }