diff --git a/pageserver/src/tenant/remote_timeline_client.rs b/pageserver/src/tenant/remote_timeline_client.rs index 95d0a8a1c8..1b3514500d 100644 --- a/pageserver/src/tenant/remote_timeline_client.rs +++ b/pageserver/src/tenant/remote_timeline_client.rs @@ -201,7 +201,6 @@ //! [`Tenant::timeline_init_and_sync`]: super::Tenant::timeline_init_and_sync //! [`Timeline::reconcile_with_remote`]: super::Timeline::reconcile_with_remote -mod delete; mod download; pub mod index; mod upload; @@ -234,7 +233,6 @@ use crate::metrics::{ }; use crate::tenant::debug_assert_current_span_has_tenant_and_timeline_id; use crate::tenant::remote_timeline_client::index::LayerFileMetadata; -use crate::tenant::upload_queue::Delete; use crate::{ config::PageServerConf, task_mgr, @@ -909,10 +907,6 @@ impl RemoteTimelineClient { // have finished. upload_queue.inprogress_tasks.is_empty() } - UploadOp::Delete(_) => { - // Wait for preceding uploads to finish. Concurrent deletions are OK, though. - upload_queue.num_inprogress_deletions == upload_queue.inprogress_tasks.len() - } UploadOp::Barrier(_) => upload_queue.inprogress_tasks.is_empty(), }; @@ -940,9 +934,6 @@ impl RemoteTimelineClient { UploadOp::UploadMetadata(_, _) => { upload_queue.num_inprogress_metadata_uploads += 1; } - UploadOp::Delete(_) => { - upload_queue.num_inprogress_deletions += 1; - } UploadOp::Barrier(sender) => { sender.send_replace(()); continue; @@ -1061,21 +1052,6 @@ impl RemoteTimelineClient { } res } - UploadOp::Delete(delete) => { - let path = &self - .conf - .timeline_path(&self.tenant_id, &self.timeline_id) - .join(delete.layer_file_name.file_name()); - delete::delete_layer(self.conf, &self.storage_impl, path) - .measure_remote_op( - self.tenant_id, - self.timeline_id, - delete.file_kind, - RemoteOpKind::Delete, - Arc::clone(&self.metrics), - ) - .await - } UploadOp::Barrier(_) => { // unreachable. Barrier operations are handled synchronously in // launch_queued_tasks @@ -1136,15 +1112,7 @@ impl RemoteTimelineClient { let mut upload_queue_guard = self.upload_queue.lock().unwrap(); let upload_queue = match upload_queue_guard.deref_mut() { UploadQueue::Uninitialized => panic!("callers are responsible for ensuring this is only called on an initialized queue"), - UploadQueue::Stopped(stopped) => { - // Special care is needed for deletions, if it was an earlier deletion (not scheduled from deletion) - // then stop() took care of it so we just return. - // For deletions that come from delete_all we still want to maintain metrics, launch following tasks, etc. - match &task.op { - UploadOp::Delete(delete) if delete.scheduled_from_timeline_delete => Some(&mut stopped.upload_queue_for_deletion), - _ => None - } - }, + UploadQueue::Stopped(_) => { None } UploadQueue::Initialized(qi) => { Some(qi) } }; @@ -1166,9 +1134,6 @@ impl RemoteTimelineClient { upload_queue.num_inprogress_metadata_uploads -= 1; upload_queue.last_uploaded_consistent_lsn = lsn; // XXX monotonicity check? } - UploadOp::Delete(_) => { - upload_queue.num_inprogress_deletions -= 1; - } UploadOp::Barrier(_) => unreachable!(), }; @@ -1200,13 +1165,6 @@ impl RemoteTimelineClient { reason: "metadata uploads are tiny", }, ), - UploadOp::Delete(delete) => ( - delete.file_kind, - RemoteOpKind::Delete, - DontTrackSize { - reason: "should we track deletes? positive or negative sign?", - }, - ), UploadOp::Barrier(_) => { // we do not account these return None; diff --git a/pageserver/src/tenant/remote_timeline_client/delete.rs b/pageserver/src/tenant/remote_timeline_client/delete.rs deleted file mode 100644 index 3f505d45ab..0000000000 --- a/pageserver/src/tenant/remote_timeline_client/delete.rs +++ /dev/null @@ -1,29 +0,0 @@ -//! Helper functions to delete files from remote storage with a RemoteStorage -use anyhow::Context; -use std::path::Path; -use tracing::debug; - -use remote_storage::GenericRemoteStorage; - -use crate::config::PageServerConf; - -pub(super) async fn delete_layer<'a>( - conf: &'static PageServerConf, - storage: &'a GenericRemoteStorage, - local_layer_path: &'a Path, -) -> anyhow::Result<()> { - fail::fail_point!("before-delete-layer", |_| { - anyhow::bail!("failpoint before-delete-layer") - }); - debug!("Deleting layer from remote storage: {local_layer_path:?}",); - - let path_to_delete = conf.remote_path(local_layer_path)?; - - // We don't want to print an error if the delete failed if the file has - // already been deleted. Thankfully, in this situation S3 already - // does not yield an error. While OS-provided local file system APIs do yield - // errors, we avoid them in the `LocalFs` wrapper. - storage.delete(&path_to_delete).await.with_context(|| { - format!("Failed to delete remote layer from storage at {path_to_delete:?}") - }) -} diff --git a/pageserver/src/tenant/upload_queue.rs b/pageserver/src/tenant/upload_queue.rs index a62cc99adf..0208992170 100644 --- a/pageserver/src/tenant/upload_queue.rs +++ b/pageserver/src/tenant/upload_queue.rs @@ -1,5 +1,3 @@ -use crate::metrics::RemoteOpFileKind; - use super::storage_layer::LayerFileName; use crate::tenant::metadata::TimelineMetadata; use crate::tenant::remote_timeline_client::index::IndexPart; @@ -212,13 +210,6 @@ pub(crate) struct UploadTask { pub(crate) op: UploadOp, } -#[derive(Debug)] -pub(crate) struct Delete { - pub(crate) file_kind: RemoteOpFileKind, - pub(crate) layer_file_name: LayerFileName, - pub(crate) scheduled_from_timeline_delete: bool, -} - #[derive(Debug)] pub(crate) enum UploadOp { /// Upload a layer file @@ -227,9 +218,6 @@ pub(crate) enum UploadOp { /// Upload the metadata file UploadMetadata(IndexPart, Lsn), - /// Delete a layer file - Delete(Delete), - /// Barrier. When the barrier operation is reached, Barrier(tokio::sync::watch::Sender<()>), } @@ -246,12 +234,6 @@ impl std::fmt::Display for UploadOp { ) } UploadOp::UploadMetadata(_, lsn) => write!(f, "UploadMetadata(lsn: {})", lsn), - UploadOp::Delete(delete) => write!( - f, - "Delete(path: {}, scheduled_from_timeline_delete: {})", - delete.layer_file_name.file_name(), - delete.scheduled_from_timeline_delete - ), UploadOp::Barrier(_) => write!(f, "Barrier"), } }