From 018a6069875658e57c132f10df98232f415fcba3 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Tue, 7 Sep 2021 17:49:29 +0300 Subject: [PATCH] Refactor code in LayerMap, for readability - Reorder the structs and functions - Delegate many of the operations in LayerMap to SegEntry. For example, `LayerMap::insert_open` now looks up the right SegEntry struct, and then calls `SegEntry::insert_open` on it. - Use HashMap::entry() function with or_default() to implement the lookups with less code --- .../src/layered_repository/layer_map.rs | 269 +++++++++--------- 1 file changed, 134 insertions(+), 135 deletions(-) diff --git a/pageserver/src/layered_repository/layer_map.rs b/pageserver/src/layered_repository/layer_map.rs index 706b8e9499..84c114c4a1 100644 --- a/pageserver/src/layered_repository/layer_map.rs +++ b/pageserver/src/layered_repository/layer_map.rs @@ -14,7 +14,6 @@ use crate::layered_repository::InMemoryLayer; use crate::relish::*; use anyhow::Result; use lazy_static::lazy_static; -use log::*; use std::cmp::Ordering; use std::collections::HashSet; use std::collections::{BTreeMap, BinaryHeap, HashMap}; @@ -49,49 +48,15 @@ pub struct LayerMap { current_generation: u64, } -/// -/// Per-segment entry in the LayerMap.segs hash map -/// -/// The last layer that is open for writes is always an InMemoryLayer, -/// and is kept in a separate field, because there can be only one for -/// each segment. The older layers, stored on disk, are kept in a -/// BTreeMap keyed by the layer's start LSN. -struct SegEntry { - pub open: Option>, - pub historic: BTreeMap>, -} - -/// Entry held LayerMap.open_layers, with boilerplate comparison routines -/// to implement a min-heap ordered by 'oldest_pending_lsn' and 'generation' -/// -/// Each entry also carries a generation number. It can be used to distinguish -/// entries with the same 'oldest_pending_lsn'. -struct OpenLayerEntry { - pub oldest_pending_lsn: Lsn, // copy of layer.get_oldest_pending_lsn() - pub generation: u64, - pub layer: Arc, -} -impl Ord for OpenLayerEntry { - fn cmp(&self, other: &Self) -> Ordering { - // BinaryHeap is a max-heap, and we want a min-heap. Reverse the ordering here - // to get that. Entries with identical oldest_pending_lsn are ordered by generation - other - .oldest_pending_lsn - .cmp(&self.oldest_pending_lsn) - .then_with(|| other.generation.cmp(&self.generation)) +impl Default for LayerMap { + fn default() -> Self { + LayerMap { + segs: HashMap::new(), + open_layers: BinaryHeap::new(), + current_generation: 0, + } } } -impl PartialOrd for OpenLayerEntry { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } -} -impl PartialEq for OpenLayerEntry { - fn eq(&self, other: &Self) -> bool { - self.cmp(&other) == Ordering::Equal - } -} -impl Eq for OpenLayerEntry {} impl LayerMap { /// @@ -103,23 +68,7 @@ impl LayerMap { pub fn get(&self, tag: &SegmentTag, lsn: Lsn) -> Option> { let segentry = self.segs.get(tag)?; - if let Some(open) = &segentry.open { - if open.get_start_lsn() <= lsn { - let x: Arc = Arc::clone(open) as _; - return Some(x); - } - } - - if let Some((_k, v)) = segentry - .historic - .range((Included(Lsn(0)), Included(lsn))) - .next_back() - { - let x: Arc = Arc::clone(v) as _; - Some(x) - } else { - None - } + segentry.get(lsn) } /// @@ -128,6 +77,7 @@ impl LayerMap { /// pub fn get_open(&self, tag: &SegmentTag) -> Option> { let segentry = self.segs.get(tag)?; + segentry.open.as_ref().map(Arc::clone) } @@ -135,21 +85,11 @@ impl LayerMap { /// Insert an open in-memory layer /// pub fn insert_open(&mut self, layer: Arc) { - let tag = layer.get_seg_tag(); + let segentry = self.segs.entry(layer.get_seg_tag()).or_default(); - if let Some(segentry) = self.segs.get_mut(&tag) { - if let Some(_old) = &segentry.open { - // FIXME: shouldn't exist, but check - } - segentry.open = Some(Arc::clone(&layer)); - } else { - let segentry = SegEntry { - open: Some(Arc::clone(&layer)), - historic: BTreeMap::new(), - }; - self.segs.insert(tag, segentry); - } + segentry.insert_open(Arc::clone(&layer)); + // Also add it to the binary heap let open_layer_entry = OpenLayerEntry { oldest_pending_lsn: layer.get_oldest_pending_lsn(), layer, @@ -162,11 +102,18 @@ impl LayerMap { /// Remove the oldest in-memory layer pub fn pop_oldest_open(&mut self) { + // Pop it from the binary heap let oldest_entry = self.open_layers.pop().unwrap(); let segtag = oldest_entry.layer.get_seg_tag(); + // Also remove it from the SegEntry of this segment let mut segentry = self.segs.get_mut(&segtag).unwrap(); + assert!(Arc::ptr_eq( + &segentry.open.as_ref().unwrap(), + &oldest_entry.layer + )); segentry.open = None; + NUM_INMEMORY_LAYERS.dec(); } @@ -174,21 +121,9 @@ impl LayerMap { /// Insert an on-disk layer /// pub fn insert_historic(&mut self, layer: Arc) { - let tag = layer.get_seg_tag(); - let start_lsn = layer.get_start_lsn(); + let segentry = self.segs.entry(layer.get_seg_tag()).or_default(); + segentry.insert_historic(layer); - if let Some(segentry) = self.segs.get_mut(&tag) { - segentry.historic.insert(start_lsn, layer); - } else { - let mut historic = BTreeMap::new(); - historic.insert(start_lsn, layer); - - let segentry = SegEntry { - open: None, - historic, - }; - self.segs.insert(tag, segentry); - } NUM_ONDISK_LAYERS.inc(); } @@ -207,7 +142,7 @@ impl LayerMap { NUM_ONDISK_LAYERS.dec(); } - // List relations that exist at the lsn + /// List relations that exist at the lsn pub fn list_rels(&self, spcnode: u32, dbnode: u32, lsn: Lsn) -> Result> { let mut rels: HashSet = HashSet::new(); @@ -217,16 +152,10 @@ impl LayerMap { && (dbnode == 0 || reltag.dbnode == dbnode) { // Add only if it exists at the requested LSN. - if let Some(open) = &segentry.open { - if open.get_end_lsn() > lsn { + if let Some(layer) = segentry.get(lsn) { + if !layer.is_dropped() || layer.get_end_lsn() > lsn { rels.insert(reltag); } - } else if let Some((_k, _v)) = segentry - .historic - .range((Included(Lsn(0)), Included(lsn))) - .next_back() - { - rels.insert(reltag); } } } @@ -234,7 +163,7 @@ impl LayerMap { Ok(rels) } - // List non-relation relishes that exist at the lsn + /// List non-relation relishes that exist at the lsn pub fn list_nonrels(&self, lsn: Lsn) -> Result> { let mut rels: HashSet = HashSet::new(); @@ -243,16 +172,10 @@ impl LayerMap { if let RelishTag::Relation(_) = seg.rel { } else { // Add only if it exists at the requested LSN. - if let Some(open) = &segentry.open { - if open.get_end_lsn() > lsn { + if let Some(layer) = segentry.get(lsn) { + if !layer.is_dropped() || layer.get_end_lsn() > lsn { rels.insert(seg.rel); } - } else if let Some((_k, _v)) = segentry - .historic - .range((Included(Lsn(0)), Included(lsn))) - .next_back() - { - rels.insert(seg.rel); } } } @@ -265,33 +188,10 @@ impl LayerMap { /// be deleted. pub fn newer_image_layer_exists(&self, seg: SegmentTag, lsn: Lsn) -> bool { if let Some(segentry) = self.segs.get(&seg) { - // We only check on-disk layers, because - // in-memory layers are not durable - for (newer_lsn, layer) in segentry - .historic - .range((Included(lsn), Included(Lsn(u64::MAX)))) - { - // Ignore layers that depend on an older layer. - if layer.is_incremental() { - continue; - } - if layer.get_end_lsn() > lsn { - trace!( - "found later layer for {}, {} {}-{}", - seg, - lsn, - newer_lsn, - layer.get_end_lsn() - ); - return true; - } else { - trace!("found singleton layer for {}, {} {}", seg, lsn, newer_lsn); - continue; - } - } + segentry.newer_image_layer_exists(lsn) + } else { + false } - trace!("no later layer found for {}, {}", seg, lsn); - false } /// Return the oldest in-memory layer, along with its generation number. @@ -336,16 +236,115 @@ impl LayerMap { } } -impl Default for LayerMap { +/// +/// Per-segment entry in the LayerMap::segs hash map. Holds all the layers +/// associated with the segment. +/// +/// The last layer that is open for writes is always an InMemoryLayer, +/// and is kept in a separate field, because there can be only one for +/// each segment. The older layers, stored on disk, are kept in a +/// BTreeMap keyed by the layer's start LSN. +struct SegEntry { + pub open: Option>, + pub historic: BTreeMap>, +} + +impl Default for SegEntry { fn default() -> Self { - LayerMap { - segs: HashMap::new(), - open_layers: BinaryHeap::new(), - current_generation: 0, + SegEntry { + open: None, + historic: BTreeMap::new(), } } } +impl SegEntry { + pub fn get(&self, lsn: Lsn) -> Option> { + if let Some(open) = &self.open { + if open.get_start_lsn() <= lsn { + let x: Arc = Arc::clone(open) as _; + return Some(x); + } + } + + if let Some((_start_lsn, layer)) = self + .historic + .range((Included(Lsn(0)), Included(lsn))) + .next_back() + { + Some(Arc::clone(layer)) + } else { + None + } + } + + pub fn newer_image_layer_exists(&self, lsn: Lsn) -> bool { + // We only check on-disk layers, because + // in-memory layers are not durable + + for (_newer_lsn, layer) in self + .historic + .range((Included(lsn), Included(Lsn(u64::MAX)))) + { + // Ignore incremental layers. + if layer.is_incremental() { + continue; + } + if layer.get_end_lsn() > lsn { + return true; + } else { + continue; + } + } + false + } + + pub fn insert_open(&mut self, layer: Arc) { + assert!(self.open.is_none()); + self.open = Some(layer); + } + + pub fn insert_historic(&mut self, layer: Arc) { + let start_lsn = layer.get_start_lsn(); + + self.historic.insert(start_lsn, layer); + } +} + +/// Entry held in LayerMap::open_layers, with boilerplate comparison routines +/// to implement a min-heap ordered by 'oldest_pending_lsn' and 'generation' +/// +/// The generation number associated with each entry can be used to distinguish +/// recently-added entries (i.e after last call to increment_generation()) from older +/// entries with the same 'oldest_pending_lsn'. +struct OpenLayerEntry { + pub oldest_pending_lsn: Lsn, // copy of layer.get_oldest_pending_lsn() + pub generation: u64, + pub layer: Arc, +} +impl Ord for OpenLayerEntry { + fn cmp(&self, other: &Self) -> Ordering { + // BinaryHeap is a max-heap, and we want a min-heap. Reverse the ordering here + // to get that. Entries with identical oldest_pending_lsn are ordered by generation + other + .oldest_pending_lsn + .cmp(&self.oldest_pending_lsn) + .then_with(|| other.generation.cmp(&self.generation)) + } +} +impl PartialOrd for OpenLayerEntry { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} +impl PartialEq for OpenLayerEntry { + fn eq(&self, other: &Self) -> bool { + self.cmp(&other) == Ordering::Equal + } +} +impl Eq for OpenLayerEntry {} + +/// Iterator returned by LayerMap::iter_historic_layers() pub struct HistoricLayerIter<'a> { segiter: std::collections::hash_map::Iter<'a, SegmentTag, SegEntry>, iter: Option>>,