Date: Mon, 8 Jan 2024 10:21:25 -0800
From: Noah Misch <noah@leadboat.com>
To: Peter Geoghegan <pg@bowt.ie>
Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>,
	robertmhaas@gmail.com, Alexander Lakhin <exclusion@gmail.com>,
	PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>,
	Andres Freund <andres@anarazel.de>
Subject: Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
Message-ID: <20240108182125.f8.nmisch@google.com>
References: <17257-1e46de26bec11433@postgresql.org>
 <d5d5af5d-ba46-aff3-9f91-776c70246cc3@gmail.com>
 <CAH2-WzknORjSVufsDQKr7VVnXspRhdsjKBh8brwHEkrUpqVXjg@mail.gmail.com>
 <20231225024402.77@rfd.leadboat.com>
 <CAH2-Wz=X88igoxQvAGW2cFrTsEXK+VJJHap=90-z=juD5DqYdw@mail.gmail.com>
 <20240106202413.e5@rfd.leadboat.com>
 <CAH2-WzkCOh3r-qpS16-TXFg6kzeg12qawJSTTHtNMavtXXG-sg@mail.gmail.com>
 <CAH2-Wz=HT+zHGyQ5bykPEh20KQWnS4uZS_uupxrTrtf-Wt886A@mail.gmail.com>
 <20240106224448.c7@rfd.leadboat.com>
 <CAH2-WznKABiqgDDM2mgYmy4Pk2j-UrT9+PwSmHB3C59D5xtpqQ@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-WznKABiqgDDM2mgYmy4Pk2j-UrT9+PwSmHB3C59D5xtpqQ@mail.gmail.com>
User-Agent: Mutt/2.2.12 (2023-09-09)
Archived-At: 
 <https://www.postgresql.org/message-id/20240108182125.f8.nmisch%40google.com>
Precedence: bulk

On Mon, Jan 08, 2024 at 12:02:01PM -0500, Peter Geoghegan wrote:
> On Sat, Jan 6, 2024 at 5:44 PM Noah Misch <noah@leadboat.com> wrote:
> > Tied to that decision is the choice of semantics when the xmin horizon moves
> > backward during one VACUUM, e.g. when a new walsender xmin does so.  Options:
> >
> > 1. Continue to remove tuples based on the OldestXmin from VACUUM's start.  We
> >    could have already removed some of those tuples, so the walsender xmin
> >    won't achieve a guarantee anyway.  (VACUUM would want ratchet-like behavior
> >    in GlobalVisState, possibly by sharing OldestXmin with pruneheap like you
> >    say.)
> >
> > 2. Move OldestXmin backward, to reflect the latest xmin horizon.  (Perhaps
> >    VACUUM would just pass GlobalVisState to a function that returns the
> >    compatible OldestXmin.)
> >
> > Which way is better?
> 
> I suppose that a hybrid of these two approaches makes the most sense.
> A design that's a lot closer to your #1 than to your #2.
> 
> Under this scheme, pruneheap.c would be explicitly aware of
> OldestXmin, and would promise to respect the exact invariant that we
> need to avoid getting stuck in lazy_scan_prune's loop (or avoid
> confused NewRelfrozenXid tracking on HEAD, which no longer has this
> loop). But it'd be limited to that exact invariant; we'd still avoid
> unduly imposing any requirements on pruning-away deleted tuples whose
> xmax was >= OldestXmin. lazy_scan_prune/vacuumlazy.c shouldn't care if
> we prune away any "extra" heap tuples, just because we can (or just
> because it's convenient to the implementation). Andres has in the past
> placed a lot of emphasis on being able to update the
> GlobalVisState-wise bounds on the fly. Not sure that it's really that
> important that VACUUM does that, but there is no reason to not allow
> it. So we can keep that property (as well as the aforementioned
> high-level OldestXmin immutability property).
> 
> More importantly (at least to me), this scheme allows vacuumlazy.c to
> continue to treat OldestXmin as an immutable cutoff for both pruning
> and freezing -- the high level design doesn't need any revisions. We
> already "freeze away" multixact member XIDs >= OldestXmin in certain
> rare cases (i.e. we remove lockers that are determined to no longer be
> running in FreezeMultiXactId's "second pass" slow path), so there is
> nothing fundamentally novel about the idea of removing some extra XIDs
> >= OldestXmin in passing, just because it happens to be convenient to
> some low-level piece of code that's external to vacuumlazy.c.
> 
> What do you think of that general approach?

That all sounds good to me.

> I see no reason why it
> matters if OldestXmin goes backwards across two VACUUM operations, so
> I haven't tried to avoid that.

That may be fully okay, or we may want to clamp OldestXmin to be no older than
relfrozenxid.  I don't feel great about the system moving relfrozenxid
backward unless it observed an older XID, and observing an older XID would be
a corruption signal.  I don't have a specific way non-monotonic relfrozenxid
breaks things, though.