Date: Mon, 24 Jun 2024 18:30:04 -0700
From: Noah Misch <noah@leadboat.com>
To: Melanie Plageman <melanieplageman@gmail.com>,
	Peter Geoghegan <pg@bowt.ie>
Cc: Robert Haas <robertmhaas@gmail.com>,
	Pg Hackers <pgsql-hackers@postgresql.org>,
	Andres Freund <andres@anarazel.de>
Subject: Re: Vacuum ERRORs out considering freezing dead tuples from before
 OldestXmin
Message-ID: <20240625013004.a8.nmisch@google.com>
References: 
 <CAAKRu_bDD7oq9ZwB2OJqub5BovMG6UjEYsoK2LVttadjEqyRGg@mail.gmail.com>
 <CA+TgmoZiArAXM7KwitYWkCmeUQ76Sfg=mgq4_oFo2awWDFdePw@mail.gmail.com>
 <CAH2-Wz=irYFYP5YP2-TajwO3B1maqSSoH-7Ps1aaRv6BHXD7pg@mail.gmail.com>
 <CA+Tgmob4m4TjFCqKFvdR9=Y71fGoH_C-nfPgni6Trt8t3AKOLA@mail.gmail.com>
 <CAAKRu_Y2oTSvhY16EZbLQ6nL3qQXydsfKMXiiCH07KC_GYoRuA@mail.gmail.com>
 <CA+TgmoYzS_bkt_MrNxr5QrXDKfedmh4tStn8UBTTBXqv=3JTew@mail.gmail.com>
 <CAH2-Wzk_nFJOcC6s004bAYGLQ41tPSesTqyBAHcVGxRM97H0Xg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CAH2-Wzk_nFJOcC6s004bAYGLQ41tPSesTqyBAHcVGxRM97H0Xg@mail.gmail.com>
User-Agent: Mutt/2.2.12 (2023-09-09)
Archived-At: 
 <https://www.postgresql.org/message-id/20240625013004.a8.nmisch%40google.com>
Precedence: bulk

On Mon, Jun 24, 2024 at 04:51:24PM -0400, Peter Geoghegan wrote:
> On Mon, Jun 24, 2024 at 4:36 PM Robert Haas <robertmhaas@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.