From 225cabd84d51e57dc10bcd3867bc76b8525d6ff6 Mon Sep 17 00:00:00 2001 From: Erik Grinaker Date: Tue, 1 Apr 2025 13:38:12 +0200 Subject: [PATCH] pageserver: update upload queue TODOs (#11377) Update some upload queue TODOs, particularly to track https://github.com/neondatabase/neon/issues/10283, which I won't get around to. --- pageserver/src/tenant/remote_timeline_client.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pageserver/src/tenant/remote_timeline_client.rs b/pageserver/src/tenant/remote_timeline_client.rs index 32c0571b97..579dbeb322 100644 --- a/pageserver/src/tenant/remote_timeline_client.rs +++ b/pageserver/src/tenant/remote_timeline_client.rs @@ -1968,9 +1968,7 @@ impl RemoteTimelineClient { /// Pick next tasks from the queue, and start as many of them as possible without violating /// the ordering constraints. /// - /// TODO: consider limiting the number of in-progress tasks, beyond what remote_storage does. - /// This can launch an unbounded number of queued tasks. `UploadQueue::next_ready()` also has - /// worst-case quadratic cost in the number of tasks, and may struggle beyond 10,000 tasks. + /// The number of inprogress tasks is limited by `Self::inprogress_tasks`, see `next_ready`. fn launch_queued_tasks(self: &Arc, upload_queue: &mut UploadQueueInitialized) { while let Some((mut next_op, coalesced_ops)) = upload_queue.next_ready() { debug!("starting op: {next_op}"); @@ -2218,6 +2216,11 @@ impl RemoteTimelineClient { } res } + // TODO: this should wait for the deletion to be executed by the deletion queue. + // Otherwise, the deletion may race with an upload and wrongfully delete a newer + // file. Some of the above logic attempts to work around this, it should be replaced + // by the upload queue ordering guarantees (see `can_bypass`). See: + // . UploadOp::Delete(delete) => { if self.config.read().unwrap().block_deletions { let mut queue_locked = self.upload_queue.lock().unwrap();