On Fri, Mar 8, 2024 at 10:41 AM Peter Geoghegan <p...@bowt.ie> wrote: > > On Fri, Mar 8, 2024 at 8:49 AM Heikki Linnakangas <hlinn...@iki.fi> wrote: > > ISTM we should revert the above hunk, and backpatch it to v16. I'm a > > little wary because I don't understand why that change was made in the > > first place, though. I think it was just an ill-advised attempt at > > tidying up the code as part of the larger commit, but I'm not sure. > > Peter, do you remember? > > I think that it makes sense to set the VM when indicated by > lazy_scan_prune, independent of what either the visibility map or the > page's PD_ALL_VISIBLE marking say. The whole point of > DISABLE_PAGE_SKIPPING is to deal with VM corruption, after all.
Not that it will be fun to maintain another special case in the VM update code in lazy_scan_prune(), but we could have a special case that checks if DISABLE_PAGE_SKIPPING was passed to vacuum and if all_visible_according_to_vm is true and all_visible is true, we update the VM but don't dirty the page. The docs on DISABLE_PAGE_SKIPPING say it is meant to deal with VM corruption -- it doesn't say anything about dealing with incorrectly set PD_ALL_VISIBLE markings. - Melanie
