Initialize 'loaded' correctly in DeltaLayer.

While we're at it, reuse the Book and the VirtualFile that's backing
it even over unload() calls. Previously, we would keep the Book open,
but on load(), we would re-open it anyway, which didn't make much
sense. Now we reuse it it. Alternatively, perhaps we should close it
on unload() to save some memory, but this I'm not going to think too
hard about it right now as the whole load/unload thing is a bit of a
hack and needs to be rewritten.

This is hard to reproduce ATM, because the incorrect state would get
fixed by an unload(). A checkpoint creates the DeltaLayer, and it also
calls unload() afterwards, so the window is not very large. I hit it
occasionally with a scale 1000 pgbench test, after I had modified
InMemoryLayer::write_to_disk() to not write an image layer every time,
which made the DeltaLayers be accessed more often.
This commit is contained in:
Heikki Linnakangas
2021-11-30 22:23:59 +02:00
parent 670205e17a
commit f49ad33f1b

View File

@@ -297,6 +297,11 @@ impl Layer for DeltaLayer {
inner.page_version_metas = VecMap::default();
inner.relsizes = VecMap::default();
inner.loaded = false;
// Note: we keep the Book open. Is that a good idea? The virtual file
// machinery has its own rules for closing the file descriptor if it's not
// needed, but the Book struct uses up some memory, too.
Ok(())
}
@@ -410,7 +415,7 @@ impl DeltaLayer {
end_lsn,
dropped,
inner: Mutex::new(DeltaLayerInner {
loaded: true,
loaded: false,
book: None,
page_version_metas: VecMap::default(),
relsizes,
@@ -495,8 +500,13 @@ impl DeltaLayer {
}
let path = self.path();
let file = VirtualFile::open(&path)?;
let book = Book::new(file)?;
// Open the file if it's not open already.
if inner.book.is_none() {
let file = VirtualFile::open(&path)?;
inner.book = Some(Book::new(file)?);
}
let book = inner.book.as_ref().unwrap();
match &self.path_or_conf {
PathOrConf::Conf(_) => {
@@ -531,12 +541,9 @@ impl DeltaLayer {
debug!("loaded from {}", &path.display());
*inner = DeltaLayerInner {
loaded: true,
book: Some(book),
page_version_metas,
relsizes,
};
inner.page_version_metas = page_version_metas;
inner.relsizes = relsizes;
inner.loaded = true;
Ok(inner)
}