From fa0b881c4c2a373ceb6d8d307637bc1194e8b911 Mon Sep 17 00:00:00 2001 From: Joonas Koivunen Date: Tue, 22 Aug 2023 01:29:46 +0300 Subject: [PATCH] remove previous generation of Layer, PersistentLayer - remove get_value_reconstruct_data for Delta, Image - remove unnecessary default trait method - remove trait PersistentLayer - remove unused {Delta,Image}Layer::new() - continued dead code removal - unify ImageLayer to be like DeltaLayer - dead code and imports cleanup - remove PathOrConfig - correct few doc links re: Layer removal --- pageserver/src/tenant/storage_layer.rs | 124 ++++-------- .../src/tenant/storage_layer/delta_layer.rs | 179 ++--------------- .../src/tenant/storage_layer/image_layer.rs | 185 ++---------------- 3 files changed, 77 insertions(+), 411 deletions(-) diff --git a/pageserver/src/tenant/storage_layer.rs b/pageserver/src/tenant/storage_layer.rs index 092cdcf58a..f1d707643b 100644 --- a/pageserver/src/tenant/storage_layer.rs +++ b/pageserver/src/tenant/storage_layer.rs @@ -80,7 +80,7 @@ pub struct ValueReconstructState { pub img: Option<(Lsn, Bytes)>, } -/// Return value from Layer::get_page_reconstruct_data +/// Return value from [`LayerE::get_value_reconstruct_data`] #[derive(Clone, Copy, Debug)] pub enum ValueReconstructResult { /// Got all the data needed to reconstruct the requested page @@ -311,7 +311,7 @@ impl LayerAccessStats { /// /// However when we want something evicted, we cannot evict it right away as there might be current /// reads happening on it. It has been for example searched from [`LayerMap`] but not yet -/// [`Layer::get_value_reconstruct_data`]. +/// [`LayerE::get_value_reconstruct_data`]. /// /// [`LayerMap`]: crate::tenant::layer_map::LayerMap enum ResidentOrWantedEvicted { @@ -348,6 +348,21 @@ impl ResidentOrWantedEvicted { } } +/// A Layer contains all data in a "rectangle" consisting of a range of keys and +/// range of LSNs. +/// +/// There are two kinds of layers, in-memory and on-disk layers. In-memory +/// layers are used to ingest incoming WAL, and provide fast access to the +/// recent page versions. On-disk layers are stored as files on disk, and are +/// immutable. This trait presents the common functionality of in-memory and +/// on-disk layers. +/// +/// Furthermore, there are two kinds of on-disk layers: delta and image layers. +/// A delta layer contains all modifications within a range of LSNs and keys. +/// An image layer is a snapshot of all the data in a key-range, at a single +/// LSN. +/// +/// This type models the on-disk layers, which can be evicted and on-demand downloaded. // TODO: // - internal arc, because I've now worked away majority of external wrapping // - load time api which checks that files are present, fixmes in load time, remote timeline @@ -614,6 +629,17 @@ impl LayerE { self.wanted_garbage_collected.store(true, Ordering::Release); } + /// Return data needed to reconstruct given page at LSN. + /// + /// It is up to the caller to collect more data from previous layer and + /// perform WAL redo, if necessary. + /// + /// See PageReconstructResult for possible return values. The collected data + /// is appended to reconstruct_data; the caller should pass an empty struct + /// on first call, or a struct with a cached older image of the page if one + /// is available. If this returns ValueReconstructResult::Continue, look up + /// the predecessor layer and call again with the same 'reconstruct_data' to + /// collect more data. pub(crate) async fn get_value_reconstruct_data( self: &Arc, key: Key, @@ -621,10 +647,21 @@ impl LayerE { reconstruct_data: &mut ValueReconstructState, ctx: &RequestContext, ) -> anyhow::Result { + use anyhow::ensure; + let layer = self.get_or_maybe_download(true, Some(ctx)).await?; self.access_stats .record_access(LayerAccessKind::GetValueReconstructData, ctx); + if self.layer_desc().is_delta { + ensure!(lsn_range.start >= self.layer_desc().lsn_range.start); + ensure!(self.layer_desc().key_range.contains(&key)); + } else { + ensure!(self.layer_desc().key_range.contains(&key)); + ensure!(lsn_range.start >= self.layer_desc().image_layer_lsn()); + ensure!(lsn_range.end >= self.layer_desc().image_layer_lsn()); + } + layer .get_value_reconstruct_data(key, lsn_range, reconstruct_data) .await @@ -1263,7 +1300,6 @@ impl DownloadedLayer { ) -> anyhow::Result { use LayerKind::*; - // FIXME: some asserts are left behind in DeltaLayer, ImageLayer match self.get().await? { Delta(d) => { d.get_value_reconstruct_data(key, lsn_range, reconstruct_data) @@ -1317,81 +1353,12 @@ enum LayerKind { Image(image_layer::ImageLayerInner), } -/// Supertrait of the [`Layer`] trait that captures the bare minimum interface -/// required by [`LayerMap`](super::layer_map::LayerMap). -/// -/// All layers should implement a minimal `std::fmt::Debug` without tenant or -/// timeline names, because those are known in the context of which the layers -/// are used in (timeline). -#[async_trait::async_trait] -pub trait Layer: std::fmt::Debug + std::fmt::Display + Send + Sync + 'static { - /// - /// Return data needed to reconstruct given page at LSN. - /// - /// It is up to the caller to collect more data from previous layer and - /// perform WAL redo, if necessary. - /// - /// See PageReconstructResult for possible return values. The collected data - /// is appended to reconstruct_data; the caller should pass an empty struct - /// on first call, or a struct with a cached older image of the page if one - /// is available. If this returns ValueReconstructResult::Continue, look up - /// the predecessor layer and call again with the same 'reconstruct_data' to - /// collect more data. - async fn get_value_reconstruct_data( - &self, - key: Key, - lsn_range: Range, - reconstruct_data: &mut ValueReconstructState, - ctx: &RequestContext, - ) -> Result; -} - /// Get a layer descriptor from a layer. pub trait AsLayerDesc { /// Get the layer descriptor. fn layer_desc(&self) -> &PersistentLayerDesc; } -/// A Layer contains all data in a "rectangle" consisting of a range of keys and -/// range of LSNs. -/// -/// There are two kinds of layers, in-memory and on-disk layers. In-memory -/// layers are used to ingest incoming WAL, and provide fast access to the -/// recent page versions. On-disk layers are stored as files on disk, and are -/// immutable. This trait presents the common functionality of in-memory and -/// on-disk layers. -/// -/// Furthermore, there are two kinds of on-disk layers: delta and image layers. -/// A delta layer contains all modifications within a range of LSNs and keys. -/// An image layer is a snapshot of all the data in a key-range, at a single -/// LSN. -pub trait PersistentLayer: Layer + AsLayerDesc { - /// File name used for this layer, both in the pageserver's local filesystem - /// state as well as in the remote storage. - fn filename(&self) -> LayerFileName { - self.layer_desc().filename() - } - - // Path to the layer file in the local filesystem. - // `None` for `RemoteLayer`. - fn local_path(&self) -> Option; - - /// Permanently remove this layer from disk. - fn delete_resident_layer_file(&self) -> Result<()>; - - fn downcast_delta_layer(self: Arc) -> Option> { - None - } - - fn is_remote_layer(&self) -> bool { - false - } - - fn info(&self, reset: LayerAccessStatsReset) -> HistoricLayerInfo; - - fn access_stats(&self) -> &LayerAccessStats; -} - pub mod tests { use super::*; @@ -1429,19 +1396,6 @@ pub mod tests { } } -/// Helper enum to hold a PageServerConf, or a path -/// -/// This is used by DeltaLayer and ImageLayer. Normally, this holds a reference to the -/// global config, and paths to layer files are constructed using the tenant/timeline -/// path from the config. But in the 'pagectl' binary, we need to construct a Layer -/// struct for a file on disk, without having a page server running, so that we have no -/// config. In that case, we use the Path variant to hold the full path to the file on -/// disk. -enum PathOrConf { - Path(PathBuf), - Conf(&'static PageServerConf), -} - /// Range wrapping newtype, which uses display to render Debug. /// /// Useful with `Key`, which has too verbose `{:?}` for printing multiple layers. diff --git a/pageserver/src/tenant/storage_layer/delta_layer.rs b/pageserver/src/tenant/storage_layer/delta_layer.rs index b358b056e5..f35b8de0f8 100644 --- a/pageserver/src/tenant/storage_layer/delta_layer.rs +++ b/pageserver/src/tenant/storage_layer/delta_layer.rs @@ -34,18 +34,16 @@ use crate::repository::{Key, Value, KEY_SIZE}; use crate::tenant::blob_io::{BlobWriter, WriteBlobWriter}; use crate::tenant::block_io::{BlockBuf, BlockCursor, BlockLease, BlockReader, FileBlockReader}; use crate::tenant::disk_btree::{DiskBtreeBuilder, DiskBtreeReader, VisitDirection}; -use crate::tenant::storage_layer::{ - LayerE, PersistentLayer, ValueReconstructResult, ValueReconstructState, -}; +use crate::tenant::storage_layer::{LayerE, ValueReconstructResult, ValueReconstructState}; use crate::tenant::Timeline; use crate::virtual_file::VirtualFile; use crate::{walrecord, TEMP_FILE_SUFFIX}; use crate::{DELTA_FILE_MAGIC, STORAGE_FORMAT_VERSION}; use anyhow::{bail, ensure, Context, Result}; -use pageserver_api::models::{HistoricLayerInfo, LayerAccessKind}; +use pageserver_api::models::LayerAccessKind; use rand::{distributions::Alphanumeric, Rng}; use serde::{Deserialize, Serialize}; -use std::fs::{self, File}; +use std::fs::File; use std::io::{BufWriter, Write}; use std::io::{Seek, SeekFrom}; use std::ops::Range; @@ -61,10 +59,7 @@ use utils::{ lsn::Lsn, }; -use super::{ - AsLayerDesc, DeltaFileName, Layer, LayerAccessStats, LayerAccessStatsReset, PathOrConf, - PersistentLayerDesc, ResidentLayer, -}; +use super::{AsLayerDesc, LayerAccessStats, PersistentLayerDesc, ResidentLayer}; /// /// Header stored in the beginning of the file @@ -192,7 +187,7 @@ impl DeltaKey { /// Otherwise the struct is just a placeholder for a file that exists on disk, /// and it needs to be loaded before using it in queries. pub struct DeltaLayer { - path_or_conf: PathOrConf, + path: PathBuf, pub desc: PersistentLayerDesc, @@ -238,19 +233,6 @@ impl std::fmt::Debug for DeltaLayerInner { } } -#[async_trait::async_trait] -impl Layer for DeltaLayer { - async fn get_value_reconstruct_data( - &self, - key: Key, - lsn_range: Range, - reconstruct_state: &mut ValueReconstructState, - ctx: &RequestContext, - ) -> anyhow::Result { - self.get_value_reconstruct_data(key, lsn_range, reconstruct_state, ctx) - .await - } -} /// Boilerplate to implement the Layer trait, always use layer_desc for persistent layers. impl std::fmt::Display for DeltaLayer { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { @@ -264,28 +246,6 @@ impl AsLayerDesc for DeltaLayer { } } -impl PersistentLayer for DeltaLayer { - fn downcast_delta_layer(self: Arc) -> Option> { - Some(self) - } - - fn local_path(&self) -> Option { - self.local_path() - } - - fn delete_resident_layer_file(&self) -> Result<()> { - self.delete_resident_layer_file() - } - - fn info(&self, reset: LayerAccessStatsReset) -> HistoricLayerInfo { - self.info(reset) - } - - fn access_stats(&self) -> &LayerAccessStats { - self.access_stats() - } -} - impl DeltaLayer { pub(crate) async fn dump(&self, verbose: bool, ctx: &RequestContext) -> Result<()> { println!( @@ -357,69 +317,6 @@ impl DeltaLayer { Ok(()) } - pub(crate) async fn get_value_reconstruct_data( - &self, - key: Key, - lsn_range: Range, - reconstruct_state: &mut ValueReconstructState, - ctx: &RequestContext, - ) -> anyhow::Result { - ensure!(lsn_range.start >= self.desc.lsn_range.start); - - ensure!(self.desc.key_range.contains(&key)); - - let inner = self - .load(LayerAccessKind::GetValueReconstructData, ctx) - .await?; - inner - .get_value_reconstruct_data(key, lsn_range, reconstruct_state) - .await - } - - pub(crate) fn local_path(&self) -> Option { - Some(self.path()) - } - - pub(crate) fn delete_resident_layer_file(&self) -> Result<()> { - // delete underlying file - fs::remove_file(self.path())?; - Ok(()) - } - - pub(crate) fn info(&self, reset: LayerAccessStatsReset) -> HistoricLayerInfo { - let layer_file_name = self.layer_desc().filename().file_name(); - let lsn_range = self.layer_desc().lsn_range.clone(); - - let access_stats = self.access_stats.as_api_model(reset); - - HistoricLayerInfo::Delta { - layer_file_name, - layer_file_size: self.desc.file_size, - lsn_start: lsn_range.start, - lsn_end: lsn_range.end, - remote: false, - access_stats, - } - } - - pub(crate) fn access_stats(&self) -> &LayerAccessStats { - &self.access_stats - } - - fn path_for( - path_or_conf: &PathOrConf, - tenant_id: &TenantId, - timeline_id: &TimelineId, - fname: &DeltaFileName, - ) -> PathBuf { - match path_or_conf { - PathOrConf::Path(path) => path.clone(), - PathOrConf::Conf(conf) => conf - .timeline_path(tenant_id, timeline_id) - .join(fname.to_string()), - } - } - fn temp_path_for( conf: &PageServerConf, tenant_id: &TenantId, @@ -463,52 +360,22 @@ impl DeltaLayer { async fn load_inner(&self) -> Result> { let path = self.path(); - let summary = match &self.path_or_conf { - PathOrConf::Conf(_) => Some(Summary::from(self)), - PathOrConf::Path(_) => None, - }; + let loaded = DeltaLayerInner::load(&path, None)?; - let loaded = DeltaLayerInner::load(&path, summary)?; + // not production code - if let PathOrConf::Path(ref path) = self.path_or_conf { - // not production code + let actual_filename = self.path.file_name().unwrap().to_str().unwrap().to_owned(); + let expected_filename = self.layer_desc().filename().file_name(); - let actual_filename = path.file_name().unwrap().to_str().unwrap().to_owned(); - let expected_filename = self.filename().file_name(); - - if actual_filename != expected_filename { - println!("warning: filename does not match what is expected from in-file summary"); - println!("actual: {:?}", actual_filename); - println!("expected: {:?}", expected_filename); - } + if actual_filename != expected_filename { + println!("warning: filename does not match what is expected from in-file summary"); + println!("actual: {:?}", actual_filename); + println!("expected: {:?}", expected_filename); } Ok(Arc::new(loaded)) } - /// Create a DeltaLayer struct representing an existing file on disk. - pub fn new( - conf: &'static PageServerConf, - timeline_id: TimelineId, - tenant_id: TenantId, - filename: &DeltaFileName, - file_size: u64, - access_stats: LayerAccessStats, - ) -> DeltaLayer { - DeltaLayer { - path_or_conf: PathOrConf::Conf(conf), - desc: PersistentLayerDesc::new_delta( - tenant_id, - timeline_id, - filename.key_range.clone(), - filename.lsn_range.clone(), - file_size, - ), - access_stats, - inner: OnceCell::new(), - } - } - /// Create a DeltaLayer struct representing an existing file on disk. /// /// This variant is only used for debugging purposes, by the 'pagectl' binary. @@ -523,7 +390,7 @@ impl DeltaLayer { .context("get file metadata to determine size")?; Ok(DeltaLayer { - path_or_conf: PathOrConf::Path(path.to_path_buf()), + path: path.to_path_buf(), desc: PersistentLayerDesc::new_delta( summary.tenant_id, summary.timeline_id, @@ -536,17 +403,9 @@ impl DeltaLayer { }) } - fn layer_name(&self) -> DeltaFileName { - self.desc.delta_file_name() - } - /// Path to the layer file in pageserver workdir. - pub fn path(&self) -> PathBuf { - Self::path_for( - &self.path_or_conf, - &self.desc.tenant_id, - &self.desc.timeline_id, - &self.layer_name(), - ) + /// Path to the layer file + fn path(&self) -> PathBuf { + self.path.clone() } /// Loads all keys stored in the layer. Returns key, lsn, value size and value reference. /// @@ -720,10 +579,10 @@ impl DeltaLayerWriterInner { metadata.len(), ); - let layer = LayerE::for_written(self.conf, timeline, desc)?; - // fsync the file file.sync_all()?; + + let layer = LayerE::for_written(self.conf, timeline, desc)?; // Rename the file to its final name // // Note: This overwrites any existing file. There shouldn't be any. diff --git a/pageserver/src/tenant/storage_layer/image_layer.rs b/pageserver/src/tenant/storage_layer/image_layer.rs index 23ec700d4f..2977828092 100644 --- a/pageserver/src/tenant/storage_layer/image_layer.rs +++ b/pageserver/src/tenant/storage_layer/image_layer.rs @@ -31,7 +31,7 @@ use crate::tenant::blob_io::{BlobWriter, WriteBlobWriter}; use crate::tenant::block_io::{BlockBuf, BlockReader, FileBlockReader}; use crate::tenant::disk_btree::{DiskBtreeBuilder, DiskBtreeReader, VisitDirection}; use crate::tenant::storage_layer::{ - LayerAccessStats, PersistentLayer, ValueReconstructResult, ValueReconstructState, + LayerAccessStats, ValueReconstructResult, ValueReconstructState, }; use crate::tenant::Timeline; use crate::virtual_file::VirtualFile; @@ -39,10 +39,10 @@ use crate::{IMAGE_FILE_MAGIC, STORAGE_FORMAT_VERSION, TEMP_FILE_SUFFIX}; use anyhow::{bail, ensure, Context, Result}; use bytes::Bytes; use hex; -use pageserver_api::models::{HistoricLayerInfo, LayerAccessKind}; +use pageserver_api::models::LayerAccessKind; use rand::{distributions::Alphanumeric, Rng}; use serde::{Deserialize, Serialize}; -use std::fs::{self, File}; +use std::fs::File; use std::io::Write; use std::io::{Seek, SeekFrom}; use std::ops::Range; @@ -59,10 +59,7 @@ use utils::{ }; use super::filename::ImageFileName; -use super::{ - AsLayerDesc, Layer, LayerAccessStatsReset, LayerE, PathOrConf, PersistentLayerDesc, - ResidentLayer, -}; +use super::{AsLayerDesc, LayerE, PersistentLayerDesc, ResidentLayer}; /// /// Header stored in the beginning of the file @@ -128,7 +125,7 @@ impl Summary { /// Otherwise the struct is just a placeholder for a file that exists on disk, /// and it needs to be loaded before using it in queries. pub struct ImageLayer { - path_or_conf: PathOrConf, + path: PathBuf, pub desc: PersistentLayerDesc, // This entry contains an image of all pages as of this LSN, should be the same as desc.lsn @@ -172,21 +169,6 @@ impl std::fmt::Debug for ImageLayerInner { } } -#[async_trait::async_trait] -impl Layer for ImageLayer { - /// Look up given page in the file - async fn get_value_reconstruct_data( - &self, - key: Key, - lsn_range: Range, - reconstruct_state: &mut ValueReconstructState, - ctx: &RequestContext, - ) -> anyhow::Result { - self.get_value_reconstruct_data(key, lsn_range, reconstruct_state, ctx) - .await - } -} - /// Boilerplate to implement the Layer trait, always use layer_desc for persistent layers. impl std::fmt::Display for ImageLayer { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { @@ -200,24 +182,6 @@ impl AsLayerDesc for ImageLayer { } } -impl PersistentLayer for ImageLayer { - fn local_path(&self) -> Option { - self.local_path() - } - - fn delete_resident_layer_file(&self) -> Result<()> { - self.delete_resident_layer_file() - } - - fn info(&self, reset: LayerAccessStatsReset) -> HistoricLayerInfo { - self.info(reset) - } - - fn access_stats(&self) -> &LayerAccessStats { - self.access_stats() - } -} - impl ImageLayer { pub(crate) async fn dump(&self, verbose: bool, ctx: &RequestContext) -> Result<()> { println!( @@ -252,68 +216,6 @@ impl ImageLayer { Ok(()) } - pub(crate) async fn get_value_reconstruct_data( - &self, - key: Key, - lsn_range: Range, - reconstruct_state: &mut ValueReconstructState, - ctx: &RequestContext, - ) -> anyhow::Result { - assert!(self.desc.key_range.contains(&key)); - assert!(lsn_range.start >= self.lsn); - assert!(lsn_range.end >= self.lsn); - - let inner = self - .load(LayerAccessKind::GetValueReconstructData, ctx) - .await?; - inner - .get_value_reconstruct_data(key, reconstruct_state) - .await - // FIXME: makes no sense to dump paths - .with_context(|| format!("read {}", self.path().display())) - } - - pub(crate) fn local_path(&self) -> Option { - Some(self.path()) - } - - pub(crate) fn delete_resident_layer_file(&self) -> Result<()> { - // delete underlying file - fs::remove_file(self.path())?; - Ok(()) - } - - pub(crate) fn info(&self, reset: LayerAccessStatsReset) -> HistoricLayerInfo { - let layer_file_name = self.layer_desc().filename().file_name(); - let lsn_start = self.layer_desc().image_layer_lsn(); - - HistoricLayerInfo::Image { - layer_file_name, - layer_file_size: self.desc.file_size, - lsn_start, - remote: false, - access_stats: self.access_stats.as_api_model(reset), - } - } - - pub(crate) fn access_stats(&self) -> &LayerAccessStats { - &self.access_stats - } - - fn path_for( - path_or_conf: &PathOrConf, - timeline_id: TimelineId, - tenant_id: TenantId, - fname: &ImageFileName, - ) -> PathBuf { - match path_or_conf { - PathOrConf::Path(path) => path.to_path_buf(), - PathOrConf::Conf(conf) => conf - .timeline_path(&tenant_id, &timeline_id) - .join(fname.to_string()), - } - } - fn temp_path_for( conf: &PageServerConf, timeline_id: TimelineId, @@ -349,52 +251,21 @@ impl ImageLayer { async fn load_inner(&self) -> Result { let path = self.path(); - let expected_summary = match &self.path_or_conf { - PathOrConf::Conf(_) => Some(Summary::from(self)), - PathOrConf::Path(_) => None, - }; + let loaded = ImageLayerInner::load(&path, self.desc.image_layer_lsn(), None)?; - let loaded = ImageLayerInner::load(&path, self.desc.image_layer_lsn(), expected_summary)?; + // not production code + let actual_filename = self.path.file_name().unwrap().to_str().unwrap().to_owned(); + let expected_filename = self.layer_desc().filename().file_name(); - if let PathOrConf::Path(ref path) = self.path_or_conf { - // not production code - let actual_filename = path.file_name().unwrap().to_str().unwrap().to_owned(); - let expected_filename = self.filename().file_name(); - - if actual_filename != expected_filename { - println!("warning: filename does not match what is expected from in-file summary"); - println!("actual: {:?}", actual_filename); - println!("expected: {:?}", expected_filename); - } + if actual_filename != expected_filename { + println!("warning: filename does not match what is expected from in-file summary"); + println!("actual: {:?}", actual_filename); + println!("expected: {:?}", expected_filename); } Ok(loaded) } - /// Create an ImageLayer struct representing an existing file on disk - pub fn new( - conf: &'static PageServerConf, - timeline_id: TimelineId, - tenant_id: TenantId, - filename: &ImageFileName, - file_size: u64, - access_stats: LayerAccessStats, - ) -> ImageLayer { - ImageLayer { - path_or_conf: PathOrConf::Conf(conf), - desc: PersistentLayerDesc::new_img( - tenant_id, - timeline_id, - filename.key_range.clone(), - filename.lsn, - file_size, - ), // Now we assume image layer ALWAYS covers the full range. This may change in the future. - lsn: filename.lsn, - access_stats, - inner: OnceCell::new(), - } - } - /// Create an ImageLayer struct representing an existing file on disk. /// /// This variant is only used for debugging purposes, by the 'pagectl' binary. @@ -407,7 +278,7 @@ impl ImageLayer { .metadata() .context("get file metadata to determine size")?; Ok(ImageLayer { - path_or_conf: PathOrConf::Path(path.to_path_buf()), + path: path.to_path_buf(), desc: PersistentLayerDesc::new_img( summary.tenant_id, summary.timeline_id, @@ -421,18 +292,9 @@ impl ImageLayer { }) } - fn layer_name(&self) -> ImageFileName { - self.desc.image_file_name() - } - /// Path to the layer file in pageserver workdir. - pub fn path(&self) -> PathBuf { - Self::path_for( - &self.path_or_conf, - self.desc.timeline_id, - self.desc.tenant_id, - &self.layer_name(), - ) + fn path(&self) -> PathBuf { + self.path.clone() } } @@ -633,22 +495,13 @@ impl ImageLayerWriterInner { // fsync the file file.sync_all()?; + let layer = LayerE::for_written(self.conf, timeline, desc)?; + // Rename the file to its final name // // Note: This overwrites any existing file. There shouldn't be any. // FIXME: throw an error instead? - let final_path = ImageLayer::path_for( - &PathOrConf::Conf(self.conf), - self.timeline_id, - self.tenant_id, - &ImageFileName { - key_range: self.key_range.clone(), - lsn: self.lsn, - }, - ); - std::fs::rename(self.path, final_path)?; - - let layer = LayerE::for_written(self.conf, timeline, desc)?; + std::fs::rename(self.path, layer.local_path())?; trace!("created image layer {}", layer.local_path().display());