On Mon, Jun 24, 2024 at 04:51:24PM -0400, Peter Geoghegan wrote: > On Mon, Jun 24, 2024 at 4:36 PM Robert Haas <robertmh...@gmail.com> wrote: > > I'm not sure I understand. The most important thing here is fixing the > > bug. But if we have a choice of how to fix the bug, I'd prefer to do > > it by having the pruning code test one horizon that is always correct, > > rather than (as I think the patch does) having it test against two > > horizons because as a way of covering possible discrepancies between > > those values. > > Your characterizing of OldestXmin + vistest as two horizons seems > pretty arbitrary to me. I know what you mean, of course, but it seems > like a distinction without a difference.
"Two horizons" matches how I model it. If the two were _always_ indicating the same notion of visibility, we wouldn't have this thread. On Mon, Jun 24, 2024 at 03:23:39PM -0400, Melanie Plageman wrote: > Right now, in master, we do use a single horizon when determining what > is pruned -- that from GlobalVisState. OldestXmin is only used for > freezing and full page visibility determinations. Using a different > horizon for pruning by vacuum than freezing is what is causing the > error on master. Agreed, and I think using different sources for pruning and freezing is a recipe for future bugs. Fundamentally, both are about answering "is snapshot_considers_xid_in_progress(snapshot, xid) false for every snapshot?" That's not to say this thread shall unify the two, but I suspect that's the right long-term direction.
