diff --git a/pageserver/src/tenant/bst_layer_map.rs b/pageserver/src/tenant/bst_layer_map.rs index 0ed62c4ec0..fbc79148f7 100644 --- a/pageserver/src/tenant/bst_layer_map.rs +++ b/pageserver/src/tenant/bst_layer_map.rs @@ -15,6 +15,11 @@ pub struct PersistentLayerMap { /// Mapping key to the latest layer (if any) until the next key. /// We use the Sync version of the map because we want Self to /// be Sync. + /// + /// TODO Separate Head into its own struct LatestLayerMap + /// TODO Merge historic with retroactive, into HistoricLayerMap + /// TODO Maintain a pair of heads, one for images, one for deltas. + /// This way we can query both of them with one BTreeMap query. head: RedBlackTreeMapSync>, /// All previous states of `self.head` @@ -275,24 +280,8 @@ fn test_persistent_overlapping() { /// /// Layer type is abstracted as Value to make unit testing easier. pub struct RetroactiveLayerMap { - /// Using Arc and Vec allows us to hack around the lack of retroactive - /// insert/delete functionality in PersistentLayerMap: - /// - For normal append-only updates, we insert Arc::new(vec![value]). - /// - For retroactive deletion (during gc) we empty the vector. The use - /// of Arc gives us a useful indirection layer so that the delete would - /// effectively retroactively update future versions, instead of creating - /// a new branch. - /// - For retroactive updates (during compaction), we find all layers below - /// the layer we're inserting, and append to their Vec-s. This is O(N), but - /// also amortized O(log N). Here's why: We don't insert image layers - /// retroactively, only deltas. And after an image gets covered by K (currently - /// K = 3) deltas, we do compaction. - /// - /// This complexity might be a limitation, or a feature. Here's how it might - /// actually help: It gives us the option to store the entire reconstruction - /// result in a single colocated Vec, and get the initial image and all necessary - /// deltas in one query. - map: PersistentLayerMap>>, + /// A persistent layer map that we rebuild when we need to retroactively update + map: PersistentLayerMap, /// We buffer insertion into the PersistentLayerMap to decrease the number of rebuilds. /// A value of None means we want to delete this item. @@ -317,7 +306,7 @@ impl Default for RetroactiveLayerMap { impl RetroactiveLayerMap { pub fn new() -> Self { Self { - map: PersistentLayerMap::>>::new(), + map: PersistentLayerMap::::new(), buffer: BTreeMap::new(), layers: BTreeMap::new(), } @@ -366,9 +355,8 @@ impl RetroactiveLayerMap { for ((lsn_start, lsn_end, key_start, key_end), layer) in self.layers.range((rebuild_since, 0, 0, 0)..) { - let wrapped = Arc::new(vec![layer.clone()]); self.map - .insert(*key_start..*key_end, *lsn_start..*lsn_end, wrapped); + .insert(*key_start..*key_end, *lsn_start..*lsn_end, layer.clone()); } } @@ -382,11 +370,7 @@ impl RetroactiveLayerMap { } match self.map.query(key, lsn) { - Some(vec) => match vec.len().cmp(&1) { - std::cmp::Ordering::Less => todo!(), - std::cmp::Ordering::Equal => Some(vec[0].clone()), - std::cmp::Ordering::Greater => todo!(), - }, + Some(layer) => Some(layer.clone()), None => None, } } diff --git a/pageserver/src/tenant/layer_map.rs b/pageserver/src/tenant/layer_map.rs index 7fd65d2215..14b6bdb263 100644 --- a/pageserver/src/tenant/layer_map.rs +++ b/pageserver/src/tenant/layer_map.rs @@ -248,19 +248,19 @@ impl LayerMap { /// layer. /// pub fn search(&self, key: Key, end_lsn: Lsn) -> Result> { - let old = self.search_old(key, end_lsn)?; + // let old = self.search_old(key, end_lsn)?; let new = self.search_new(key, end_lsn)?; - match (&old, &new) { - (None, None) => {} - (None, Some(_)) => panic!("returned Some, expected None"), - (Some(_), None) => panic!("returned None, expected Some"), - (Some(old), Some(new)) => { - // TODO be more verbose and flexible - let context = format!("query: key {}, end_lsn: {}", key, end_lsn); - assert_eq!(old.layer.filename(), new.layer.filename(), "{}", context); - assert_eq!(old.lsn_floor, new.lsn_floor, "{}", context); - } - } + // match (&old, &new) { + // (None, None) => {} + // (None, Some(_)) => panic!("returned Some, expected None"), + // (Some(_), None) => panic!("returned None, expected Some"), + // (Some(old), Some(new)) => { + // // TODO be more verbose and flexible + // let context = format!("query: key {}, end_lsn: {}", key, end_lsn); + // assert_eq!(old.layer.filename(), new.layer.filename(), "{}", context); + // assert_eq!(old.lsn_floor, new.lsn_floor, "{}", context); + // } + // } return Ok(new); }