diff --git a/pageserver/src/tenant/delta_layer.rs b/pageserver/src/tenant/delta_layer.rs index a252abf2a0..e1006dfe00 100644 --- a/pageserver/src/tenant/delta_layer.rs +++ b/pageserver/src/tenant/delta_layer.rs @@ -387,8 +387,8 @@ impl PersistentLayer for DeltaLayer { self.layer_name().into() } - fn local_path(&self) -> PathBuf { - self.path() + fn local_path(&self) -> Option { + Some(self.path()) } fn iter(&self) -> Result> { diff --git a/pageserver/src/tenant/image_layer.rs b/pageserver/src/tenant/image_layer.rs index c907d21af5..b1dbbfb683 100644 --- a/pageserver/src/tenant/image_layer.rs +++ b/pageserver/src/tenant/image_layer.rs @@ -208,8 +208,8 @@ impl PersistentLayer for ImageLayer { self.layer_name().into() } - fn local_path(&self) -> PathBuf { - self.path() + fn local_path(&self) -> Option { + Some(self.path()) } fn get_tenant_id(&self) -> TenantId { diff --git a/pageserver/src/tenant/storage_layer.rs b/pageserver/src/tenant/storage_layer.rs index ba0311574d..79eaa96591 100644 --- a/pageserver/src/tenant/storage_layer.rs +++ b/pageserver/src/tenant/storage_layer.rs @@ -147,7 +147,8 @@ pub trait PersistentLayer: Layer { fn filename(&self) -> LayerFileName; // Path to the layer file in the local filesystem. - fn local_path(&self) -> PathBuf; + // `None` for `RemoteLayer`. + fn local_path(&self) -> Option; /// Iterate through all keys and values stored in the layer fn iter(&self) -> Result>; diff --git a/pageserver/src/tenant/timeline.rs b/pageserver/src/tenant/timeline.rs index 0b31c9bdc4..59d3486644 100644 --- a/pageserver/src/tenant/timeline.rs +++ b/pageserver/src/tenant/timeline.rs @@ -1031,11 +1031,13 @@ impl Timeline { .unwrap_or(LayerFileMetadata::MISSING); // Is the local layer's size different from the size stored in the - // remote index file? If so, rename_to_backup those files & remove - // local_layer form the layer map. - // We'll download a fresh copy of the layer file below. + // remote index file? + // If so, rename_to_backup those files & replace their local layer with + // a RemoteLayer in the laye rmap so that we re-download them on-demand. if let Some(local_layer) = local_layer { - let local_layer_path = local_layer.local_path(); + let local_layer_path = local_layer + .local_path() + .expect("caller must ensure that local_layers only contains local layers"); ensure!( local_layer_path.exists(), "every layer from local_layers must exist on disk: {}", @@ -1210,7 +1212,10 @@ impl Timeline { // Are there local files that don't exist remotely? Schedule uploads for them for (layer_name, layer) in &local_only_layers { - let layer_path = layer.local_path(); + // XXX solve this in the type system + let layer_path = layer + .local_path() + .expect("local_only_layers only contains local layers"); let layer_size = layer_path .metadata() .with_context(|| format!("failed to get file {layer_path:?} metadata"))? @@ -1450,12 +1455,21 @@ trait TraversalLayerExt { impl TraversalLayerExt for Arc { fn traversal_id(&self) -> String { - debug_assert!( - self.local_path().to_str().unwrap() - .contains(&format!("{}", self.get_timeline_id())), - "need timeline ID to uniquely identify the layer when tranversal crosses ancestor boundary", - ); - format!("{}", self.local_path().display()) + match self.local_path() { + Some(local_path) => { + debug_assert!(local_path.to_str().unwrap().contains(&format!("{}", self.get_timeline_id())), + "need timeline ID to uniquely identify the layer when tranversal crosses ancestor boundary", + ); + format!("{}", local_path.display()) + } + None => { + format!( + "remote {}/{}", + self.get_timeline_id(), + self.filename().file_name() + ) + } + } } } @@ -2440,10 +2454,11 @@ impl Timeline { // delete the old ones let mut layer_names_to_delete = Vec::with_capacity(deltas_to_compact.len()); for l in deltas_to_compact { - let path = l.local_path(); - self.metrics - .current_physical_size_gauge - .sub(path.metadata()?.len()); + if let Some(path) = l.local_path() { + self.metrics + .current_physical_size_gauge + .sub(path.metadata()?.len()); + } layer_names_to_delete.push(l.filename()); l.delete()?; layers.remove_historic(l); @@ -2726,10 +2741,11 @@ impl Timeline { // while iterating it. BTreeMap::retain() would be another option) let mut layer_names_to_delete = Vec::with_capacity(layers_to_remove.len()); for doomed_layer in layers_to_remove { - let path = doomed_layer.local_path(); - self.metrics - .current_physical_size_gauge - .sub(path.metadata()?.len()); + if let Some(path) = doomed_layer.local_path() { + self.metrics + .current_physical_size_gauge + .sub(path.metadata()?.len()); + } layer_names_to_delete.push(doomed_layer.filename()); doomed_layer.delete()?; layers.remove_historic(doomed_layer);