From 674807eee1e3cb4c88e94f1a2d717a69f741c16d Mon Sep 17 00:00:00 2001 From: anastasia Date: Sun, 5 Sep 2021 19:21:24 +0300 Subject: [PATCH] Add test for dropped reltaions. Fix list_rels() and list_nonrels() functions --- pageserver/src/layered_repository.rs | 68 +++++++++++++++++-- .../src/layered_repository/inmemory_layer.rs | 5 ++ .../src/layered_repository/layer_map.rs | 47 +++++++++---- pageserver/src/repository.rs | 48 +++++++++++++ 4 files changed, 147 insertions(+), 21 deletions(-) diff --git a/pageserver/src/layered_repository.rs b/pageserver/src/layered_repository.rs index 4eda43553b..e2c41d5eb5 100644 --- a/pageserver/src/layered_repository.rs +++ b/pageserver/src/layered_repository.rs @@ -649,16 +649,35 @@ impl Timeline for LayeredTimeline { trace!("list_rels called at {}", lsn); // List all rels in this timeline, and all its ancestors. - let mut all_rels = HashSet::new(); + let mut all_rels_map: HashMap = HashMap::new(); + let mut result = HashSet::new(); let mut timeline = self; + loop { + timeline.layers.lock().unwrap().dump()?; + let rels = timeline .layers .lock() .unwrap() .list_rels(spcnode, dbnode, lsn)?; - all_rels.extend(rels.iter()); + for (&new_rel, &new_rel_exists) in rels.iter() { + if let Some(rel_exists) = all_rels_map.get(&new_rel) { + trace!( + "list_rels() Newer version of the object {} is already found: exists {}", + new_rel, + rel_exists + ); + } else { + all_rels_map.insert(new_rel, new_rel_exists); + trace!( + "list_rels() Newer version of the object {} NOT found: exists {}", + new_rel, + new_rel_exists + ); + } + } if let Some(ancestor) = timeline.ancestor_timeline.as_ref() { timeline = ancestor; @@ -668,7 +687,17 @@ impl Timeline for LayeredTimeline { } } - Ok(all_rels) + // Filter out dropped relations + for (&new_rel, &new_rel_exists) in all_rels_map.iter() { + if new_rel_exists { + result.insert(new_rel); + trace!("list_rels() List object {}", new_rel); + } else { + trace!("list_rels() Filter out droped object {}", new_rel); + } + } + + Ok(result) } fn list_nonrels(&self, lsn: Lsn) -> Result> { @@ -677,13 +706,28 @@ impl Timeline for LayeredTimeline { let lsn = self.wait_lsn(lsn)?; // List all nonrels in this timeline, and all its ancestors. - let mut all_rels = HashSet::new(); + let mut all_rels_map: HashMap = HashMap::new(); + let mut result = HashSet::new(); let mut timeline = self; loop { let rels = timeline.layers.lock().unwrap().list_nonrels(lsn)?; - all_rels.extend(rels.iter()); - + for (&new_rel, &new_rel_exists) in rels.iter() { + if let Some(rel_exists) = all_rels_map.get(&new_rel) { + trace!( + "list_nonrels() Newer version of the object {} is already found: exists {}", + new_rel, + rel_exists + ); + } else { + all_rels_map.insert(new_rel, new_rel_exists); + trace!( + "list_nonrels() Newer version of the object {} NOT found: exists {}", + new_rel, + new_rel_exists + ); + } + } if let Some(ancestor) = timeline.ancestor_timeline.as_ref() { timeline = ancestor; continue; @@ -692,7 +736,17 @@ impl Timeline for LayeredTimeline { } } - Ok(all_rels) + // Filter out dropped relations + for (&new_rel, &new_rel_exists) in all_rels_map.iter() { + if new_rel_exists { + result.insert(new_rel); + trace!("list_nonrels() List object {}", new_rel); + } else { + trace!("list_nonrels() Filter out droped object {}", new_rel); + } + } + + Ok(result) } fn put_wal_record(&self, rel: RelishTag, blknum: u32, rec: WALRecord) -> Result<()> { diff --git a/pageserver/src/layered_repository/inmemory_layer.rs b/pageserver/src/layered_repository/inmemory_layer.rs index 0514ac9236..4a422f242a 100644 --- a/pageserver/src/layered_repository/inmemory_layer.rs +++ b/pageserver/src/layered_repository/inmemory_layer.rs @@ -199,6 +199,11 @@ impl Layer for InMemoryLayer { fn get_seg_exists(&self, lsn: Lsn) -> Result { let inner = self.inner.lock().unwrap(); + // If the segment created after requested LSN, + // it doesn't exist in the layer. But we shouldn't + // have requested it in the first place. + assert!(lsn >= self.start_lsn); + // Is the requested LSN after the segment was dropped? if let Some(drop_lsn) = inner.drop_lsn { if lsn >= drop_lsn { diff --git a/pageserver/src/layered_repository/layer_map.rs b/pageserver/src/layered_repository/layer_map.rs index 49a216e8b5..f5706606e5 100644 --- a/pageserver/src/layered_repository/layer_map.rs +++ b/pageserver/src/layered_repository/layer_map.rs @@ -14,9 +14,10 @@ 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}; +use std::ops::Bound::Included; use std::sync::Arc; use zenith_metrics::{register_int_gauge, IntGauge}; use zenith_utils::lsn::Lsn; @@ -141,20 +142,29 @@ impl LayerMap { NUM_ONDISK_LAYERS.dec(); } - /// 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(); + // List relations along with a flag that marks if they exist at the given lsn. + // spcnode 0 and dbnode 0 have special meanings and mean all tabespaces/databases. + pub fn list_rels(&self, spcnode: u32, dbnode: u32, lsn: Lsn) -> Result> { + let mut rels: HashMap = HashMap::new(); for (seg, segentry) in self.segs.iter() { if let RelishTag::Relation(reltag) = seg.rel { if (spcnode == 0 || reltag.spcnode == spcnode) && (dbnode == 0 || reltag.dbnode == dbnode) { - // Add only if it exists at the requested LSN. - if let Some(layer) = segentry.get(lsn) { - if !layer.is_dropped() || layer.get_end_lsn() > lsn { - rels.insert(reltag); + if let Some(layer) = &segentry.open { + if layer.get_start_lsn() <= lsn && lsn <= layer.get_end_lsn() { + let exists = layer.get_seg_exists(lsn)?; + trace!("Open layer list object {}: exists {}", reltag, exists); + rels.insert(reltag, exists); } + } else if let Some((_, layer)) = segentry + .historic + .range((Included(Lsn(0)), Included(lsn))) + .next_back() + { + let exists = layer.get_seg_exists(lsn)?; + rels.insert(reltag, exists); } } } @@ -162,19 +172,28 @@ impl LayerMap { Ok(rels) } - /// List non-relation relishes that exist at the lsn - pub fn list_nonrels(&self, lsn: Lsn) -> Result> { - let mut rels: HashSet = HashSet::new(); + // List non-relation relishes that exist at the lsn + pub fn list_nonrels(&self, lsn: Lsn) -> Result> { + let mut rels: HashMap = HashMap::new(); // Scan the timeline directory to get all rels in this timeline. for (seg, segentry) in self.segs.iter() { if let RelishTag::Relation(_) = seg.rel { } else { // Add only if it exists at the requested LSN. - if let Some(layer) = segentry.get(lsn) { - if !layer.is_dropped() || layer.get_end_lsn() > lsn { - rels.insert(seg.rel); + if let Some(layer) = &segentry.open { + if layer.get_start_lsn() <= lsn && lsn <= layer.get_end_lsn() { + let exists = layer.get_seg_exists(lsn)?; + trace!("Open layer list object {}: exists {}", seg.rel, exists); + rels.insert(seg.rel, exists); } + } else if let Some((_k, layer)) = segentry + .historic + .range((Included(Lsn(0)), Included(lsn))) + .next_back() + { + let exists = layer.get_seg_exists(lsn)?; + rels.insert(seg.rel, exists); } } } diff --git a/pageserver/src/repository.rs b/pageserver/src/repository.rs index 2ffda7d8e1..2357b910ea 100644 --- a/pageserver/src/repository.rs +++ b/pageserver/src/repository.rs @@ -463,6 +463,54 @@ mod tests { Ok(()) } + /// + /// Test list_rels() function, with branches and dropped relations + /// + #[test] + fn test_list_rels_drop() -> Result<()> { + let repo = get_test_repo("test_list_rels_drop")?; + let timelineid = ZTimelineId::from_str("11223344556677881122334455667788").unwrap(); + let tline = repo.create_empty_timeline(timelineid, Lsn(0x00))?; + const TESTDB: u32 = 111; + + // Import initial dummy checkpoint record, otherwise the get_timeline() call + // after branching fails below + tline.put_page_image(RelishTag::Checkpoint, 0, Lsn(0x10), ZERO_PAGE.clone())?; + + // Create a relation on the timeline + tline.put_page_image(TESTREL_A, 0, Lsn(0x20), TEST_IMG("foo blk 0 at 2"))?; + + // Check that list_rels() lists it after LSN 2, but no before it. + let reltag = match TESTREL_A { + RelishTag::Relation(reltag) => reltag, + _ => panic!("unexpected relish"), + }; + assert!(!tline.list_rels(0, TESTDB, Lsn(0x10))?.contains(&reltag)); + assert!(tline.list_rels(0, TESTDB, Lsn(0x20))?.contains(&reltag)); + assert!(tline.list_rels(0, TESTDB, Lsn(0x30))?.contains(&reltag)); + + // Create a branch, check that the relation is visible there + let newtimelineid = ZTimelineId::from_str("AA223344556677881122334455667788").unwrap(); + repo.branch_timeline(timelineid, newtimelineid, Lsn(0x30))?; + let newtline = repo.get_timeline(newtimelineid)?; + + assert!(newtline.list_rels(0, TESTDB, Lsn(0x30))?.contains(&reltag)); + + // Drop it on the branch + newtline.drop_relish(TESTREL_A, Lsn(0x40))?; + + // Check that it's no longer listed on the branch after the point where it was dropped + assert!(newtline.list_rels(0, TESTDB, Lsn(0x30))?.contains(&reltag)); + assert!(!newtline.list_rels(0, TESTDB, Lsn(0x40))?.contains(&reltag)); + + // Run checkpoint and garbage collection and check that it's still not visible + newtline.checkpoint()?; + repo.gc_iteration(Some(newtimelineid), 0, true)?; + assert!(!newtline.list_rels(0, TESTDB, Lsn(0x40))?.contains(&reltag)); + + Ok(()) + } + /// /// Test branch creation ///