diff --git a/pageserver/benches/bench_layer_map.rs b/pageserver/benches/bench_layer_map.rs index 5d05af0c00..32ccd456a6 100644 --- a/pageserver/benches/bench_layer_map.rs +++ b/pageserver/benches/bench_layer_map.rs @@ -3,7 +3,6 @@ use pageserver::repository::Key; use pageserver::tenant::layer_map::LayerMap; use pageserver::tenant::storage_layer::LayerFileName; use pageserver::tenant::storage_layer::PersistentLayerDesc; -use pageserver_api::shard::TenantShardId; use rand::prelude::{SeedableRng, SliceRandom, StdRng}; use std::cmp::{max, min}; use std::fs::File; @@ -11,7 +10,6 @@ use std::io::{BufRead, BufReader}; use std::path::PathBuf; use std::str::FromStr; use std::time::Instant; -use utils::id::{TenantId, TimelineId}; use utils::lsn::Lsn; @@ -211,13 +209,8 @@ fn bench_sequential(c: &mut Criterion) { for i in 0..100_000 { let i32 = (i as u32) % 100; let zero = Key::from_hex("000000000000000000000000000000000000").unwrap(); - let layer = PersistentLayerDesc::new_img( - TenantShardId::unsharded(TenantId::generate()), - TimelineId::generate(), - zero.add(10 * i32)..zero.add(10 * i32 + 1), - Lsn(i), - 0, - ); + let layer = + PersistentLayerDesc::new_img(zero.add(10 * i32)..zero.add(10 * i32 + 1), Lsn(i), 0); updates.insert_historic(layer); } updates.flush(); diff --git a/pageserver/src/disk_usage_eviction_task.rs b/pageserver/src/disk_usage_eviction_task.rs index f01cd1cf8c..89bcb904b2 100644 --- a/pageserver/src/disk_usage_eviction_task.rs +++ b/pageserver/src/disk_usage_eviction_task.rs @@ -310,8 +310,8 @@ pub async fn disk_usage_eviction_task_iteration_impl( .unwrap() .as_micros(), partition, - desc.tenant_shard_id, - desc.timeline_id, + candidate.timeline.tenant_shard_id, + candidate.timeline.timeline_id, candidate.layer, ); } diff --git a/pageserver/src/tenant/storage_layer.rs b/pageserver/src/tenant/storage_layer.rs index 944e05883f..693bb1e751 100644 --- a/pageserver/src/tenant/storage_layer.rs +++ b/pageserver/src/tenant/storage_layer.rs @@ -24,7 +24,7 @@ use tracing::warn; use utils::history_buffer::HistoryBufferWithDropCounter; use utils::rate_limit::RateLimit; -use utils::{id::TimelineId, lsn::Lsn}; +use utils::lsn::Lsn; pub use delta_layer::{DeltaLayer, DeltaLayerWriter, ValueRef}; pub use filename::{DeltaFileName, ImageFileName, LayerFileName}; @@ -301,31 +301,17 @@ pub trait AsLayerDesc { } pub mod tests { - use pageserver_api::shard::TenantShardId; - use super::*; impl From for PersistentLayerDesc { fn from(value: DeltaFileName) -> Self { - PersistentLayerDesc::new_delta( - TenantShardId::from([0; 18]), - TimelineId::from_array([0; 16]), - value.key_range, - value.lsn_range, - 233, - ) + PersistentLayerDesc::new_delta(value.key_range, value.lsn_range, 233) } } impl From for PersistentLayerDesc { fn from(value: ImageFileName) -> Self { - PersistentLayerDesc::new_img( - TenantShardId::from([0; 18]), - TimelineId::from_array([0; 16]), - value.key_range, - value.lsn, - 233, - ) + PersistentLayerDesc::new_img(value.key_range, value.lsn, 233) } } diff --git a/pageserver/src/tenant/storage_layer/delta_layer.rs b/pageserver/src/tenant/storage_layer/delta_layer.rs index d339204127..4d9b0a9838 100644 --- a/pageserver/src/tenant/storage_layer/delta_layer.rs +++ b/pageserver/src/tenant/storage_layer/delta_layer.rs @@ -84,17 +84,6 @@ pub struct Summary { pub index_root_blk: u32, } -impl From<&DeltaLayer> for Summary { - fn from(layer: &DeltaLayer) -> Self { - Self::expected( - layer.desc.tenant_shard_id.tenant_id, - layer.desc.timeline_id, - layer.desc.key_range.clone(), - layer.desc.lsn_range.clone(), - ) - } -} - impl Summary { pub(super) fn expected( tenant_id: TenantId, @@ -320,15 +309,9 @@ impl DeltaLayer { .metadata() .context("get file metadata to determine size")?; - // TODO(sharding): we must get the TenantShardId from the path instead of reading the Summary. - // we should also validate the path against the Summary, as both should contain the same tenant, timeline, key, lsn. - let tenant_shard_id = TenantShardId::unsharded(summary.tenant_id); - Ok(DeltaLayer { path: path.to_path_buf(), desc: PersistentLayerDesc::new_delta( - tenant_shard_id, - summary.timeline_id, summary.key_range, summary.lsn_range, metadata.len(), @@ -505,8 +488,6 @@ impl DeltaLayerWriterInner { // set inner.file here. The first read will have to re-open it. let desc = PersistentLayerDesc::new_delta( - self.tenant_shard_id, - self.timeline_id, self.key_start..key_end, self.lsn_range.clone(), metadata.len(), diff --git a/pageserver/src/tenant/storage_layer/image_layer.rs b/pageserver/src/tenant/storage_layer/image_layer.rs index 023122c0b1..07182ea82c 100644 --- a/pageserver/src/tenant/storage_layer/image_layer.rs +++ b/pageserver/src/tenant/storage_layer/image_layer.rs @@ -85,17 +85,6 @@ pub struct Summary { // the 'values' part starts after the summary header, on block 1. } -impl From<&ImageLayer> for Summary { - fn from(layer: &ImageLayer) -> Self { - Self::expected( - layer.desc.tenant_shard_id.tenant_id, - layer.desc.timeline_id, - layer.desc.key_range.clone(), - layer.lsn, - ) - } -} - impl Summary { pub(super) fn expected( tenant_id: TenantId, @@ -278,19 +267,9 @@ impl ImageLayer { .metadata() .context("get file metadata to determine size")?; - // TODO(sharding): we should get TenantShardId from path. - // OR, not at all: any layer we load from disk should also get reconciled with remote IndexPart. - let tenant_shard_id = TenantShardId::unsharded(summary.tenant_id); - Ok(ImageLayer { path: path.to_path_buf(), - desc: PersistentLayerDesc::new_img( - tenant_shard_id, - summary.timeline_id, - summary.key_range, - summary.lsn, - metadata.len(), - ), // Now we assume image layer ALWAYS covers the full range. This may change in the future. + desc: PersistentLayerDesc::new_img(summary.key_range, summary.lsn, metadata.len()), // Now we assume image layer ALWAYS covers the full range. This may change in the future. lsn: summary.lsn, access_stats: LayerAccessStats::empty_will_record_residence_event_later(), inner: OnceCell::new(), @@ -581,13 +560,7 @@ impl ImageLayerWriterInner { .await .context("get metadata to determine file size")?; - let desc = PersistentLayerDesc::new_img( - self.tenant_shard_id, - self.timeline_id, - self.key_range.clone(), - self.lsn, - metadata.len(), - ); + let desc = PersistentLayerDesc::new_img(self.key_range.clone(), self.lsn, metadata.len()); // Note: Because we open the file in write-only mode, we cannot // reuse the same VirtualFile for reading later. That's why we don't diff --git a/pageserver/src/tenant/storage_layer/layer.rs b/pageserver/src/tenant/storage_layer/layer.rs index e203d9d334..e01034face 100644 --- a/pageserver/src/tenant/storage_layer/layer.rs +++ b/pageserver/src/tenant/storage_layer/layer.rs @@ -81,12 +81,7 @@ impl Layer { file_name: LayerFileName, metadata: LayerFileMetadata, ) -> Self { - let desc = PersistentLayerDesc::from_filename( - timeline.tenant_shard_id, - timeline.timeline_id, - file_name, - metadata.file_size(), - ); + let desc = PersistentLayerDesc::from_filename(file_name, metadata.file_size()); let access_stats = LayerAccessStats::for_loading_layer(LayerResidenceStatus::Evicted); @@ -112,12 +107,7 @@ impl Layer { file_name: LayerFileName, metadata: LayerFileMetadata, ) -> ResidentLayer { - let desc = PersistentLayerDesc::from_filename( - timeline.tenant_shard_id, - timeline.timeline_id, - file_name, - metadata.file_size(), - ); + let desc = PersistentLayerDesc::from_filename(file_name, metadata.file_size()); let access_stats = LayerAccessStats::for_loading_layer(LayerResidenceStatus::Resident); @@ -490,7 +480,10 @@ impl Drop for LayerInner { return; } - let span = tracing::info_span!(parent: None, "layer_gc", tenant_id = %self.layer_desc().tenant_shard_id.tenant_id, shard_id=%self.layer_desc().tenant_shard_id.shard_slug(), timeline_id = %self.layer_desc().timeline_id); + // If timeline is alive, we can construct a span with IDs for this function. + let span = self.timeline.upgrade().map(|timeline| { + tracing::info_span!(parent: None, "layer_gc", tenant_id = %timeline.tenant_shard_id.tenant_id, shard_id=%timeline.tenant_shard_id.shard_slug(), timeline_id = %timeline.timeline_id) + }); let path = std::mem::take(&mut self.path); let file_name = self.layer_desc().filename(); @@ -500,7 +493,7 @@ impl Drop for LayerInner { let status = self.status.clone(); crate::task_mgr::BACKGROUND_RUNTIME.spawn_blocking(move || { - let _g = span.entered(); + let _g = span.map(|s| s.entered()); // carry this until we are finished for [`Layer::wait_drop`] support let _status = status; @@ -836,8 +829,8 @@ impl LayerInner { crate::task_mgr::spawn( &tokio::runtime::Handle::current(), crate::task_mgr::TaskKind::RemoteDownloadTask, - Some(self.desc.tenant_shard_id.tenant_id), - Some(self.desc.timeline_id), + Some(timeline.tenant_shard_id.tenant_id), + Some(timeline.timeline_id), &task_name, false, async move { @@ -1001,7 +994,10 @@ impl LayerInner { if gc { // do nothing now, only in LayerInner::drop } else if can_evict && evict { - let span = tracing::info_span!(parent: None, "layer_evict", tenant_id = %self.desc.tenant_shard_id.tenant_id, shard_id = %self.desc.tenant_shard_id.shard_slug(), timeline_id = %self.desc.timeline_id, layer=%self, %version); + // If timeline is alive, we can construct a span with IDs for this function. + let span = self.timeline.upgrade().map(|timeline| { + tracing::info_span!(parent: None, "layer_evict", tenant_id = %timeline.tenant_shard_id.tenant_id, shard_id=%timeline.tenant_shard_id.shard_slug(), timeline_id = %timeline.timeline_id) + }); // downgrade for queueing, in case there's a tear down already ongoing we should not // hold it alive. @@ -1012,7 +1008,7 @@ impl LayerInner { // drop while the `self.inner` is being locked, leading to a deadlock. crate::task_mgr::BACKGROUND_RUNTIME.spawn_blocking(move || { - let _g = span.entered(); + let _g = span.map(|s| s.entered()); // if LayerInner is already dropped here, do nothing because the garbage collection // has already ran while we were in queue @@ -1224,6 +1220,11 @@ impl DownloadedLayer { owner: &Arc, ctx: &RequestContext, ) -> anyhow::Result<&'a LayerKind> { + let timeline = owner + .timeline + .upgrade() + .ok_or(DownloadError::TimelineShutdown)?; + let init = || async { assert_eq!( Weak::as_ptr(&self.owner), @@ -1233,8 +1234,8 @@ impl DownloadedLayer { let res = if owner.desc.is_delta { let summary = Some(delta_layer::Summary::expected( - owner.desc.tenant_shard_id.tenant_id, - owner.desc.timeline_id, + timeline.tenant_shard_id.tenant_id, + timeline.timeline_id, owner.desc.key_range.clone(), owner.desc.lsn_range.clone(), )); @@ -1244,8 +1245,8 @@ impl DownloadedLayer { } else { let lsn = owner.desc.image_layer_lsn(); let summary = Some(image_layer::Summary::expected( - owner.desc.tenant_shard_id.tenant_id, - owner.desc.timeline_id, + timeline.tenant_shard_id.tenant_id, + timeline.timeline_id, owner.desc.key_range.clone(), lsn, )); diff --git a/pageserver/src/tenant/storage_layer/layer_desc.rs b/pageserver/src/tenant/storage_layer/layer_desc.rs index bf24407fc5..717314101a 100644 --- a/pageserver/src/tenant/storage_layer/layer_desc.rs +++ b/pageserver/src/tenant/storage_layer/layer_desc.rs @@ -1,7 +1,6 @@ use core::fmt::Display; -use pageserver_api::shard::TenantShardId; use std::ops::Range; -use utils::{id::TimelineId, lsn::Lsn}; +use utils::lsn::Lsn; use crate::repository::Key; @@ -9,16 +8,11 @@ use super::{DeltaFileName, ImageFileName, LayerFileName}; use serde::{Deserialize, Serialize}; -#[cfg(test)] -use utils::id::TenantId; - /// 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, Serialize, Deserialize)] pub struct PersistentLayerDesc { - pub tenant_shard_id: TenantShardId, - pub timeline_id: TimelineId, /// Range of keys that this layer covers pub key_range: Range, /// Inclusive start, exclusive end of the LSN range that this layer holds. @@ -57,8 +51,6 @@ impl PersistentLayerDesc { #[cfg(test)] pub fn new_test(key_range: Range) -> Self { Self { - tenant_shard_id: TenantShardId::unsharded(TenantId::generate()), - timeline_id: TimelineId::generate(), key_range, lsn_range: Lsn(0)..Lsn(1), is_delta: false, @@ -66,16 +58,8 @@ impl PersistentLayerDesc { } } - pub fn new_img( - tenant_shard_id: TenantShardId, - timeline_id: TimelineId, - key_range: Range, - lsn: Lsn, - file_size: u64, - ) -> Self { + pub fn new_img(key_range: Range, lsn: Lsn, file_size: u64) -> Self { Self { - tenant_shard_id, - timeline_id, key_range, lsn_range: Self::image_layer_lsn_range(lsn), is_delta: false, @@ -83,16 +67,8 @@ impl PersistentLayerDesc { } } - pub fn new_delta( - tenant_shard_id: TenantShardId, - timeline_id: TimelineId, - key_range: Range, - lsn_range: Range, - file_size: u64, - ) -> Self { + pub fn new_delta(key_range: Range, lsn_range: Range, file_size: u64) -> Self { Self { - tenant_shard_id, - timeline_id, key_range, lsn_range, is_delta: true, @@ -100,23 +76,10 @@ impl PersistentLayerDesc { } } - pub fn from_filename( - tenant_shard_id: TenantShardId, - timeline_id: TimelineId, - filename: LayerFileName, - file_size: u64, - ) -> Self { + pub fn from_filename(filename: LayerFileName, file_size: u64) -> Self { match filename { - LayerFileName::Image(i) => { - Self::new_img(tenant_shard_id, timeline_id, i.key_range, i.lsn, file_size) - } - LayerFileName::Delta(d) => Self::new_delta( - tenant_shard_id, - timeline_id, - d.key_range, - d.lsn_range, - file_size, - ), + LayerFileName::Image(i) => Self::new_img(i.key_range, i.lsn, file_size), + LayerFileName::Delta(d) => Self::new_delta(d.key_range, d.lsn_range, file_size), } } @@ -173,10 +136,6 @@ impl PersistentLayerDesc { self.key_range.clone() } - pub fn get_timeline_id(&self) -> TimelineId { - self.timeline_id - } - /// Does this layer only contain some data for the key-range (incremental), /// or does it contain a version of every page? This is important to know /// for garbage collecting old layers: an incremental layer depends on @@ -192,9 +151,7 @@ impl PersistentLayerDesc { pub fn dump(&self) { if self.is_delta { println!( - "----- delta layer for ten {} tli {} keys {}-{} lsn {}-{} is_incremental {} size {} ----", - self.tenant_shard_id, - self.timeline_id, + "----- delta layer keys {}-{} lsn {}-{} is_incremental {} size {} ----", self.key_range.start, self.key_range.end, self.lsn_range.start, @@ -204,9 +161,7 @@ impl PersistentLayerDesc { ); } else { println!( - "----- image layer for ten {} tli {} key {}-{} at {} is_incremental {} size {} ----", - self.tenant_shard_id, - self.timeline_id, + "----- image layer key {}-{} at {} is_incremental {} size {} ----", self.key_range.start, self.key_range.end, self.image_layer_lsn(),