On Mon, Jan 9, 2023 at 12:58 PM Peter Geoghegan <p...@bowt.ie> wrote: > I didn't realize that affected visibilitymap_set() calls could > generate useless set-VM WAL records until you pointed it out. That's > far more likely to happen than the race condition that I described -- > it has nothing at all to do with concurrency. That's what clinches it > for me.
I didn't spend as much time on this as I'd like to so far, but I think that this concern about visibilitymap_set() actually turns out to not apply. The visibilitymap_set() call in question is gated by a "!VM_ALL_FROZEN()", which is enough to avoid the problem with writing useless VM set records. That doesn't make me doubt my original concern about races where the all-frozen bit can be set, without setting the all-visible bit, and without accounting for the fact that it changed underneath us. That scenario will have !VM_ALL_FROZEN(), so that won't save us. (And we won't test VM_ALL_VISIBLE() or PD_ALL_VISIBLE in a way that is sufficient to realize that all_visible_according_to_vm is stale. prunestate.all_visible being set doesn't reliably indicate that it's not stale, but lazy_scan_heap incorrectly believes that it really does work that way.) -- Peter Geoghegan
