On Thu, May 6, 2021 at 12:32 PM Andres Freund <and...@anarazel.de> wrote: > I think it'd be a good idea to audit the other uses of > all_visible_according_to_vm to make sure there's no issues there. Can't > this e.g. make us miss setting all-visible in > > /* > * Handle setting visibility map bit based on what the VM > said about > * the page before pruning started, and using prunestate > */ > if (!all_visible_according_to_vm && prunestate.all_visible)
I don't think so, because it's the inverse case -- the condition that you quote is concerned with the case where we found the VM all_visible bit to not be set earlier, and then found that we could set it now. The assertion failed because the VM's all_visible bit was set initially, but concurrently unset by some other backend. The all_visible_according_to_vm tracking variable became stale, so it wasn't correct to expect current information from prunestate to agree that the page is still all_visible. High level philosophical observation: This reminds me of another way in which things are too tightly coupled in VACUUM. It's really a pity that the visibility map's all_visible bit serves two purposes -- it remembers pages that VACUUM doesn't have to visit (except perhaps if it's an aggressive VACUUM), and is also used for index-only scans. If it was just used for index-only scans then I don't think it would be necessary for a HOT update to unset a page's all_visible bit. Since a HOT chain's members are always versions of the same logical row, there is no reason why an index-only scan needs to care which precise version is actually visible to its MVCC snapshot (once we know that there must be exactly one version from each HOT chain). -- Peter Geoghegan
