On Tue, Mar 19, 2024 at 9:36 PM Melanie Plageman <melanieplage...@gmail.com> wrote: > * "Freeze" NewRelfrozenXid/NewRelminMxid trackers. > * > * Trackers used when heap_freeze_execute_prepared freezes, or when > there > * are zero freeze plans for a page. It is always valid for > vacuumlazy.c > * to freeze any page, by definition. This even includes pages that > have > * no tuples with storage to consider in the first place. That way > the > * 'totally_frozen' results from heap_prepare_freeze_tuple can always > be > * used in the same way, even when no freeze plans need to be > executed to > * "freeze the page". Only the "freeze" path needs to consider the > need > * to set pages all-frozen in the visibility map under this scheme. > * > * When we freeze a page, we generally freeze all XIDs < OldestXmin, > only > * leaving behind XIDs that are ineligible for freezing, if any. And > so > * you might wonder why these trackers are necessary at all; why > should > * _any_ page that VACUUM freezes _ever_ be left with XIDs/MXIDs that > * ratchet back the top-level NewRelfrozenXid/NewRelminMxid trackers? > * > * It is useful to use a definition of "freeze the page" that does not > * overspecify how MultiXacts are affected. heap_prepare_freeze_tuple > * generally prefers to remove Multis eagerly, but lazy processing is > used > * in cases where laziness allows VACUUM to avoid allocating a new > Multi. > * The "freeze the page" trackers enable this flexibility. > */ > > So, I don't really know if it is right to just check presult->nfrozen > > 0 when updating relminmxid. I have changed it to the way you suggested. > But we can change it back.
I think that this is just about safe. I had to check, though. I see that the FRM_NOOP case (within FreezeMultiXactId/heap_prepare_freeze_tuple) will ratchet back both sets of trackers (both the freeze and no freeze variants). However, it's rather hard to see that this is true. The intent here was that cases where "presult->nfrozen == 0" would always take the "freeze" path. That seems more natural to me, at least, since I think of the freeze path as the default choice. By definition, lazy_scan_prune() can always take the freeze path -- even when the page has no tuples with storage. But it cannot always take the no-freeze path -- "disobeying" pagefrz.freeze_required creates the risk that relfrozenxid/relminmxid will be advanced to unsafe values at the end of the VACUUM. IMV you should stick with that approach now, even if it is currently safe to do it the other way around. -- Peter Geoghegan
