From f70871dfd0eb3d0cd3a0d63da765c3e188c6ddcd Mon Sep 17 00:00:00 2001 From: Joonas Koivunen Date: Thu, 24 Aug 2023 17:22:36 +0300 Subject: [PATCH] internal-devx: pageserver future layers (#5092) I've personally forgotten why/how can we have future layers during reconciliation. Adds `#[cfg(feature = "testing")]` logging when we upload such index_part.json, with a cross reference to where the cleanup happens. Latest private slack thread: https://neondb.slack.com/archives/C033RQ5SPDH/p1692879032573809?thread_ts=1692792276.173979&cid=C033RQ5SPDH Builds upon #5074. Should had been considered on #4837. --- pageserver/src/tenant/remote_timeline_client.rs | 13 +++++++++++++ pageserver/src/tenant/storage_layer/filename.rs | 6 +++--- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/pageserver/src/tenant/remote_timeline_client.rs b/pageserver/src/tenant/remote_timeline_client.rs index a82e32b659..835ee6aea4 100644 --- a/pageserver/src/tenant/remote_timeline_client.rs +++ b/pageserver/src/tenant/remote_timeline_client.rs @@ -1076,6 +1076,15 @@ impl RemoteTimelineClient { .await } UploadOp::UploadMetadata(ref index_part, _lsn) => { + let mention_having_future_layers = if cfg!(feature = "testing") { + index_part + .layer_metadata + .keys() + .any(|x| x.is_in_future(*_lsn)) + } else { + false + }; + let res = upload::upload_index_part( self.conf, &self.storage_impl, @@ -1093,6 +1102,10 @@ impl RemoteTimelineClient { .await; if res.is_ok() { self.update_remote_physical_size_gauge(Some(index_part)); + if mention_having_future_layers { + // find rationale near crate::tenant::timeline::init::cleanup_future_layer + tracing::info!(disk_consistent_lsn=%_lsn, "uploaded an index_part.json with future layers -- this is ok! if shutdown now, expect future layer cleanup"); + } } res } diff --git a/pageserver/src/tenant/storage_layer/filename.rs b/pageserver/src/tenant/storage_layer/filename.rs index 922bb18bff..b52c20a7c6 100644 --- a/pageserver/src/tenant/storage_layer/filename.rs +++ b/pageserver/src/tenant/storage_layer/filename.rs @@ -212,7 +212,7 @@ pub enum LayerFileName { } impl LayerFileName { - pub fn file_name(&self) -> String { + pub(crate) fn file_name(&self) -> String { self.to_string() } @@ -274,8 +274,8 @@ impl serde::Serialize for LayerFileName { S: serde::Serializer, { match self { - Self::Image(fname) => serializer.serialize_str(&fname.to_string()), - Self::Delta(fname) => serializer.serialize_str(&fname.to_string()), + Self::Image(fname) => serializer.collect_str(fname), + Self::Delta(fname) => serializer.collect_str(fname), } } }