From 66cdba990a364f7ca3d2a95a6c042bbfbb9ade87 Mon Sep 17 00:00:00 2001 From: Alex Chi Z Date: Thu, 1 Jun 2023 15:06:28 -0400 Subject: [PATCH] refactor: use PersistentLayerDesc for persistent layers (#4398) ## Problem Part of https://github.com/neondatabase/neon/issues/4373 ## Summary of changes This PR adds `PersistentLayerDesc`, which will be used in LayerMap mapping and probably layer cache. After this PR and after we change LayerMap to map to layer desc, we can safely drop RemoteLayerDesc. --------- Signed-off-by: Alex Chi Co-authored-by: bojanserafimov --- pageserver/src/tenant/storage_layer.rs | 17 ++- .../src/tenant/storage_layer/delta_layer.rs | 123 ++++++++-------- .../src/tenant/storage_layer/filename.rs | 4 +- .../src/tenant/storage_layer/image_layer.rs | 122 +++++++++------- .../src/tenant/storage_layer/layer_desc.rs | 109 ++++++++++++++ .../src/tenant/storage_layer/remote_layer.rs | 137 +++++++----------- pageserver/src/tenant/timeline.rs | 5 +- 7 files changed, 310 insertions(+), 207 deletions(-) create mode 100644 pageserver/src/tenant/storage_layer/layer_desc.rs diff --git a/pageserver/src/tenant/storage_layer.rs b/pageserver/src/tenant/storage_layer.rs index 3ca8e28c16..7c071463de 100644 --- a/pageserver/src/tenant/storage_layer.rs +++ b/pageserver/src/tenant/storage_layer.rs @@ -4,6 +4,7 @@ pub mod delta_layer; mod filename; mod image_layer; mod inmemory_layer; +mod layer_desc; mod remote_layer; use crate::config::PageServerConf; @@ -37,6 +38,7 @@ pub use delta_layer::{DeltaLayer, DeltaLayerWriter}; pub use filename::{DeltaFileName, ImageFileName, LayerFileName}; pub use image_layer::{ImageLayer, ImageLayerWriter}; pub use inmemory_layer::InMemoryLayer; +pub use layer_desc::PersistentLayerDesc; pub use remote_layer::RemoteLayer; use super::layer_map::BatchedUpdates; @@ -406,14 +408,23 @@ pub type LayerKeyIter<'i> = Box + 'i>; /// An image layer is a snapshot of all the data in a key-range, at a single /// LSN. pub trait PersistentLayer: Layer { - fn get_tenant_id(&self) -> TenantId; + /// Get the layer descriptor. + fn layer_desc(&self) -> &PersistentLayerDesc; + + fn get_tenant_id(&self) -> TenantId { + self.layer_desc().tenant_id + } /// Identify the timeline this layer belongs to - fn get_timeline_id(&self) -> TimelineId; + fn get_timeline_id(&self) -> TimelineId { + self.layer_desc().timeline_id + } /// 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; + fn filename(&self) -> LayerFileName { + self.layer_desc().filename() + } // Path to the layer file in the local filesystem. // `None` for `RemoteLayer`. diff --git a/pageserver/src/tenant/storage_layer/delta_layer.rs b/pageserver/src/tenant/storage_layer/delta_layer.rs index 63b8e57bb0..5f2fb1ebea 100644 --- a/pageserver/src/tenant/storage_layer/delta_layer.rs +++ b/pageserver/src/tenant/storage_layer/delta_layer.rs @@ -56,8 +56,8 @@ use utils::{ }; use super::{ - DeltaFileName, Layer, LayerAccessStats, LayerAccessStatsReset, LayerFileName, LayerIter, - LayerKeyIter, PathOrConf, + DeltaFileName, Layer, LayerAccessStats, LayerAccessStatsReset, LayerIter, LayerKeyIter, + PathOrConf, PersistentLayerDesc, }; /// @@ -89,10 +89,10 @@ impl From<&DeltaLayer> for Summary { magic: DELTA_FILE_MAGIC, format_version: STORAGE_FORMAT_VERSION, - tenant_id: layer.tenant_id, - timeline_id: layer.timeline_id, - key_range: layer.key_range.clone(), - lsn_range: layer.lsn_range.clone(), + tenant_id: layer.desc.tenant_id, + timeline_id: layer.desc.timeline_id, + key_range: layer.desc.key_range.clone(), + lsn_range: layer.desc.lsn_range.clone(), index_start_blk: 0, index_root_blk: 0, @@ -180,10 +180,7 @@ impl DeltaKey { pub struct DeltaLayer { path_or_conf: PathOrConf, - pub tenant_id: TenantId, - pub timeline_id: TimelineId, - pub key_range: Range, - pub lsn_range: Range, + pub desc: PersistentLayerDesc, pub file_size: u64, @@ -197,8 +194,8 @@ impl std::fmt::Debug for DeltaLayer { use super::RangeDisplayDebug; f.debug_struct("DeltaLayer") - .field("key_range", &RangeDisplayDebug(&self.key_range)) - .field("lsn_range", &self.lsn_range) + .field("key_range", &RangeDisplayDebug(&self.desc.key_range)) + .field("lsn_range", &self.desc.lsn_range) .field("file_size", &self.file_size) .field("inner", &self.inner) .finish() @@ -228,30 +225,16 @@ impl std::fmt::Debug for DeltaLayerInner { } impl Layer for DeltaLayer { - fn get_key_range(&self) -> Range { - self.key_range.clone() - } - - fn get_lsn_range(&self) -> Range { - self.lsn_range.clone() - } - fn is_incremental(&self) -> bool { - true - } - - fn short_id(&self) -> String { - self.filename().file_name() - } /// debugging function to print out the contents of the layer fn dump(&self, verbose: bool, ctx: &RequestContext) -> Result<()> { println!( "----- delta layer for ten {} tli {} keys {}-{} lsn {}-{} ----", - self.tenant_id, - self.timeline_id, - self.key_range.start, - self.key_range.end, - self.lsn_range.start, - self.lsn_range.end + self.desc.tenant_id, + self.desc.timeline_id, + self.desc.key_range.start, + self.desc.key_range.end, + self.desc.lsn_range.start, + self.desc.lsn_range.end ); if !verbose { @@ -324,10 +307,10 @@ impl Layer for DeltaLayer { reconstruct_state: &mut ValueReconstructState, ctx: &RequestContext, ) -> anyhow::Result { - ensure!(lsn_range.start >= self.lsn_range.start); + ensure!(lsn_range.start >= self.desc.lsn_range.start); let mut need_image = true; - ensure!(self.key_range.contains(&key)); + ensure!(self.desc.key_range.contains(&key)); { // Open the file and lock the metadata in memory @@ -402,19 +385,31 @@ impl Layer for DeltaLayer { Ok(ValueReconstructResult::Complete) } } + + /// Boilerplate to implement the Layer trait, always use layer_desc for persistent layers. + fn get_key_range(&self) -> Range { + self.layer_desc().key_range.clone() + } + + /// Boilerplate to implement the Layer trait, always use layer_desc for persistent layers. + fn get_lsn_range(&self) -> Range { + self.layer_desc().lsn_range.clone() + } + + /// Boilerplate to implement the Layer trait, always use layer_desc for persistent layers. + fn is_incremental(&self) -> bool { + self.layer_desc().is_incremental + } + + /// Boilerplate to implement the Layer trait, always use layer_desc for persistent layers. + fn short_id(&self) -> String { + self.layer_desc().short_id() + } } impl PersistentLayer for DeltaLayer { - fn get_tenant_id(&self) -> TenantId { - self.tenant_id - } - - fn get_timeline_id(&self) -> TimelineId { - self.timeline_id - } - - fn filename(&self) -> LayerFileName { - self.layer_name().into() + fn layer_desc(&self) -> &PersistentLayerDesc { + &self.desc } fn local_path(&self) -> Option { @@ -602,10 +597,12 @@ impl DeltaLayer { ) -> DeltaLayer { DeltaLayer { path_or_conf: PathOrConf::Conf(conf), - timeline_id, - tenant_id, - key_range: filename.key_range.clone(), - lsn_range: filename.lsn_range.clone(), + desc: PersistentLayerDesc::new_delta( + tenant_id, + timeline_id, + filename.key_range.clone(), + filename.lsn_range.clone(), + ), file_size, access_stats, inner: RwLock::new(DeltaLayerInner { @@ -632,10 +629,12 @@ impl DeltaLayer { Ok(DeltaLayer { path_or_conf: PathOrConf::Path(path.to_path_buf()), - timeline_id: summary.timeline_id, - tenant_id: summary.tenant_id, - key_range: summary.key_range, - lsn_range: summary.lsn_range, + desc: PersistentLayerDesc::new_delta( + summary.tenant_id, + summary.timeline_id, + summary.key_range, + summary.lsn_range, + ), file_size: metadata.len(), access_stats: LayerAccessStats::empty_will_record_residence_event_later(), inner: RwLock::new(DeltaLayerInner { @@ -648,18 +647,14 @@ impl DeltaLayer { } fn layer_name(&self) -> DeltaFileName { - DeltaFileName { - key_range: self.key_range.clone(), - lsn_range: self.lsn_range.clone(), - } + 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.timeline_id, - self.tenant_id, + self.desc.timeline_id, + self.desc.tenant_id, &self.layer_name(), ) } @@ -803,10 +798,12 @@ impl DeltaLayerWriterInner { // set inner.file here. The first read will have to re-open it. let layer = DeltaLayer { path_or_conf: PathOrConf::Conf(self.conf), - tenant_id: self.tenant_id, - timeline_id: self.timeline_id, - key_range: self.key_start..key_end, - lsn_range: self.lsn_range.clone(), + desc: PersistentLayerDesc::new_delta( + self.tenant_id, + self.timeline_id, + self.key_start..key_end, + self.lsn_range.clone(), + ), file_size: metadata.len(), access_stats: LayerAccessStats::empty_will_record_residence_event_later(), inner: RwLock::new(DeltaLayerInner { diff --git a/pageserver/src/tenant/storage_layer/filename.rs b/pageserver/src/tenant/storage_layer/filename.rs index e2112fc388..5dcd54689e 100644 --- a/pageserver/src/tenant/storage_layer/filename.rs +++ b/pageserver/src/tenant/storage_layer/filename.rs @@ -9,6 +9,8 @@ use std::str::FromStr; use utils::lsn::Lsn; +use super::PersistentLayerDesc; + // Note: Timeline::load_layer_map() relies on this sort order #[derive(PartialEq, Eq, Clone, Hash)] pub struct DeltaFileName { @@ -153,7 +155,7 @@ impl Ord for ImageFileName { impl ImageFileName { pub fn lsn_as_range(&self) -> Range { // Saves from having to copypaste this all over - self.lsn..(self.lsn + 1) + PersistentLayerDesc::image_layer_lsn_range(self.lsn) } } diff --git a/pageserver/src/tenant/storage_layer/image_layer.rs b/pageserver/src/tenant/storage_layer/image_layer.rs index a5dd16fae2..b55dd08a6d 100644 --- a/pageserver/src/tenant/storage_layer/image_layer.rs +++ b/pageserver/src/tenant/storage_layer/image_layer.rs @@ -52,8 +52,8 @@ use utils::{ lsn::Lsn, }; -use super::filename::{ImageFileName, LayerFileName}; -use super::{Layer, LayerAccessStatsReset, LayerIter, PathOrConf}; +use super::filename::ImageFileName; +use super::{Layer, LayerAccessStatsReset, LayerIter, PathOrConf, PersistentLayerDesc}; /// /// Header stored in the beginning of the file @@ -84,9 +84,9 @@ impl From<&ImageLayer> for Summary { Self { magic: IMAGE_FILE_MAGIC, format_version: STORAGE_FORMAT_VERSION, - tenant_id: layer.tenant_id, - timeline_id: layer.timeline_id, - key_range: layer.key_range.clone(), + tenant_id: layer.desc.tenant_id, + timeline_id: layer.desc.timeline_id, + key_range: layer.desc.key_range.clone(), lsn: layer.lsn, index_start_blk: 0, @@ -104,14 +104,13 @@ impl From<&ImageLayer> for Summary { /// and it needs to be loaded before using it in queries. pub struct ImageLayer { path_or_conf: PathOrConf, - pub tenant_id: TenantId, - pub timeline_id: TimelineId, - pub key_range: Range, - pub file_size: u64, - // This entry contains an image of all pages as of this LSN + pub desc: PersistentLayerDesc, + // This entry contains an image of all pages as of this LSN, should be the same as desc.lsn pub lsn: Lsn, + pub file_size: u64, + access_stats: LayerAccessStats, inner: RwLock, @@ -122,7 +121,7 @@ impl std::fmt::Debug for ImageLayer { use super::RangeDisplayDebug; f.debug_struct("ImageLayer") - .field("key_range", &RangeDisplayDebug(&self.key_range)) + .field("key_range", &RangeDisplayDebug(&self.desc.key_range)) .field("file_size", &self.file_size) .field("lsn", &self.lsn) .field("inner", &self.inner) @@ -153,27 +152,15 @@ impl std::fmt::Debug for ImageLayerInner { } impl Layer for ImageLayer { - fn get_key_range(&self) -> Range { - self.key_range.clone() - } - - fn get_lsn_range(&self) -> Range { - // End-bound is exclusive - self.lsn..(self.lsn + 1) - } - fn is_incremental(&self) -> bool { - false - } - - fn short_id(&self) -> String { - self.filename().file_name() - } - /// debugging function to print out the contents of the layer fn dump(&self, verbose: bool, ctx: &RequestContext) -> Result<()> { println!( "----- image layer for ten {} tli {} key {}-{} at {} ----", - self.tenant_id, self.timeline_id, self.key_range.start, self.key_range.end, self.lsn + self.desc.tenant_id, + self.desc.timeline_id, + self.desc.key_range.start, + self.desc.key_range.end, + self.lsn ); if !verbose { @@ -203,7 +190,7 @@ impl Layer for ImageLayer { reconstruct_state: &mut ValueReconstructState, ctx: &RequestContext, ) -> anyhow::Result { - assert!(self.key_range.contains(&key)); + assert!(self.desc.key_range.contains(&key)); assert!(lsn_range.start >= self.lsn); assert!(lsn_range.end >= self.lsn); @@ -230,24 +217,37 @@ impl Layer for ImageLayer { Ok(ValueReconstructResult::Missing) } } + + /// Boilerplate to implement the Layer trait, always use layer_desc for persistent layers. + fn get_key_range(&self) -> Range { + self.layer_desc().key_range.clone() + } + + /// Boilerplate to implement the Layer trait, always use layer_desc for persistent layers. + fn get_lsn_range(&self) -> Range { + self.layer_desc().lsn_range.clone() + } + + /// Boilerplate to implement the Layer trait, always use layer_desc for persistent layers. + fn is_incremental(&self) -> bool { + self.layer_desc().is_incremental + } + + /// Boilerplate to implement the Layer trait, always use layer_desc for persistent layers. + fn short_id(&self) -> String { + self.layer_desc().short_id() + } } impl PersistentLayer for ImageLayer { - fn filename(&self) -> LayerFileName { - self.layer_name().into() + fn layer_desc(&self) -> &PersistentLayerDesc { + &self.desc } fn local_path(&self) -> Option { Some(self.path()) } - fn get_tenant_id(&self) -> TenantId { - self.tenant_id - } - - fn get_timeline_id(&self) -> TimelineId { - self.timeline_id - } fn iter(&self, _ctx: &RequestContext) -> Result> { unimplemented!(); } @@ -405,9 +405,13 @@ impl ImageLayer { ) -> ImageLayer { ImageLayer { path_or_conf: PathOrConf::Conf(conf), - timeline_id, - tenant_id, - key_range: filename.key_range.clone(), + desc: PersistentLayerDesc::new_img( + tenant_id, + timeline_id, + filename.key_range.clone(), + filename.lsn, + false, + ), // Now we assume image layer ALWAYS covers the full range. This may change in the future. lsn: filename.lsn, file_size, access_stats, @@ -433,9 +437,13 @@ impl ImageLayer { .context("get file metadata to determine size")?; Ok(ImageLayer { path_or_conf: PathOrConf::Path(path.to_path_buf()), - timeline_id: summary.timeline_id, - tenant_id: summary.tenant_id, - key_range: summary.key_range, + desc: PersistentLayerDesc::new_img( + summary.tenant_id, + summary.timeline_id, + summary.key_range, + summary.lsn, + false, + ), // Now we assume image layer ALWAYS covers the full range. This may change in the future. lsn: summary.lsn, file_size: metadata.len(), access_stats: LayerAccessStats::empty_will_record_residence_event_later(), @@ -449,18 +457,15 @@ impl ImageLayer { } fn layer_name(&self) -> ImageFileName { - ImageFileName { - key_range: self.key_range.clone(), - lsn: self.lsn, - } + 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.timeline_id, - self.tenant_id, + self.desc.timeline_id, + self.desc.tenant_id, &self.layer_name(), ) } @@ -484,6 +489,7 @@ struct ImageLayerWriterInner { tenant_id: TenantId, key_range: Range, lsn: Lsn, + is_incremental: bool, blob_writer: WriteBlobWriter, tree: DiskBtreeBuilder, @@ -499,6 +505,7 @@ impl ImageLayerWriterInner { tenant_id: TenantId, key_range: &Range, lsn: Lsn, + is_incremental: bool, ) -> anyhow::Result { // Create the file initially with a temporary filename. // We'll atomically rename it to the final name when we're done. @@ -533,6 +540,7 @@ impl ImageLayerWriterInner { lsn, tree: tree_builder, blob_writer, + is_incremental, }; Ok(writer) @@ -570,6 +578,14 @@ impl ImageLayerWriterInner { file.write_all(buf.as_ref())?; } + let desc = PersistentLayerDesc::new_img( + self.tenant_id, + self.timeline_id, + self.key_range.clone(), + self.lsn, + self.is_incremental, // for now, image layer ALWAYS covers the full range + ); + // Fill in the summary on blk 0 let summary = Summary { magic: IMAGE_FILE_MAGIC, @@ -593,9 +609,7 @@ impl ImageLayerWriterInner { // set inner.file here. The first read will have to re-open it. let layer = ImageLayer { path_or_conf: PathOrConf::Conf(self.conf), - timeline_id: self.timeline_id, - tenant_id: self.tenant_id, - key_range: self.key_range.clone(), + desc, lsn: self.lsn, file_size: metadata.len(), access_stats: LayerAccessStats::empty_will_record_residence_event_later(), @@ -667,6 +681,7 @@ impl ImageLayerWriter { tenant_id: TenantId, key_range: &Range, lsn: Lsn, + is_incremental: bool, ) -> anyhow::Result { Ok(Self { inner: Some(ImageLayerWriterInner::new( @@ -675,6 +690,7 @@ impl ImageLayerWriter { tenant_id, key_range, lsn, + is_incremental, )?), }) } diff --git a/pageserver/src/tenant/storage_layer/layer_desc.rs b/pageserver/src/tenant/storage_layer/layer_desc.rs new file mode 100644 index 0000000000..a9859681d3 --- /dev/null +++ b/pageserver/src/tenant/storage_layer/layer_desc.rs @@ -0,0 +1,109 @@ +use std::ops::Range; +use utils::{ + id::{TenantId, TimelineId}, + lsn::Lsn, +}; + +use crate::repository::Key; + +use super::{DeltaFileName, ImageFileName, LayerFileName}; + +/// A unique identifier of a persistent layer. This is different from `LayerDescriptor`, which is only used in the +/// benchmarks. This struct contains all necessary information to find the image / delta layer. It also provides +/// a unified way to generate layer information like file name. +#[derive(Debug, PartialEq, Eq, Clone)] +pub struct PersistentLayerDesc { + pub tenant_id: TenantId, + pub timeline_id: TimelineId, + pub key_range: Range, + /// For image layer, this is `[lsn, lsn+1)`. + pub lsn_range: Range, + /// Whether this is a delta layer. + pub is_delta: bool, + /// Whether this layer only contains page images for part of the keys in the range. In the current implementation, this should + /// always be equal to `is_delta`. If we land the partial image layer PR someday, image layer could also be + /// incremental. + pub is_incremental: bool, +} + +impl PersistentLayerDesc { + pub fn short_id(&self) -> String { + self.filename().file_name() + } + + pub fn new_img( + tenant_id: TenantId, + timeline_id: TimelineId, + key_range: Range, + lsn: Lsn, + is_incremental: bool, + ) -> Self { + Self { + tenant_id, + timeline_id, + key_range, + lsn_range: Self::image_layer_lsn_range(lsn), + is_delta: false, + is_incremental, + } + } + + pub fn new_delta( + tenant_id: TenantId, + timeline_id: TimelineId, + key_range: Range, + lsn_range: Range, + ) -> Self { + Self { + tenant_id, + timeline_id, + key_range, + lsn_range, + is_delta: true, + is_incremental: true, + } + } + + /// Get the LSN that the image layer covers. + pub fn image_layer_lsn(&self) -> Lsn { + assert!(!self.is_delta); + assert!(self.lsn_range.start + 1 == self.lsn_range.end); + self.lsn_range.start + } + + /// Get the LSN range corresponding to a single image layer LSN. + pub fn image_layer_lsn_range(lsn: Lsn) -> Range { + lsn..(lsn + 1) + } + + /// Get a delta file name for this layer. + /// + /// Panic: if this is not a delta layer. + pub fn delta_file_name(&self) -> DeltaFileName { + assert!(self.is_delta); + DeltaFileName { + key_range: self.key_range.clone(), + lsn_range: self.lsn_range.clone(), + } + } + + /// Get a delta file name for this layer. + /// + /// Panic: if this is not an image layer, or the lsn range is invalid + pub fn image_file_name(&self) -> ImageFileName { + assert!(!self.is_delta); + assert!(self.lsn_range.start + 1 == self.lsn_range.end); + ImageFileName { + key_range: self.key_range.clone(), + lsn: self.lsn_range.start, + } + } + + pub fn filename(&self) -> LayerFileName { + if self.is_delta { + self.delta_file_name().into() + } else { + self.image_file_name().into() + } + } +} diff --git a/pageserver/src/tenant/storage_layer/remote_layer.rs b/pageserver/src/tenant/storage_layer/remote_layer.rs index 2106587ab2..ff0f44da92 100644 --- a/pageserver/src/tenant/storage_layer/remote_layer.rs +++ b/pageserver/src/tenant/storage_layer/remote_layer.rs @@ -18,11 +18,10 @@ use utils::{ lsn::Lsn, }; -use super::filename::{DeltaFileName, ImageFileName, LayerFileName}; -use super::image_layer::ImageLayer; +use super::filename::{DeltaFileName, ImageFileName}; use super::{ - DeltaLayer, LayerAccessStats, LayerAccessStatsReset, LayerIter, LayerKeyIter, - LayerResidenceStatus, PersistentLayer, + DeltaLayer, ImageLayer, LayerAccessStats, LayerAccessStatsReset, LayerIter, LayerKeyIter, + LayerResidenceStatus, PersistentLayer, PersistentLayerDesc, }; /// RemoteLayer is a not yet downloaded [`ImageLayer`] or @@ -34,19 +33,10 @@ use super::{ /// /// See: [`crate::context::RequestContext`] for authorization to download pub struct RemoteLayer { - tenantid: TenantId, - timelineid: TimelineId, - key_range: Range, - lsn_range: Range, - - pub file_name: LayerFileName, + pub desc: PersistentLayerDesc, pub layer_metadata: LayerFileMetadata, - is_delta: bool, - - is_incremental: bool, - access_stats: LayerAccessStats, pub(crate) ongoing_download: Arc, @@ -66,22 +56,14 @@ pub struct RemoteLayer { impl std::fmt::Debug for RemoteLayer { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("RemoteLayer") - .field("file_name", &self.file_name) + .field("file_name", &self.desc.filename()) .field("layer_metadata", &self.layer_metadata) - .field("is_incremental", &self.is_incremental) + .field("is_incremental", &self.desc.is_incremental) .finish() } } impl Layer for RemoteLayer { - fn get_key_range(&self) -> Range { - self.key_range.clone() - } - - fn get_lsn_range(&self) -> Range { - self.lsn_range.clone() - } - fn get_value_reconstruct_data( &self, _key: Key, @@ -95,53 +77,45 @@ impl Layer for RemoteLayer { ); } - fn is_incremental(&self) -> bool { - self.is_incremental - } - /// debugging function to print out the contents of the layer fn dump(&self, _verbose: bool, _ctx: &RequestContext) -> Result<()> { println!( "----- remote layer for ten {} tli {} keys {}-{} lsn {}-{} ----", - self.tenantid, - self.timelineid, - self.key_range.start, - self.key_range.end, - self.lsn_range.start, - self.lsn_range.end + self.desc.tenant_id, + self.desc.timeline_id, + self.desc.key_range.start, + self.desc.key_range.end, + self.desc.lsn_range.start, + self.desc.lsn_range.end ); Ok(()) } + /// Boilerplate to implement the Layer trait, always use layer_desc for persistent layers. + fn get_key_range(&self) -> Range { + self.layer_desc().key_range.clone() + } + + /// Boilerplate to implement the Layer trait, always use layer_desc for persistent layers. + fn get_lsn_range(&self) -> Range { + self.layer_desc().lsn_range.clone() + } + + /// Boilerplate to implement the Layer trait, always use layer_desc for persistent layers. + fn is_incremental(&self) -> bool { + self.layer_desc().is_incremental + } + + /// Boilerplate to implement the Layer trait, always use layer_desc for persistent layers. fn short_id(&self) -> String { - self.filename().file_name() + self.layer_desc().short_id() } } impl PersistentLayer for RemoteLayer { - fn get_tenant_id(&self) -> TenantId { - self.tenantid - } - - fn get_timeline_id(&self) -> TimelineId { - self.timelineid - } - - fn filename(&self) -> LayerFileName { - if self.is_delta { - DeltaFileName { - key_range: self.key_range.clone(), - lsn_range: self.lsn_range.clone(), - } - .into() - } else { - ImageFileName { - key_range: self.key_range.clone(), - lsn: self.lsn_range.start, - } - .into() - } + fn layer_desc(&self) -> &PersistentLayerDesc { + &self.desc } fn local_path(&self) -> Option { @@ -176,7 +150,7 @@ impl PersistentLayer for RemoteLayer { let layer_file_name = self.filename().file_name(); let lsn_range = self.get_lsn_range(); - if self.is_delta { + if self.desc.is_delta { HistoricLayerInfo::Delta { layer_file_name, layer_file_size: self.layer_metadata.file_size(), @@ -210,13 +184,13 @@ impl RemoteLayer { access_stats: LayerAccessStats, ) -> RemoteLayer { RemoteLayer { - tenantid, - timelineid, - key_range: fname.key_range.clone(), - lsn_range: fname.lsn_as_range(), - is_delta: false, - is_incremental: false, - file_name: fname.to_owned().into(), + desc: PersistentLayerDesc::new_img( + tenantid, + timelineid, + fname.key_range.clone(), + fname.lsn, + false, + ), layer_metadata: layer_metadata.clone(), ongoing_download: Arc::new(tokio::sync::Semaphore::new(1)), download_replacement_failure: std::sync::atomic::AtomicBool::default(), @@ -232,13 +206,12 @@ impl RemoteLayer { access_stats: LayerAccessStats, ) -> RemoteLayer { RemoteLayer { - tenantid, - timelineid, - key_range: fname.key_range.clone(), - lsn_range: fname.lsn_range.clone(), - is_delta: true, - is_incremental: true, - file_name: fname.to_owned().into(), + desc: PersistentLayerDesc::new_delta( + tenantid, + timelineid, + fname.key_range.clone(), + fname.lsn_range.clone(), + ), layer_metadata: layer_metadata.clone(), ongoing_download: Arc::new(tokio::sync::Semaphore::new(1)), download_replacement_failure: std::sync::atomic::AtomicBool::default(), @@ -256,15 +229,12 @@ impl RemoteLayer { where L: ?Sized + Layer, { - if self.is_delta { - let fname = DeltaFileName { - key_range: self.key_range.clone(), - lsn_range: self.lsn_range.clone(), - }; + if self.desc.is_delta { + let fname = self.desc.delta_file_name(); Arc::new(DeltaLayer::new( conf, - self.timelineid, - self.tenantid, + self.desc.timeline_id, + self.desc.tenant_id, &fname, file_size, self.access_stats.clone_for_residence_change( @@ -273,14 +243,11 @@ impl RemoteLayer { ), )) } else { - let fname = ImageFileName { - key_range: self.key_range.clone(), - lsn: self.lsn_range.start, - }; + let fname = self.desc.image_file_name(); Arc::new(ImageLayer::new( conf, - self.timelineid, - self.tenantid, + self.desc.timeline_id, + self.desc.tenant_id, &fname, file_size, self.access_stats.clone_for_residence_change( diff --git a/pageserver/src/tenant/timeline.rs b/pageserver/src/tenant/timeline.rs index 8885e761a2..b45dcc4e42 100644 --- a/pageserver/src/tenant/timeline.rs +++ b/pageserver/src/tenant/timeline.rs @@ -2533,7 +2533,7 @@ impl Timeline { (DownloadBehavior::Error, false) => { return Err(PageReconstructError::NeedsDownload( TenantTimelineId::new(self.tenant_id, self.timeline_id), - remote_layer.file_name.clone(), + remote_layer.filename(), )) } } @@ -3066,6 +3066,7 @@ impl Timeline { self.tenant_id, &img_range, lsn, + false, // image layer always covers the full range )?; fail_point!("image-layer-writer-fail-before-finish", |_| { @@ -4126,7 +4127,7 @@ impl Timeline { // Does retries + exponential back-off internally. // When this fails, don't layer further retry attempts here. let result = remote_client - .download_layer_file(&remote_layer.file_name, &remote_layer.layer_metadata) + .download_layer_file(&remote_layer.filename(), &remote_layer.layer_metadata) .await; if let Ok(size) = &result {