From 008d2a22d9180753da7fbae07f645b56ce23592f Mon Sep 17 00:00:00 2001 From: Christian Schwarz Date: Thu, 2 Mar 2023 09:57:56 +0100 Subject: [PATCH] compact_level0_phase1: hold layer map lock while layer iters are alive The iters hold onto page cache pages, via their internal BlockCursor. If we acquire these page cache locks without holding the layer map lock, another thread can come, acquire the the layer map lock, and try to lock the same cache pages. Deadlock. Let's see whether the deadlock from #3712 still reproduces with this fix. refs #3712 --- pageserver/src/tenant/timeline.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pageserver/src/tenant/timeline.rs b/pageserver/src/tenant/timeline.rs index d46ac26e7d..0b00f72591 100644 --- a/pageserver/src/tenant/timeline.rs +++ b/pageserver/src/tenant/timeline.rs @@ -2843,7 +2843,6 @@ impl Timeline { ) -> Result { let layers = self.layers.read().unwrap(); let mut level0_deltas = layers.get_level0_deltas()?; - drop(layers); // Only compact if enough layers have accumulated. let threshold = self.get_compaction_threshold(); @@ -2964,7 +2963,6 @@ impl Timeline { // Determine N largest holes where N is number of compacted layers. let max_holes = deltas_to_compact.len(); let last_record_lsn = self.get_last_record_lsn(); - let layers = self.layers.read().unwrap(); // Is'n it better to hold original layers lock till here? let min_hole_range = (target_file_size / page_cache::PAGE_SZ as u64) as i128; let min_hole_coverage_size = 3; // TODO: something more flexible? @@ -2997,7 +2995,6 @@ impl Timeline { } prev = Some(next_key.next()); } - drop(layers); let mut holes = heap.into_vec(); holes.sort_unstable_by_key(|hole| hole.key_range.start); let mut next_hole = 0; // index of next hole in holes vector @@ -3163,7 +3160,7 @@ impl Timeline { } drop(all_keys_iter); // So that deltas_to_compact is no longer borrowed - + drop(layers); Ok(CompactLevel0Phase1Result { new_layers, deltas_to_compact,