From bcef542d5b5411c2111a605dfa288461a2159b3a Mon Sep 17 00:00:00 2001 From: Erik Grinaker Date: Tue, 15 Apr 2025 16:25:58 +0200 Subject: [PATCH] pageserver: don't rewrite invisible layers during ancestor compaction (#11580) ## Problem Shard ancestor compaction can be very expensive following shard splits of large tenants. We currently rewrite garbage layers after shard splits as well, which can be a significant amount of data. Touches https://github.com/neondatabase/cloud/issues/22532. ## Summary of changes Don't rewrite invisible layers after shard splits. --- pageserver/src/tenant/timeline/compaction.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/pageserver/src/tenant/timeline/compaction.rs b/pageserver/src/tenant/timeline/compaction.rs index 3d5f11aeb9..5d5149e2d4 100644 --- a/pageserver/src/tenant/timeline/compaction.rs +++ b/pageserver/src/tenant/timeline/compaction.rs @@ -56,7 +56,8 @@ use crate::tenant::storage_layer::batch_split_writer::{ use crate::tenant::storage_layer::filter_iterator::FilterIterator; use crate::tenant::storage_layer::merge_iterator::MergeIterator; use crate::tenant::storage_layer::{ - AsLayerDesc, PersistentLayerDesc, PersistentLayerKey, ValueReconstructState, + AsLayerDesc, LayerVisibilityHint, PersistentLayerDesc, PersistentLayerKey, + ValueReconstructState, }; use crate::tenant::tasks::log_compaction_error; use crate::tenant::timeline::{ @@ -1348,12 +1349,19 @@ impl Timeline { continue; } + // We do not yet implement rewrite of delta layers. if layer_desc.is_delta() { - // We do not yet implement rewrite of delta layers debug!(%layer, "Skipping rewrite of delta layer"); continue; } + // We don't bother rewriting layers that aren't visible, since these won't be needed by + // reads and will likely be garbage collected soon. + if layer.visibility() != LayerVisibilityHint::Visible { + debug!(%layer, "Skipping rewrite of invisible layer"); + continue; + } + // Only rewrite layers if their generations differ. This guarantees: // - that local rewrite is safe, as local layer paths will differ between existing layer and rewritten one // - that the layer is persistent in remote storage, as we only see old-generation'd layer via loading from remote storage