From d629e136fa96767132b28f3821f112eea7ebfacc Mon Sep 17 00:00:00 2001 From: Christian Schwarz Date: Thu, 11 May 2023 17:25:25 +0200 Subject: [PATCH] clippy-allow await while get_value_reconstruct_data calls + explainer --- pageserver/src/tenant/timeline.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/pageserver/src/tenant/timeline.rs b/pageserver/src/tenant/timeline.rs index 62b0cd4d44..a1734b9762 100644 --- a/pageserver/src/tenant/timeline.rs +++ b/pageserver/src/tenant/timeline.rs @@ -2143,6 +2143,24 @@ impl Timeline { /// /// This function takes the current timeline's locked LayerMap as an argument, /// so callers can avoid potential race conditions. + /// + // TODO: find a way to not hold the Timeline::layers lock during get_value_reconstruct_data calls. + // + // Since these calls do local disk IO, they'll be reasonably fast, until come disk IOPS bound. + // We have lots of headroom on current pageservers, so, it's going to be fine for now. + // + // We can't use tokio::sync::RwLock that easily because its guard is not Send, but, + // many tasks that access Timeline::layers run inside task_mgr tasks, which are required + // to be Send. It has been tried in origin/problame/asyncify-get-reconstruct-data--tokio-sync. + // + // The solution will probably be to have an immutable + multi-versioned layer map, allowing + // us to grab a snapshot of the layer map once and execute this function on the snapshot. + // + // Or, we could invest time to figure out whether we can drop the layer map lock after + // we grabbed the layer, do the IO, re-aquire, and continue the traversal. + // + // (Why is this allow() not inside the function? Because clippy doesn't respect it then). + #[allow(clippy::await_holding_lock)] async fn get_reconstruct_data( &self, key: Key,