diff --git a/pageserver/src/tenant/storage_layer/image_layer.rs b/pageserver/src/tenant/storage_layer/image_layer.rs index 875e223c9c..5de2582ab7 100644 --- a/pageserver/src/tenant/storage_layer/image_layer.rs +++ b/pageserver/src/tenant/storage_layer/image_layer.rs @@ -38,7 +38,7 @@ use crate::tenant::timeline::GetVectoredError; use crate::tenant::vectored_blob_io::{ BlobFlag, StreamingVectoredReadPlanner, VectoredBlobReader, VectoredRead, VectoredReadPlanner, }; -use crate::tenant::{PageReconstructError, Timeline}; +use crate::tenant::PageReconstructError; use crate::virtual_file::owned_buffers_io::io_buf_ext::IoBufExt; use crate::virtual_file::{self, VirtualFile}; use crate::{IMAGE_FILE_MAGIC, STORAGE_FORMAT_VERSION, TEMP_FILE_SUFFIX}; @@ -58,7 +58,6 @@ use std::io::SeekFrom; use std::ops::Range; use std::os::unix::prelude::FileExt; use std::str::FromStr; -use std::sync::Arc; use tokio::sync::OnceCell; use tokio_stream::StreamExt; use tracing::*; @@ -70,9 +69,7 @@ use utils::{ }; use super::layer_name::ImageLayerName; -use super::{ - AsLayerDesc, Layer, LayerName, PersistentLayerDesc, ResidentLayer, ValuesReconstructState, -}; +use super::{AsLayerDesc, LayerName, PersistentLayerDesc, ValuesReconstructState}; /// /// Header stored in the beginning of the file @@ -800,10 +797,9 @@ impl ImageLayerWriterInner { /// async fn finish( self, - timeline: &Arc, ctx: &RequestContext, end_key: Option, - ) -> anyhow::Result { + ) -> anyhow::Result<(PersistentLayerDesc, Utf8PathBuf)> { let index_start_blk = ((self.blob_writer.size() + PAGE_SZ as u64 - 1) / PAGE_SZ as u64) as u32; @@ -879,12 +875,9 @@ impl ImageLayerWriterInner { // fsync the file file.sync_all().await?; - // FIXME: why not carry the virtualfile here, it supports renaming? - let layer = Layer::finish_creating(self.conf, timeline, desc, &self.path)?; + trace!("created image layer {}", self.path); - info!("created image layer {}", layer.local_path()); - - Ok(layer) + Ok((desc, self.path)) } } @@ -963,24 +956,18 @@ impl ImageLayerWriter { /// pub(crate) async fn finish( mut self, - timeline: &Arc, ctx: &RequestContext, - ) -> anyhow::Result { - self.inner.take().unwrap().finish(timeline, ctx, None).await + ) -> anyhow::Result<(PersistentLayerDesc, Utf8PathBuf)> { + self.inner.take().unwrap().finish(ctx, None).await } /// Finish writing the image layer with an end key, used in [`super::split_writer::SplitImageLayerWriter`]. The end key determines the end of the image layer's covered range and is exclusive. pub(super) async fn finish_with_end_key( mut self, - timeline: &Arc, end_key: Key, ctx: &RequestContext, - ) -> anyhow::Result { - self.inner - .take() - .unwrap() - .finish(timeline, ctx, Some(end_key)) - .await + ) -> anyhow::Result<(PersistentLayerDesc, Utf8PathBuf)> { + self.inner.take().unwrap().finish(ctx, Some(end_key)).await } } @@ -1084,7 +1071,7 @@ mod test { tenant::{ config::TenantConf, harness::{TenantHarness, TIMELINE_ID}, - storage_layer::ResidentLayer, + storage_layer::{Layer, ResidentLayer}, vectored_blob_io::StreamingVectoredReadPlanner, Tenant, Timeline, }, @@ -1155,7 +1142,8 @@ mod test { key = key.next(); } - writer.finish(&timeline, &ctx).await.unwrap() + let (desc, path) = writer.finish(&ctx).await.unwrap(); + Layer::finish_creating(tenant.conf, &timeline, desc, &path).unwrap() }; let original_size = resident.metadata().file_size; @@ -1217,7 +1205,9 @@ mod test { .await .unwrap(); let replacement = if wrote_keys > 0 { - Some(filtered_writer.finish(&timeline, &ctx).await.unwrap()) + let (desc, path) = filtered_writer.finish(&ctx).await.unwrap(); + let resident = Layer::finish_creating(tenant.conf, &timeline, desc, &path).unwrap(); + Some(resident) } else { None }; @@ -1290,7 +1280,8 @@ mod test { for (key, img) in images { writer.put_image(key, img, ctx).await?; } - let img_layer = writer.finish(tline, ctx).await?; + let (desc, path) = writer.finish(ctx).await?; + let img_layer = Layer::finish_creating(tenant.conf, tline, desc, &path)?; Ok::<_, anyhow::Error>(img_layer) } diff --git a/pageserver/src/tenant/storage_layer/split_writer.rs b/pageserver/src/tenant/storage_layer/split_writer.rs index 40a6a77a50..b499a0eef4 100644 --- a/pageserver/src/tenant/storage_layer/split_writer.rs +++ b/pageserver/src/tenant/storage_layer/split_writer.rs @@ -121,11 +121,11 @@ impl SplitImageLayerWriter { self.generated_layers .push(SplitWriterResult::Discarded(layer_key)); } else { - self.generated_layers.push(SplitWriterResult::Produced( - prev_image_writer - .finish_with_end_key(tline, key, ctx) - .await?, - )); + let (desc, path) = prev_image_writer.finish_with_end_key(key, ctx).await?; + + let layer = Layer::finish_creating(self.conf, tline, desc, &path)?; + self.generated_layers + .push(SplitWriterResult::Produced(layer)); } } self.inner.put_image(key, img, ctx).await @@ -170,9 +170,9 @@ impl SplitImageLayerWriter { if discard(&layer_key).await { generated_layers.push(SplitWriterResult::Discarded(layer_key)); } else { - generated_layers.push(SplitWriterResult::Produced( - inner.finish_with_end_key(tline, end_key, ctx).await?, - )); + let (desc, path) = inner.finish_with_end_key(end_key, ctx).await?; + let layer = Layer::finish_creating(self.conf, tline, desc, &path)?; + generated_layers.push(SplitWriterResult::Produced(layer)); } Ok(generated_layers) } diff --git a/pageserver/src/tenant/timeline.rs b/pageserver/src/tenant/timeline.rs index 262dccac7d..f66491d962 100644 --- a/pageserver/src/tenant/timeline.rs +++ b/pageserver/src/tenant/timeline.rs @@ -4013,7 +4013,8 @@ impl Timeline { if wrote_keys { // Normal path: we have written some data into the new image layer for this // partition, so flush it to disk. - let image_layer = image_layer_writer.finish(self, ctx).await?; + let (desc, path) = image_layer_writer.finish(ctx).await?; + let image_layer = Layer::finish_creating(self.conf, self, desc, &path)?; Ok(ImageLayerCreationOutcome { image: Some(image_layer), next_start_key: img_range.end, @@ -4101,7 +4102,8 @@ impl Timeline { if wrote_any_image { // Normal path: we have written some data into the new image layer for this // partition, so flush it to disk. - let image_layer = image_layer_writer.finish(self, ctx).await?; + let (desc, path) = image_layer_writer.finish(ctx).await?; + let image_layer = Layer::finish_creating(self.conf, self, desc, &path)?; Ok(ImageLayerCreationOutcome { image: Some(image_layer), next_start_key: img_range.end, @@ -5403,7 +5405,8 @@ impl Timeline { for (key, img) in images { image_layer_writer.put_image(key, img, ctx).await?; } - let image_layer = image_layer_writer.finish(self, ctx).await?; + let (desc, path) = image_layer_writer.finish(ctx).await?; + let image_layer = Layer::finish_creating(self.conf, self, desc, &path)?; { let mut guard = self.layers.write().await; diff --git a/pageserver/src/tenant/timeline/compaction.rs b/pageserver/src/tenant/timeline/compaction.rs index 0b5c520ba7..d1f06e3480 100644 --- a/pageserver/src/tenant/timeline/compaction.rs +++ b/pageserver/src/tenant/timeline/compaction.rs @@ -563,10 +563,12 @@ impl Timeline { .await?; if keys_written > 0 { - let new_layer = image_layer_writer - .finish(self, ctx) + let (desc, path) = image_layer_writer + .finish(ctx) .await .map_err(CompactionError::Other)?; + let new_layer = Layer::finish_creating(self.conf, self, desc, &path) + .map_err(CompactionError::Other)?; tracing::info!(layer=%new_layer, "Rewrote layer, {} -> {} bytes", layer.metadata().file_size, new_layer.metadata().file_size);