On Fri, Nov 18, 2022 at 5:06 PM Peter Geoghegan <pg@bowt.ie> wrote:
> I've already prototyped a dedicated immutable "cutoffs" struct, which
> is instantiated exactly once per VACUUM. Seems like a good approach to
> me. The immutable state can be shared by heapam.c's
> heap_prepare_freeze_tuple(), vacuumlazy.c, and even
> vacuum_set_xid_limits() -- so everybody can work off of the same
> struct directly. Will try to get that into shape for the next
> revision.
Attached is v8.
Notable improvement over v7:
* As anticipated on November 18th, his revision adds a new refactoring
commit/patch, which adds a struct that contains fields like
FreezeLimit and OldestXmin, which is used by vacuumlazy.c to pass the
information to heap_prepare_freeze_tuple().
This refactoring makes everything easier to understand -- it's a
significant structural improvement.
* The changes intended to avoid allocating a new Multi during VACUUM
no longer appear in their own commit. That was squashed/combined with
the earlier page-level freezing commit.
This is another structural improvement.
The FreezeMultiXactId() changes were never really an optimization, and
I shouldn't have explained them that way. They are only needed to
avoid MultiXactId related regressions that page-level freezing would
otherwise cause. Doing these changes in the page-level freezing patch
makes that far clearer.
* Fixes an issue with snapshotConflictHorizon values for FREEZE_PAGE
records, where earlier revisions could have more false recovery
conflicts relative to the behavior on HEAD.
In other words, v8 addresses a concern that you (Andres) had in your
review of v6, here:
> > Won't using OldestXmin instead of FreezeLimit potentially cause additional
> > conflicts? Is there any reason to not compute an accurate value?
As anticipated, it is possible to generate valid FREEZE_PAGE
snapshotConflictHorizon using LVPagePruneState.visibility_cutoff_xid
in almost all cases -- so we should avoid almost all false recovery
conflicts. Granted, my approach here only works when the page will
become eligible to mark all-frozen (otherwise we can't trust
LVPagePruneState.visibility_cutoff_xid and have to fall back on
OldestXmin), but that's not really a problem in practice. Since in
practice page-level freezing is supposed to find a way to freeze pages
as a group, or not at all (so falling back on OldestXmin should be
very rare).
I could be more precise about generating a FREEZE_PAGE
snapshotConflictHorizon than this, but that didn't seem worth the
added complexity (I'd prefer to be able to ignore MultiXacts/xmax for
this stuff). I'm pretty sure that the new v8 approach is more than
good enough. It's actually an improvement on HEAD, where
snapshotConflictHorizon is derived from FreezeLimit, an approach with
the same basic problem as deriving snapshotConflictHorizon from
OldestXmin. Namely: using FreezeLimit is a poor proxy for what we
really want to use, which is a cutoff that comes from the specific
latest XID in some specific tuple header on the page we're freezing.
There are no remaining blockers to commit for the first two patches
from v8 (the two patches that add page-level freezing). I think that
I'll be able to commit page-level freezing in a matter of weeks, in
fact. All specific outstanding concerns about page-level freezing have
been addressed.
I believe that page-level freezing is uncontroversial. Unlike later
patches in the series, it changes nothing user-facing about VACUUM --
nothing very high level. Having the freeze plan deduplication work
added by commit 9e540599 helps here. The focus is WAL overhead over
time, and page level freezing can almost be understood as a mechanical
improvement to freezing that keeps costs over time down.
--
Peter Geoghegan
