From e096c62494a7fc370dde4aacd95d4645e513b1a1 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Wed, 9 Mar 2022 01:26:36 +0200 Subject: [PATCH] Misc fixes and stuff --- pageserver/src/keyspace.rs | 1 + pageserver/src/layered_repository.rs | 56 +++++++++++-------- .../src/layered_repository/layer_map.rs | 8 +-- .../src/layered_repository/storage_layer.rs | 2 +- pageserver/src/relish.rs | 35 +++++++++++- pageserver/src/repository.rs | 2 +- 6 files changed, 72 insertions(+), 32 deletions(-) diff --git a/pageserver/src/keyspace.rs b/pageserver/src/keyspace.rs index 4e1ebdd32a..0ad0c404ff 100644 --- a/pageserver/src/keyspace.rs +++ b/pageserver/src/keyspace.rs @@ -41,6 +41,7 @@ impl KeyPartitioning { if range.start == accum.end { accum.end = range.end; } else { + assert!(range.start > accum.end); self.ranges.push(accum.clone()); *accum = range; } diff --git a/pageserver/src/layered_repository.rs b/pageserver/src/layered_repository.rs index 35a864f2a6..a319563c33 100644 --- a/pageserver/src/layered_repository.rs +++ b/pageserver/src/layered_repository.rs @@ -1011,6 +1011,8 @@ impl LayeredTimeline { let mut timeline_owned; let mut timeline = self; + let mut path: Vec<(ValueReconstructResult, Lsn, Arc)> = Vec::new(); + // 'prev_lsn' tracks the last LSN that we were at in our search. It's used // to check that each iteration make some progress, to break infinite // looping if something goes wrong. @@ -1028,6 +1030,9 @@ impl LayeredTimeline { if prev_lsn <= cont_lsn { // Didn't make any progress in last iteration. Error out to avoid // getting stuck in the loop. + for (r, c, l) in path { + error!("PATH: result {:?}, cont_lsn {}, layer: {}", r, c, l.filename().display()); + } bail!("could not find layer with more data for key {} at LSN {}, request LSN {}, ancestor {}", key, Lsn(cont_lsn.0 - 1), @@ -1062,18 +1067,20 @@ impl LayeredTimeline { if let Some(open_layer) = &layers.open_layer { let start_lsn = open_layer.get_lsn_range().start; if cont_lsn > start_lsn { - //info!("CHECKING for {} at {} on open layer {}", reconstruct_state.key, reconstruct_state.lsn, open_layer.filename().display()); + //info!("CHECKING for {} at {} on open layer {}", key, cont_lsn, open_layer.filename().display()); result = open_layer.get_value_reconstruct_data(key, open_layer.get_lsn_range().start..cont_lsn, reconstruct_state)?; - cont_lsn = open_layer.get_lsn_range().start; + cont_lsn = start_lsn; + path.push((result, cont_lsn, open_layer.clone())); continue; } } if let Some(frozen_layer) = &layers.frozen_layer { let start_lsn = frozen_layer.get_lsn_range().start; if cont_lsn > start_lsn { - //info!("CHECKING for {} at {} on frozen layer {}", reconstruct_state.key, reconstruct_state.lsn, frozen_layer.filename().display()); + //info!("CHECKING for {} at {} on frozen layer {}", key, cont_lsn, frozen_layer.filename().display()); result = frozen_layer.get_value_reconstruct_data(key, frozen_layer.get_lsn_range().start..cont_lsn, reconstruct_state)?; - cont_lsn = frozen_layer.get_lsn_range().start; + cont_lsn = start_lsn; + path.push((result, cont_lsn, frozen_layer.clone())); continue; } } @@ -1081,10 +1088,11 @@ impl LayeredTimeline { if let Some(SearchResult { lsn_floor, layer }) = layers .search(key, cont_lsn)? { - //info!("CHECKING for {} at {} on historic layer {}", reconstruct_state.key, reconstruct_state.lsn, layer.filename().display()); + //info!("CHECKING for {} at {} on historic layer {}", key, cont_lsn, layer.filename().display()); result = layer.get_value_reconstruct_data(key, lsn_floor..cont_lsn, reconstruct_state)?; cont_lsn = lsn_floor; + path.push((result, cont_lsn, layer)); } else if self.ancestor_timeline.is_some() { // Nothing on this timeline. Traverse to parent result = ValueReconstructResult::Continue; @@ -1247,10 +1255,7 @@ impl LayeredTimeline { // TODO: the threshold for how often we create image layers is // currently hard-coded at 3. It means, write out a new image layer, // if there are at least three delta layers on top of it. - if false { - self.compact(TARGET_FILE_SIZE_BYTES as usize)?; - } - //self.compact_level0()?; + self.compact(TARGET_FILE_SIZE_BYTES as usize)?; } // TODO: We should also compact existing delta layers here. @@ -1595,9 +1600,6 @@ impl LayeredTimeline { fn gc(&self) -> Result { let now = Instant::now(); let mut result: GcResult = Default::default(); - if true { - return Ok(result); - } let disk_consistent_lsn = self.get_disk_consistent_lsn(); let _checkpoint_cs = self.checkpoint_cs.lock().unwrap(); @@ -2001,15 +2003,16 @@ mod tests { const NUM_KEYS: usize = 1000; - let mut lsn = Lsn(0x10); - let mut test_key = Key::from_hex("012222222233333333444444445500000000").unwrap(); - let mut blknum = 0; let mut parts = KeyPartitioning::new(); - for _ in 0..NUM_KEYS { - test_key.field6 = blknum; + let mut updated = [Lsn(0); NUM_KEYS]; + + let mut lsn = Lsn(0); + for blknum in 0..NUM_KEYS { + lsn = Lsn(lsn.0 + 0x10); + test_key.field6 = blknum as u32; let writer = tline.writer(); writer.put( test_key, @@ -2017,12 +2020,10 @@ mod tests { Value::Image(TEST_IMG(&format!("{} at {}", blknum, lsn))), )?; writer.advance_last_record_lsn(lsn); + updated[blknum] = lsn; drop(writer); parts.add_key(test_key); - - lsn = Lsn(lsn.0 + 0x10); - blknum += 1; } parts.repartition(TEST_FILE_SIZE as u64); @@ -2030,20 +2031,27 @@ mod tests { for _ in 0..50 { for _ in 0..NUM_KEYS { - blknum = thread_rng().gen_range(0..NUM_KEYS) as u32; - test_key.field6 = blknum; + lsn = Lsn(lsn.0 + 0x10); + let blknum = thread_rng().gen_range(0..NUM_KEYS); + test_key.field6 = blknum as u32; let writer = tline.writer(); writer.put( test_key, lsn, Value::Image(TEST_IMG(&format!("{} at {}", blknum, lsn))), )?; + println!("updating {} at {}", blknum, lsn); writer.advance_last_record_lsn(lsn); drop(writer); - - lsn = Lsn(lsn.0 + 0x10); + updated[blknum] = lsn; } + for blknum in 0..NUM_KEYS { + test_key.field6 = blknum as u32; + assert_eq!(tline.get(test_key, lsn)?, TEST_IMG(&format!("{} at {}", blknum, updated[blknum]))); + } + println!("checkpointing {}", lsn); + let cutoff = tline.get_last_record_lsn(); tline.update_gc_info(Vec::new(), cutoff); tline.checkpoint(CheckpointConfig::Forced)?; diff --git a/pageserver/src/layered_repository/layer_map.rs b/pageserver/src/layered_repository/layer_map.rs index 1ad6351539..4d2af960a6 100644 --- a/pageserver/src/layered_repository/layer_map.rs +++ b/pageserver/src/layered_repository/layer_map.rs @@ -141,11 +141,9 @@ impl LayerMap { key, end_lsn ); - let lsn_floor = if let Some(latest_img_lsn) = latest_img_lsn { - Lsn(latest_img_lsn.0 + 1) - } else { - l.get_lsn_range().start - }; + let lsn_floor = std::cmp::max( + Lsn(latest_img_lsn.unwrap_or(Lsn(0)).0 + 1), + l.get_lsn_range().start); Ok(Some(SearchResult { lsn_floor, layer: l, diff --git a/pageserver/src/layered_repository/storage_layer.rs b/pageserver/src/layered_repository/storage_layer.rs index 7ddaf7a581..76795e1bb0 100644 --- a/pageserver/src/layered_repository/storage_layer.rs +++ b/pageserver/src/layered_repository/storage_layer.rs @@ -56,7 +56,7 @@ pub struct ValueReconstructState { } /// Return value from Layer::get_page_reconstruct_data -#[derive(Debug)] +#[derive(Clone, Copy, Debug)] pub enum ValueReconstructResult { /// Got all the data needed to reconstruct the requested page Complete, diff --git a/pageserver/src/relish.rs b/pageserver/src/relish.rs index 7304c41a0e..f775ce933a 100644 --- a/pageserver/src/relish.rs +++ b/pageserver/src/relish.rs @@ -26,6 +26,7 @@ //! use serde::{Deserialize, Serialize}; +use std::cmp::Ordering; use std::fmt; use postgres_ffi::relfile_utils::forknumber_to_name; @@ -46,7 +47,10 @@ use postgres_ffi::Oid; /// are used for the same purpose. /// [See more related comments here](https:///github.com/postgres/postgres/blob/99c5852e20a0987eca1c38ba0c09329d4076b6a0/src/include/storage/relfilenode.h#L57). /// -#[derive(Debug, PartialEq, Eq, PartialOrd, Hash, Ord, Clone, Copy, Serialize, Deserialize)] +// FIXME: should move 'forknum' as last field to keep this consistent with Postgres. +// Then we could replace the custo Ord and PartialOrd implementations below with +// deriving them. +#[derive(Debug, PartialEq, Eq, Hash, Clone, Copy, Serialize, Deserialize)] pub struct RelTag { pub forknum: u8, pub spcnode: Oid, @@ -54,6 +58,35 @@ pub struct RelTag { pub relnode: Oid, } +impl PartialOrd for RelTag { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for RelTag { + fn cmp(&self, other: &Self) -> Ordering { + let mut cmp; + + cmp = self.spcnode.cmp(&other.spcnode); + if cmp != Ordering::Equal { + return cmp; + } + cmp = self.dbnode.cmp(&other.dbnode); + if cmp != Ordering::Equal { + return cmp; + } + cmp = self.relnode.cmp(&other.relnode); + if cmp != Ordering::Equal { + return cmp; + } + cmp = self.forknum.cmp(&other.forknum); + + cmp + } +} + + /// Display RelTag in the same format that's used in most PostgreSQL debug messages: /// /// //[_fsm|_vm|_init] diff --git a/pageserver/src/repository.rs b/pageserver/src/repository.rs index 1abf2c365a..d8f68aaa6c 100644 --- a/pageserver/src/repository.rs +++ b/pageserver/src/repository.rs @@ -427,7 +427,7 @@ pub mod repo_harness { pub fn TEST_IMG(s: &str) -> Bytes { let mut buf = BytesMut::new(); buf.extend_from_slice(s.as_bytes()); - buf.resize(8192, 0); + buf.resize(64, 0); buf.freeze() }