Hi,
while thinking about vacuum freeze I noticed that since the checksums
patch visibilitymap_set() does:/* * If data checksums are enabled, we need to protect the heap * page from being torn.
*/if(DataChecksumsEnabled()){ Page heapPage = BufferGetPage(heapBuf);
/* caller is expected to set PD_ALL_VISIBLE first */ Assert(PageIsAllVisible(heapPage)); PageSetLSN(heapPage,
recptr);}
That pattern looks dangerous. Setting the lsn of the heap page will
prevent the next action from doing a FPI even if it would be required.
Its e.g. called like this from lazy_scan_heap:
if (all_visible && !all_visible_according_to_vm){ /* * It should never be the case that the visibility map page
isset * while the page-level bit is clear, but the reverse is allowed * (if checksums are not enabled).
Regardless,set the both bits * so that we get back in sync. * * NB: If the heap page is all-visible but the
VMbit is not set, * we don't need to dirty the heap page. However, if checksums are * enabled, we do need to
makesure that the heap page is dirtied * before passing it to visibilitymap_set(), because it may be * logged.
Giventhat this situation should only happen in rare * cases after a crash, it is not worth optimizing. */
PageSetAllVisible(page); MarkBufferDirty(buf); visibilitymap_set(onerel, blkno, buf, InvalidXLogRecPtr,
vmbuffer, visibility_cutoff_xid);}
other callers look similarly dangerous.
Greetings,
Andres Freund
-- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services
