Partial fix for issue with extending relation with a gap.

This should fix the sporadic regression test failures we've been seeing
lately with "no base img found" errors.

This fixes the common case, but one corner case is still not handled:
If a relation is extended across a segment boundary, leaving a gap block
in the segment preceding the segment containing the target block, the
preceding segment will not be padded with zeros correctly. This adds
a test case for that, but it's commented out.

See github issue https://github.com/zenithdb/zenith/issues/500
This commit is contained in:
Heikki Linnakangas
2021-09-02 22:01:46 +03:00
parent 7507f4b309
commit 1686715ad0
2 changed files with 62 additions and 0 deletions

View File

@@ -7,6 +7,7 @@ use crate::layered_repository::storage_layer::{
Layer, PageReconstructData, PageReconstructResult, PageVersion, SegmentTag, RELISH_SEG_SIZE,
};
use crate::layered_repository::LayeredTimeline;
use crate::layered_repository::ZERO_PAGE;
use crate::layered_repository::{DeltaLayer, ImageLayer};
use crate::repository::WALRecord;
use crate::PageServerConf;
@@ -364,6 +365,36 @@ impl InMemoryLayer {
newsize,
lsn
);
// If we are extending the relation by more than one page, initialize the "gap"
// with zeros
//
// XXX: What if the caller initializes the gap with subsequent call with same LSN?
// I don't think that can happen currently, but that is highly dependent on how
// PostgreSQL writes its WAL records and there's no guarantee of it. If it does
// happen, we would hit the "page version already exists" warning above on the
// subsequent call to initialize the gap page.
let gapstart = self.seg.segno * RELISH_SEG_SIZE + oldsize;
for gapblknum in gapstart..blknum {
let zeropv = PageVersion {
page_image: Some(ZERO_PAGE.clone()),
record: None,
};
println!(
"filling gap blk {} with zeros for write of {}",
gapblknum, blknum
);
let old = inner.page_versions.insert((gapblknum, lsn), zeropv);
// We already had an entry for this LSN. That's odd..
if old.is_some() {
warn!(
"Page version of rel {} blk {} at {} already exists",
self.seg.rel, blknum, lsn
);
}
}
inner.segsizes.insert(lsn, newsize);
}
}

View File

@@ -358,6 +358,37 @@ mod tests {
TEST_IMG("foo blk 2 at 5")
);
// Truncate to zero length
tline.put_truncation(TESTREL_A, Lsn(0x60), 0)?;
tline.advance_last_record_lsn(Lsn(0x60));
assert_eq!(tline.get_relish_size(TESTREL_A, Lsn(0x60))?.unwrap(), 0);
// Extend from 0 to 2 blocks, leaving a gap
tline.put_page_image(TESTREL_A, 1, Lsn(0x70), TEST_IMG("foo blk 1"))?;
tline.advance_last_record_lsn(Lsn(0x70));
assert_eq!(tline.get_relish_size(TESTREL_A, Lsn(0x70))?.unwrap(), 2);
assert_eq!(tline.get_page_at_lsn(TESTREL_A, 0, Lsn(0x70))?, ZERO_PAGE);
assert_eq!(
tline.get_page_at_lsn(TESTREL_A, 1, Lsn(0x70))?,
TEST_IMG("foo blk 1")
);
// Extend a lot more, leaving a big gap that spans across segments
// FIXME: This is currently broken, see https://github.com/zenithdb/zenith/issues/500
/*
tline.put_page_image(TESTREL_A, 1500, Lsn(0x80), TEST_IMG("foo blk 1500"))?;
tline.advance_last_record_lsn(Lsn(0x80));
assert_eq!(tline.get_relish_size(TESTREL_A, Lsn(0x80))?.unwrap(), 1501);
for blk in 2..1500 {
assert_eq!(
tline.get_page_at_lsn(TESTREL_A, blk, Lsn(0x80))?,
ZERO_PAGE);
}
assert_eq!(
tline.get_page_at_lsn(TESTREL_A, 1500, Lsn(0x80))?,
TEST_IMG("foo blk 1500"));
*/
Ok(())
}