diff --git a/pageserver/src/layered_repository.rs b/pageserver/src/layered_repository.rs index 3124cb3488..ba9ac2237d 100644 --- a/pageserver/src/layered_repository.rs +++ b/pageserver/src/layered_repository.rs @@ -1737,8 +1737,8 @@ impl LayeredTimeline { // We landed on the same layer again. Shouldn't happen, but if it does, // don't get stuck in an infinite loop. bail!( - "could not find predecessor layer of segment {} at {}", - seg.rel, + "could not find predecessor of layer {} at {}, layer returned its own LSN", + layer_ref.filename().display(), cont_lsn ); } @@ -1748,8 +1748,8 @@ impl LayeredTimeline { continue; } else { bail!( - "could not find predecessor layer of segment {} at {}", - seg.rel, + "could not find predecessor of layer {} at {}", + layer_ref.filename().display(), cont_lsn ); } diff --git a/pageserver/src/layered_repository/delta_layer.rs b/pageserver/src/layered_repository/delta_layer.rs index f335ddfbdb..7538d22571 100644 --- a/pageserver/src/layered_repository/delta_layer.rs +++ b/pageserver/src/layered_repository/delta_layer.rs @@ -41,7 +41,7 @@ use crate::layered_repository::blob::BlobWriter; use crate::layered_repository::filename::{DeltaFileName, PathOrConf}; use crate::layered_repository::page_versions::PageVersions; use crate::layered_repository::storage_layer::{ - Layer, PageReconstructData, PageReconstructResult, PageVersion, SegmentTag, + Layer, PageReconstructData, PageReconstructResult, PageVersion, SegmentTag, RELISH_SEG_SIZE, }; use crate::virtual_file::VirtualFile; use crate::waldecoder; @@ -150,6 +150,19 @@ pub struct DeltaLayerInner { relsizes: VecMap, } +impl DeltaLayerInner { + fn get_seg_size(&self, lsn: Lsn) -> Result { + let slice = self + .relsizes + .slice_range((Included(&Lsn(0)), Included(&lsn))); + if let Some((_entry_lsn, entry)) = slice.last() { + Ok(*entry) + } else { + Err(anyhow::anyhow!("could not find seg size in delta layer")) + } + } +} + impl Layer for DeltaLayer { fn get_tenant_id(&self) -> ZTenantId { self.tenantid @@ -244,6 +257,15 @@ impl Layer for DeltaLayer { } } + // If we didn't find any records for this, check if the request is beyond EOF + if need_image + && reconstruct_data.records.is_empty() + && self.seg.rel.is_blocky() + && blknum - self.seg.segno * RELISH_SEG_SIZE >= inner.get_seg_size(lsn)? + { + return Ok(PageReconstructResult::Missing(self.start_lsn)); + } + // release metadata lock and close the file } @@ -266,15 +288,7 @@ impl Layer for DeltaLayer { // Scan the BTreeMap backwards, starting from the given entry. let inner = self.load()?; - let slice = inner - .relsizes - .slice_range((Included(&Lsn(0)), Included(&lsn))); - - if let Some((_entry_lsn, entry)) = slice.last() { - Ok(*entry) - } else { - Err(anyhow::anyhow!("could not find seg size in delta layer")) - } + inner.get_seg_size(lsn) } /// Does this segment exist at given LSN? diff --git a/pageserver/src/layered_repository/image_layer.rs b/pageserver/src/layered_repository/image_layer.rs index b56dfd17d5..22cf3347d8 100644 --- a/pageserver/src/layered_repository/image_layer.rs +++ b/pageserver/src/layered_repository/image_layer.rs @@ -162,6 +162,7 @@ impl Layer for ImageLayer { let buf = match &inner.image_type { ImageType::Blocky { num_blocks } => { + // Check if the request is beyond EOF if base_blknum >= *num_blocks { return Ok(PageReconstructResult::Missing(lsn)); } diff --git a/pageserver/src/layered_repository/inmemory_layer.rs b/pageserver/src/layered_repository/inmemory_layer.rs index 3efe62666f..dd02c8f951 100644 --- a/pageserver/src/layered_repository/inmemory_layer.rs +++ b/pageserver/src/layered_repository/inmemory_layer.rs @@ -193,6 +193,16 @@ impl Layer for InMemoryLayer { } } } + + // If we didn't find any records for this, check if the request is beyond EOF + if need_image + && reconstruct_data.records.is_empty() + && self.seg.rel.is_blocky() + && blknum - self.seg.segno * RELISH_SEG_SIZE >= self.get_seg_size(lsn)? + { + return Ok(PageReconstructResult::Missing(self.start_lsn)); + } + // release lock on 'inner' } diff --git a/pageserver/src/repository.rs b/pageserver/src/repository.rs index 14047661d1..a253608c27 100644 --- a/pageserver/src/repository.rs +++ b/pageserver/src/repository.rs @@ -948,6 +948,55 @@ mod tests { Ok(()) } + #[test] + fn test_read_beyond_eof() -> Result<()> { + let harness = RepoHarness::create("test_read_beyond_eof")?; + let repo = harness.load(); + let tline = repo.create_empty_timeline(TIMELINE_ID, Lsn(0))?; + + make_some_layers(&tline, Lsn(0x20))?; + { + let writer = tline.writer(); + writer.put_page_image( + TESTREL_A, + 0, + Lsn(0x60), + TEST_IMG(&format!("foo blk 0 at {}", Lsn(0x50))), + )?; + writer.advance_last_record_lsn(Lsn(0x60)); + } + + // Test read before rel creation. Should error out. + assert!(tline.get_page_at_lsn(TESTREL_A, 1, Lsn(0x10)).is_err()); + + // Read block beyond end of relation at different points in time. + // These reads should fall into different delta, image, and in-memory layers. + assert_eq!(tline.get_page_at_lsn(TESTREL_A, 1, Lsn(0x20))?, ZERO_PAGE); + assert_eq!(tline.get_page_at_lsn(TESTREL_A, 1, Lsn(0x25))?, ZERO_PAGE); + assert_eq!(tline.get_page_at_lsn(TESTREL_A, 1, Lsn(0x30))?, ZERO_PAGE); + assert_eq!(tline.get_page_at_lsn(TESTREL_A, 1, Lsn(0x35))?, ZERO_PAGE); + assert_eq!(tline.get_page_at_lsn(TESTREL_A, 1, Lsn(0x40))?, ZERO_PAGE); + assert_eq!(tline.get_page_at_lsn(TESTREL_A, 1, Lsn(0x45))?, ZERO_PAGE); + assert_eq!(tline.get_page_at_lsn(TESTREL_A, 1, Lsn(0x50))?, ZERO_PAGE); + assert_eq!(tline.get_page_at_lsn(TESTREL_A, 1, Lsn(0x55))?, ZERO_PAGE); + assert_eq!(tline.get_page_at_lsn(TESTREL_A, 1, Lsn(0x60))?, ZERO_PAGE); + + // Test on an in-memory layer with no preceding layer + { + let writer = tline.writer(); + writer.put_page_image( + TESTREL_B, + 0, + Lsn(0x70), + TEST_IMG(&format!("foo blk 0 at {}", Lsn(0x70))), + )?; + writer.advance_last_record_lsn(Lsn(0x70)); + } + assert_eq!(tline.get_page_at_lsn(TESTREL_B, 1, Lsn(0x70))?, ZERO_PAGE); + + Ok(()) + } + #[test] fn corrupt_metadata() -> Result<()> { const TEST_NAME: &str = "corrupt_metadata";