Thread: New strategies for freezing, advancing relfrozenxid early

New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

26 August 2022, 00:21:12

Attached patch series is a completely overhauled version of earlier
work on freezing. Related work from the Postgres 15 cycle became
commits 0b018fab, f3c15cbe, and 44fa8488.

Recap
=====

The main high level goal of this work is to avoid painful, disruptive
antiwraparound autovacuums (and other aggressive VACUUMs) that do way
too much "catch up" freezing, all at once, causing significant
disruption to production workloads. The patches teach VACUUM to care
about how far behind it is on freezing for each table -- the number of
unfrozen all-visible pages that have accumulated so far is directly
and explicitly kept under control over time. Unfrozen pages can be
seen as debt. There isn't necessarily anything wrong with getting into
debt (getting into debt to a small degree is all but inevitable), but
debt can be dangerous when it isn't managed carefully. Accumulating
large amounts of debt doesn't always end badly, but it does seem to
reliably create the *risk* that things will end badly.

Right now, a standard append-only table could easily do *all* freezing
in aggressive/antiwraparound VACUUM, without any earlier
non-aggressive VACUUM operations triggered by
autovacuum_vacuum_insert_threshold doing any freezing at all (unless
the user goes out of their way to tune vacuum_freeze_min_age). There
is currently no natural limit on the number of unfrozen all-visible
pages that can accumulate -- unless you count age(relfrozenxid), the
triggering condition for antiwraparound autovacuum. But relfrozenxid
age predicts almost nothing about how much freezing is required (or
will be required later on). The overall result is that it oftens takes
far too long for freezing to finally happen, even when the table
receives plenty of autovacuums (they all could freeze something, but
in practice just don't freeze anything). It's very hard to avoid that
through tuning, because what we really care about is something pretty
closely related to (if not exactly) the number of unfrozen heap pages
in the system. XID age is fundamentally "the wrong unit" here -- the
physical cost of freezing is the most important thing, by far.

In short, the goal of the patch series/project is to make autovacuum
scheduling much more predictable over time. Especially with very large
append-only tables. The patches improve the performance stability of
VACUUM by managing costs holistically, over time. What happens in one
single VACUUM operation is much less important than the behavior of
successive VACUUM operations over time.

What's new: freezing/skipping strategies
========================================

This newly overhauled version introduces the concept of
per-VACUUM-operation strategies, which we decide on once per VACUUM,
at the very start. There are 2 choices to be made at this point (right
after we acquire OldestXmin and similar cutoffs):

1) Do we scan all-visible pages, or do we skip instead? (Added by
second patch, involves a trade-off between eagerness and laziness.)
2) How should we freeze -- eagerly or lazily? (Added by third patch)

The strategy-based approach can be thought of as something that blurs
the distinction between aggressive and non-aggressive VACUUM, giving
VACUUM more freedom to do either more or less work, based on known
costs and benefits. This doesn't completely supersede
aggressive/antiwraparound VACUUMs, but should make them much rarer
with larger tables, where controlling freeze debt actually matters.
There is a need to keep laziness and eagerness in balance here. We try
to get the benefit of lazy behaviors/strategies, but will still course
correct when it doesn't work out.

A new GUC/reloption called vacuum_freeze_strategy_threshold is added
to control freezing strategy (also influences our choice of skipping
strategy). It defaults to 4GB, so tables smaller than that cutoff
(which are usually the majority of all tables) will continue to freeze
in much the same way as today by default. Our current lazy approach to
freezing makes sense there, and should be preserved for its own sake.

Compatibility
=============

Structuring the new freezing behavior as an explicit user-configurable
strategy is also useful as a bridge between the old and new freezing
behaviors. It makes it fairly easy to get the old/current behavior
where that's preferred -- which, I must admit, is something that
wasn't well thought through last time around. The
vacuum_freeze_strategy_threshold GUC is effectively (though not
explicitly) a compatibility option. Users that want something close to
the old/current behavior can use the GUC or reloption to more or less
opt-out of the new freezing behavior, and can do so selectively. The
GUC should be easy for users to understand, too -- it's just a table
size cutoff.

Skipping pages using a snapshot of the visibility map
=====================================================

We now take a copy of the visibility map at the point that VACUUM
begins, and work off of that when skipping, instead of working off of
the mutable/authoritative VM -- this is a visibility map snapshot.
This new infrastructure helps us to decide on a skipping strategy.
Every non-aggressive VACUUM operation now has a choice to make: Which
skipping strategy should it use?  (This was introduced as
item/question #1 a moment ago.)

The decision on skipping strategy is a decision about our priorities
for this table, at this time: Is it more important to advance
relfrozenxid early (be eager), or to skip all-visible pages instead
(be lazy)? If it's the former, then we must scan every single page
that isn't all-frozen according to the VM snapshot (including every
all-visible page). If it's the latter, we'll scan exactly 0
all-visible pages. Either way, once a decision has been made, we don't
leave much to chance -- we commit. ISTM that this is the only approach
that really makes sense. Fundamentally, we advance relfrozenxid a
table at a time, and at most once per VACUUM operation. And for larger
tables it's just impossible as a practical matter to have frequent
VACUUM operations. We ought to be *somewhat* biased in the direction
of advancing relfrozenxid by *some* amount during each VACUUM, even
when relfrozenxid isn't all that old right now.

A strategy (whether for skipping or for freezing) is a big, up-front
decision -- and there are certain kinds of risks that naturally
accompany that approach. The information driving the decision had
better be fairly reliable! By using a VM snapshot, we can choose our
skipping strategy based on precise information about how many *extra*
pages we will have to scan if we go with eager scanning/relfrozenxid
advancement. Concurrent activity cannot change what we scan and what
we skip, either -- everything is locked in from the start. That seems
important to me. It justifies trying to advance relfrozenxid early,
just because the added cost of scanning any all-visible pages happens
to be low.

This is quite a big shift for VACUUM, at least in some ways. The patch
adds a DETAIL to the "starting vacuuming" INFO message shown by VACUUM
VERBOSE. The VERBOSE output is already supposed to work as a
rudimentary progress indicator (at least when it is run at the
database level), so it now shows the final scanned_pages up-front,
before the physical scan of the heap even begins:

regression=# vacuum verbose tenk1;
INFO:  vacuuming "regression.public.tenk1"
DETAIL:  total table size is 486 pages, 3 pages (0.62% of total) must be scanned
INFO:  finished vacuuming "regression.public.tenk1": index scans: 0
pages: 0 removed, 486 remain, 3 scanned (0.62% of total)
*** SNIP ***
system usage: CPU: user: 0.00 s, system: 0.00 s, elapsed: 0.00 s
VACUUM

I included this VERBOSE tweak in the second patch because it became
natural with VM snapshots, and not because it felt particularly
compelling -- scanned_pages just works like this now (an assertion
verifies that our initial scanned_pages is always an exact match to
what happened during the physical scan, in fact).

There are many things that VM snapshots might also enable that aren't
particularly related to freeze debt. VM snapshotting has the potential
to enable more flexible behavior by VACUUM. I'm thinking of things
like suspend-and-resume for VACUUM/autovacuum, or even autovacuum
scheduling that coordinates autovacuum workers before and during
processing by vacuumlazy.c. Locking-in scanned_pages up-front avoids
the main downside that comes with throttling VACUUM right now: the
fact that simply taking our time during VACUUM will tend to increase
the number of concurrently modified pages that we end up scanning.
These pages are bound to mostly just contain "recently dead" tuples
that the ongoing VACUUM can't do much about anyway -- we could dirty a
lot more heap pages as a result, for little to no benefit.

New patch to avoid allocating MultiXacts
========================================

The fourth and final patch is also new. It corrects an undesirable
consequence of the work done by the earlier patches: it makes VACUUM
avoid allocating new MultiXactIds (unless it's fundamentally
impossible, like in a VACUUM FREEZE). With just the first 3 patches
applied, VACUUM will naively process xmax using a cutoff XID that
comes from OldestXmin (and not FreezeLimit, which is how it works on
HEAD). But with the fourth patch in place VACUUM applies an XID cutoff
of either OldestXmin or FreezeLimit selectively, based on the costs
and benefits for any given xmax.

Just like in lazy_scan_noprune, the low level xmax-freezing code can
pick and choose as it goes, within certain reasonable constraints. We
must accept an older final relfrozenxid/relminmxid value for the rel's
authoritative pg_class tuple as a consequence of avoiding xmax
processing, of course, but that shouldn't matter at all (it's
definitely better than the alternative).

Reducing the WAL space overhead of freezing
===========================================

Not included in this new v1 are other patches that control the
overhead of added freezing -- my focus since joining AWS has been to
get these more strategic patches in shape, and telling the right story
about what I'm trying to do here. I'm going to say a little on the
patches that I have in the pipeline here, though. Getting the
low-level/mechanical overhead of freezing under control will probably
require a few complementary techniques, not just high-level strategies
(though the strategy stuff is the most important piece).

The really interesting omitted-in-v1 patch adds deduplication of
xl_heap_freeze_page WAL records. This reduces the space overhead of
WAL records used to freeze by ~5x in most cases. It works in the
obvious way: we just store the 12 byte freeze plans that appear in
each xl_heap_freeze_page record only once, and then store an array of
item offset numbers for each entry (rather than naively storing a full
12 bytes per tuple frozen per page-level WAL record). This means that
we only need an "extra" ~2 bytes of WAL space per "extra" tuple frozen
(2 bytes for an OffsetNumber) once we decide to freeze something on
the same page. The *marginal* cost can be much lower than it is today,
which makes page-based batching of freezing much more compelling IMV.

Thoughts?
--
Peter Geoghegan

On Thu, Aug 25, 2022 at 2:21 PM Peter Geoghegan <pg@bowt.ie> wrote:
> Attached patch series is a completely overhauled version of earlier
> work on freezing. Related work from the Postgres 15 cycle became
> commits 0b018fab, f3c15cbe, and 44fa8488.

Attached is v2.

This is just to keep CFTester happy, since v1 now has conflicts when
applied against HEAD. There are no notable changes in this v2 compared
to v1.

-- 
Peter Geoghegan

Attachment

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

08 September 2022, 23:23:52

On Wed, Aug 31, 2022 at 12:03 AM Peter Geoghegan <pg@bowt.ie> wrote:
> Actually I'm also ignoring some subtleties with Multis that could make
> this not quite happen, but again, that's only a super obscure corner case.
> The idea that just setting vacuum_freeze_min_age = 0 and
> vacuum_multixact_freeze_min_age = 0 will be enough is definitely true
> in spirit. You don't need to touch vacuum_freeze_table_age (if you did
> then you'd get aggressive VACUUMs, and one goal here is to avoid
> those whenever possible -- especially aggressive antiwraparound
> autovacuums).

Attached is v3. There is a new patch included here -- v3-0004-*patch,
or "Unify aggressive VACUUM with antiwraparound VACUUM". No other
notable changes.

I decided to work on this now because it seems like it might give a
more complete picture of the high level direction that I'm pushing
towards. Perhaps this will make it easier to review the patch series
as a whole, even. The new patch unifies the concept of antiwraparound
VACUUM with the concept of aggressive VACUUM. Now there is only
antiwraparound and regular VACUUM (uh, barring VACUUM FULL). And now
antiwraparound VACUUMs are not limited to antiwraparound autovacuums
-- a manual VACUUM can also be antiwraparound (that's just the new
name for "aggressive").

We will typically only get antiwraparound vacuuming in a regular
VACUUM when the user goes out of their way to get that behavior.
VACUUM FREEZE is the best example. For the most part the
skipping/freezing strategy stuff has a good sense of what matters
already, and shouldn't need to be guided very often.

The patch relegates vacuum_freeze_table_age to a compatibility option,
making its default -1, meaning "just use autovacuum_freeze_max_age". I
always thought that having two table age based GUCs was confusing.
There was a period between 2006 and 2009 when we had
autovacuum_freeze_max_age, but did not yet have
vacuum_freeze_table_age. This change can almost be thought of as a
return to the simpler user interface that existed at that time. Of
course we must not resurrect the problems that vacuum_freeze_table_age
was intended to address (see originating commit 65878185) by mistake.
We need an improved version of the same basic concept, too.

The patch more or less replaces the table-age-aggressive-escalation
concept (previously implemented using vacuum_freeze_table_age) with
new logic that makes lazyvacuum.c's choice of skipping strategy *also*
depend upon table age -- it is now one more factor to be considered.
Both costs and benefits are weighed here. We now give just a little
weight to table age at a relatively early stage (XID-age-wise early),
and escalate from there. As the table's relfrozenxid gets older and
older, we give less and less weight to putting off the cost of
freezing. This general approach is possible because the false
dichotomy that is "aggressive vs non-aggressive" has mostly been
eliminated. This makes things less confusing for users and hackers.

The details of the skipping-strategy-choice algorithm are still
unsettled in v3 (no real change there). ISTM that the important thing
is still the high level concepts. Jeff was slightly puzzled by the
emphasis placed on the cost model/strategy stuff, at least at one
point. Hopefully my intent will be made clearer by the ideas featured
in the new patch. The skipping strategy decision making process isn't
particularly complicated, but it now looks more like an optimization
problem of some kind or other.

It might make sense to go further in the same direction by making
"regular vs aggressive/antiwraparound" into a *strict* continuum. In
other words, it might make sense to get rid of the two remaining cases
where VACUUM conditions its behavior on whether this VACUUM operation
is antiwraparound/aggressive or not. I'm referring to the cleanup lock
skipping behavior around lazy_scan_noprune(), as well as the
PROC_VACUUM_FOR_WRAPAROUND no-auto-cancellation behavior enforced in
autovacuum workers. We will still need to keep roughly the same two
behaviors, but the timelines can be totally different. We must be
reasonably sure that the cure won't be worse than the disease -- I'm
aware of quite a few individual cases that felt that way [1].
Aggressive interventions can make sense, but they need to be
proportionate to the problem that's right in front of us. "Kicking the
can down the road" is often the safest and most responsible approach
-- it all depends on the details.

[1] https://www.tritondatacenter.com/blog/manta-postmortem-7-27-2015
is the most high profile example, but I have personally been called in
to deal with similar problems in the past
--
Peter Geoghegan

Attachment

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

13 September 2022, 20:53:06

On Thu, Sep 8, 2022 at 1:23 PM Peter Geoghegan <pg@bowt.ie> wrote:
> Attached is v3. There is a new patch included here -- v3-0004-*patch,
> or "Unify aggressive VACUUM with antiwraparound VACUUM". No other
> notable changes.
>
> I decided to work on this now because it seems like it might give a
> more complete picture of the high level direction that I'm pushing
> towards. Perhaps this will make it easier to review the patch series
> as a whole, even.

This needed to be rebased over the guc.c work recently pushed to HEAD.

Attached is v4. This isn't just to fix bitrot, though; I'm also
including one new patch -- v4-0006-*.patch. This small patch teaches
VACUUM to size dead_items while capping the allocation at the space
required for "scanned_pages * MaxHeapTuplesPerPage" item pointers. In
other words, we now use scanned_pages instead of rel_pages to cap the
size of dead_items, potentially saving quite a lot of memory. There is
no possible downside to this approach, because we already know exactly
how many pages will be scanned from the VM snapshot -- there is zero
added risk of a second pass over the indexes.

This is still only scratching the surface of what is possible with
dead_items. The visibility map snapshot concept can enable a far more
sophisticated approach to resource management in vacuumlazy.c. It
could help us to replace a simple array of item pointers (the current
dead_items array) with a faster and more space-efficient data
structure. Masahiko Sawada has done a lot of work on this recently, so
this may interest him.

We don't just have up-front knowledge of the total number of
scanned_pages with VM snapshots -- we also have up-front knowledge of
which specific pages will be scanned. So we have reliable information
about the final distribution of dead_items (which specific heap blocks
might have dead_items) right from the start. While this extra
information/context is not a totally complete picture, it still seems
like it could be very useful as a way of driving how some new
dead_items data structure compresses TIDs. That will depend on the
distribution of TIDs -- the final "heap TID key space".

VM snapshots could also make it practical for the new data structure
to spill to disk to avoid multiple index scans/passed by VACUUM.
Perhaps this will result in behavior that's similar to how hash joins
spill to disk -- having 90% of the memory required to do everything
in-memory *usually* has similar performance characteristics to just
doing everything in memory. Most individual TID lookups from
ambulkdelete() will find that the TID *doesn't* need to be deleted --
a little like a hash join with low join selectivity (the common case
for hash joins). It's not like a merge join + sort, where we must
either spill everything or nothing (a merge join can be better than a
hash join with high join selectivity).

-- 
Peter Geoghegan

On Thu, Sep 8, 2022 at 1:23 PM Peter Geoghegan <pg@bowt.ie> wrote:
> It might make sense to go further in the same direction by making
> "regular vs aggressive/antiwraparound" into a *strict* continuum. In
> other words, it might make sense to get rid of the two remaining cases
> where VACUUM conditions its behavior on whether this VACUUM operation
> is antiwraparound/aggressive or not.

I decided to go ahead with this in the attached revision, v5. This
revision totally gets rid of the general concept of discrete
aggressive/non-aggressive modes for each VACUUM operation (see
"v5-0004-Make-VACUUM-s-aggressive-behaviors-continuous.patch" and its
commit message). My new approach turned out to be simpler than the
previous half measures that I described as "unifying aggressive and
antiwraparound" (which itself first appeared in v3).

I now wish that I had all of these pieces in place for v1, since this
was the direction I was thinking of all along -- that might have made
life easier for reviewers like Jeff. What we have in v5 is what I had
in mind all along, which turns out to have only a little extra code
anyway. It might have been less confusing if I'd started this thread
with something like v5 -- the story I need to tell would have been
simpler that way. This is pretty much the end point I had in mind.

Note that we still retain what were previously "aggressive only"
behaviors. We only remove "aggressive" as a distinct mode of operation
that exclusively applies the aggressive behaviors. We're now selective
in how we apply each of the behaviors, based on the needs of the
table. We want to behave in a way that's proportionate to the problem
at hand, which is made easy by not tying anything to a discrete mode
of operation. It's a false dichotomy; why should we ever have only one
reason for running VACUUM, that's determined up front?

There are still antiwraparound autovacuums in v5, but that is really
just another way that autovacuum can launch an autovacuum worker (much
like it was before the introduction of the visibility map in 8.4) --
both conceptually, and in terms of how the code works in vacuumlazy.c.
In practice an antiwraparound autovacuum is guaranteed to advance
relfrozenxid in roughly the same way as on HEAD (otherwise what's the
point?), but that doesn't make the VACUUM operation itself special in
any way. Besides, antiwraparound autovacuums will naturally be rare,
because there are many more opportunities for a VACUUM to advance
relfrozenxid "early" now (only "early" relative to how it would work
on early Postgres versions). It's already clear that having
antiwraparound autovacuums and aggressive mode VACUUMs as two separate
concepts that are closely associated has some problems [1]. Formally
making antiwraparound autovacuums just another way to launch a VACUUM
via autovacuum seems quite useful to me.

For the most part users are expected to just take relfrozenxid
advancement for granted now. They should mostly be able to assume that
VACUUM will do whatever is required to keep it sufficiently current
over time. They can influence VACUUM's behavior, but that mostly works
at the level of the table (not the level of any individual VACUUM
operation). The freezing and skipping strategy stuff should do what is
necessary to keep up in the long run. We don't want to put too much
emphasis on relfrozenxid in the short run, because it isn't a reliable
proxy for how we've kept up with the physical work of freezing --
that's what really matters. It should be okay to "fall behind on table
age" in the short run, provided we don't fall behind on the physical
work of freezing. Those two things shouldn't be conflated.

We now use a separate pair of XID/MXID-based cutoffs to determine
whether or not we're willing to wait for a cleanup lock the hard way
(which can happen in any VACUUM, since of course there is no longer
any special VACUUM with special behaviors). The new pair of cutoffs
replace the use of FreezeLimit/MultiXactCutoff by lazy_scan_noprune
(those are now only used to decide on what to freeze inside
lazy_scan_prune). Same concept, but with a different, independent
timeline. This was necessary just to get an existing isolation test
(vacuum-no-cleanup-lock) to continue to work. But it just makes sense
to have a different timeline for a completely different behavior. And
it'll be more robust.

It's a really bad idea for VACUUM to try to wait indefinitely long for
a cleanup lock, since that's totally outside of its control. It
typically won't take very long at all for VACUUM to acquire a cleanup
lock, of course, but that is beside the point -- who really cares
what's true on average, for something like this? Sometimes it'll take
hours to acquire a cleanup lock, and there is no telling when that
might happen! And so pausing VACUUM/freezing of all other pages just
to freeze one page makes little sense. Waiting for a cleanup lock
before we really need to is just an overreaction, which risks making
the situation worse. The cure must not be worse than the disease.

This revision also resolves problems with freezing MultiXactIds too
lazily [2]. We now always trigger page level freezing in the event of
encountering a Multi. This is more consistent with the behavior on
HEAD, where we can easily process a Multi well before the cutoff
represented by vacuum_multixact_freeze_min_age (e.g., we notice that a
Multi has no members still running, making it safe to remove before
the cutoff is reached).

Also attaching a prebuilt copy of the "routine vacuuming" docs as of
v5. This is intended to be a convenience for reviewers, or anybody
with a general interest in the patch series. The docs certainly still
need work, but I feel that I'm making progress on that side of things
(especially in this latest revision). Making life easier for DBAs is
the single most important goal of this work, so the user docs are of
central importance. The current "Routine Vacuuming" docs have lots of
problems, but to some extent the problems are with the concepts
themselves.

[1] https://postgr.es/m/CAH2-Wz=DJAokY_GhKJchgpa8k9t_H_OVOvfPEn97jGNr9W=deg@mail.gmail.com
[2] https://postgr.es/m/CAH2-Wz=+B5f1izRDPYKw+sUgOr6=AkWXp2NikU5cub0ftbRQhA@mail.gmail.com
--
Peter Geoghegan

Attachment

Re: New strategies for freezing, advancing relfrozenxid early

From

Justin Pryzby

Date:

11 November 2022, 06:44:55

Note that this fails under -fsanitize=align

Subject: [PATCH v5 2/6] Teach VACUUM to use visibility map snapshot.

performing post-bootstrap initialization ...
../src/backend/access/heap/visibilitymap.c:482:38: runtime error: load of misaligned address 0x5559e1352424 for type
'uint64',which requires 8 byte alignment
 

>                         *all_visible += pg_popcount64(umap[i] & VISIBLE_MASK64);

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

16 November 2022, 06:02:12

On Thu, Nov 10, 2022 at 7:44 PM Justin Pryzby <pryzby@telsasoft.com> wrote:
> performing post-bootstrap initialization ...
> ../src/backend/access/heap/visibilitymap.c:482:38: runtime error: load of misaligned address 0x5559e1352424 for type
'uint64',which requires 8 byte alignment

This issue is fixed in the attached revision, v6. I now avoid breaking
alignment-picky platforms in visibilitymap.c by using PGAlignedBlock
in the vm snapshot struct (this replaces the raw char buffer used in
earlier revisions).

Posting v6 will also keep CFTester happy. v5 no longer applies cleanly
due to conflicts caused by today's "Deduplicate freeze plans in freeze
WAL records" commit.

No other changes in v6 that are worth noting here.

Thanks
--
Peter Geoghegan

Attachment

Re: New strategies for freezing, advancing relfrozenxid early

From

Andres Freund

Date:

16 November 2022, 08:20:23

Hi,

On 2022-11-15 19:02:12 -0800, Peter Geoghegan wrote:
> From 352867c5027fae6194ab1c6480cd326963e201b1 Mon Sep 17 00:00:00 2001
> From: Peter Geoghegan <pg@bowt.ie>
> Date: Sun, 12 Jun 2022 15:46:08 -0700
> Subject: [PATCH v6 1/6] Add page-level freezing to VACUUM.
> 
> Teach VACUUM to decide on whether or not to trigger freezing at the
> level of whole heap pages, not individual tuple fields.  OldestXmin is
> now treated as the cutoff for freezing eligibility in all cases, while
> FreezeLimit is used to trigger freezing at the level of each page (we
> now freeze all eligible XIDs on a page when freezing is triggered for
> the page).
> 
> This approach decouples the question of _how_ VACUUM could/will freeze a
> given heap page (which of its XIDs are eligible to be frozen) from the
> question of whether it actually makes sense to do so right now.
> 
> Just adding page-level freezing does not change all that much on its
> own: VACUUM will still typically freeze very lazily, since we're only
> forcing freezing of all of a page's eligible tuples when we decide to
> freeze at least one (on the basis of XID age and FreezeLimit).  For now
> VACUUM still freezes everything almost as lazily as it always has.
> Later work will teach VACUUM to apply an alternative eager freezing
> strategy that triggers page-level freezing earlier, based on additional
> criteria.
> ---
>  src/include/access/heapam.h          |  42 +++++-
>  src/backend/access/heap/heapam.c     | 199 +++++++++++++++++----------
>  src/backend/access/heap/vacuumlazy.c |  95 ++++++++-----
>  3 files changed, 222 insertions(+), 114 deletions(-)
> 
> diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
> index ebe723abb..ea709bf1b 100644
> --- a/src/include/access/heapam.h
> +++ b/src/include/access/heapam.h
> @@ -112,6 +112,38 @@ typedef struct HeapTupleFreeze
>      OffsetNumber offset;
>  } HeapTupleFreeze;
>  
> +/*
> + * State used by VACUUM to track what the oldest extant XID/MXID will become
> + * when determing whether and how to freeze a page's heap tuples via calls to
> + * heap_prepare_freeze_tuple.

Perhaps this could say something like "what the oldest extant XID/MXID
currently is and what it would be if we decide to freeze the page" or such?


> + * The relfrozenxid_out and relminmxid_out fields are the current target
> + * relfrozenxid and relminmxid for VACUUM caller's heap rel.  Any and all

"VACUUM caller's heap rel." could stand to be rephrased.


> + * unfrozen XIDs or MXIDs that remain in caller's rel after VACUUM finishes
> + * _must_ have values >= the final relfrozenxid/relminmxid values in pg_class.
> + * This includes XIDs that remain as MultiXact members from any tuple's xmax.
> + * Each heap_prepare_freeze_tuple call pushes back relfrozenxid_out and/or
> + * relminmxid_out as needed to avoid unsafe values in rel's authoritative
> + * pg_class tuple.
> + *
> + * Alternative "no freeze" variants of relfrozenxid_nofreeze_out and
> + * relminmxid_nofreeze_out must also be maintained for !freeze pages.
> + */

relfrozenxid_nofreeze_out isn't really a "no freeze variant" :)

I think it might be better to just always maintain the nofreeze state.


> +typedef struct HeapPageFreeze
> +{
> +    /* Is heap_prepare_freeze_tuple caller required to freeze page? */
> +    bool        freeze;

s/freeze/freeze_required/?


> +    /* Values used when page is to be frozen based on freeze plans */
> +    TransactionId relfrozenxid_out;
> +    MultiXactId relminmxid_out;
> +
> +    /* Used by caller for '!freeze' pages */
> +    TransactionId relfrozenxid_nofreeze_out;
> +    MultiXactId relminmxid_nofreeze_out;
> +
> +} HeapPageFreeze;
> +

Given the number of parameters to heap_prepare_freeze_tuple, why don't we pass
in more of them in via HeapPageFreeze?


>  /* ----------------
>   *        function prototypes for heap access method
>   *
> @@ -180,17 +212,17 @@ extern void heap_inplace_update(Relation relation, HeapTuple tuple);
>  extern bool heap_prepare_freeze_tuple(HeapTupleHeader tuple,
>                                        TransactionId relfrozenxid, TransactionId relminmxid,
>                                        TransactionId cutoff_xid, TransactionId cutoff_multi,
> +                                      TransactionId limit_xid, MultiXactId limit_multi,
>                                        HeapTupleFreeze *frz, bool *totally_frozen,
> -                                      TransactionId *relfrozenxid_out,
> -                                      MultiXactId *relminmxid_out);
> +                                      HeapPageFreeze *xtrack);

What does 'xtrack' stand for? Xid Tracking?


>   * VACUUM caller must assemble HeapFreezeTuple entries for every tuple that we
>   * returned true for when called.  A later heap_freeze_execute_prepared call
> - * will execute freezing for caller's page as a whole.
> + * will execute freezing for caller's page as a whole.  Caller should also
> + * initialize xtrack fields for page as a whole before calling here with first
> + * tuple for the page.  See page_frozenxid_tracker comments.

s/should/need to/?

page_frozenxid_tracker appears to be a dangling pointer.


> +     * VACUUM calls limit_xid "FreezeLimit", and cutoff_xid "OldestXmin".
> +     * (limit_multi is "MultiXactCutoff", and cutoff_multi "OldestMxact".)

Hm. Perhaps we should just rename them if it requires this kind of
explanation? They're really not good names.



> @@ -6524,8 +6524,8 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple,
>          else
>          {
>              /* xmin to remain unfrozen.  Could push back relfrozenxid_out. */
> -            if (TransactionIdPrecedes(xid, *relfrozenxid_out))
> -                *relfrozenxid_out = xid;
> +            if (TransactionIdPrecedes(xid, xtrack->relfrozenxid_out))
> +                xtrack->relfrozenxid_out = xid;
>          }
>      }

Could use TransactionIdOlder().


> @@ -6563,8 +6564,11 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple,
>               */
>              Assert(!freeze_xmax);
>              Assert(TransactionIdIsValid(newxmax));
> -            if (TransactionIdPrecedes(newxmax, *relfrozenxid_out))
> -                *relfrozenxid_out = newxmax;
> +            Assert(heap_tuple_would_freeze(tuple, limit_xid, limit_multi,
> +                                           &xtrack->relfrozenxid_nofreeze_out,
> +                                           &xtrack->relminmxid_nofreeze_out));
> +            if (TransactionIdPrecedes(newxmax, xtrack->relfrozenxid_out))
> +                xtrack->relfrozenxid_out = newxmax;

Perhaps the Assert(heap_tuple_would_freeze()) bit could be handled once at the
end of the routine, for all paths?


> @@ -6731,18 +6751,36 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple,
>              else
>                  frz->frzflags |= XLH_FREEZE_XVAC;
>  
> -            /*
> -             * Might as well fix the hint bits too; usually XMIN_COMMITTED
> -             * will already be set here, but there's a small chance not.
> -             */
> +            /* Set XMIN_COMMITTED defensively */
>              Assert(!(tuple->t_infomask & HEAP_XMIN_INVALID));
>              frz->t_infomask |= HEAP_XMIN_COMMITTED;
> +
> +            /*
> +             * Force freezing any page with an xvac to keep things simple.
> +             * This allows totally_frozen tracking to ignore xvac.
> +             */
>              changed = true;
> +            xtrack->freeze = true;
>          }
>      }

Oh - I totally didn't realize that ->freeze is an out parameter. Seems a bit
odd to have the other fields suffied with _out but not this one?



> @@ -6786,13 +6824,13 @@ heap_execute_freeze_tuple(HeapTupleHeader tuple, HeapTupleFreeze *frz)
>   */
>  void
>  heap_freeze_execute_prepared(Relation rel, Buffer buffer,
> -                             TransactionId FreezeLimit,
> +                             TransactionId OldestXmin,
>                               HeapTupleFreeze *tuples, int ntuples)
>  {
>      Page        page = BufferGetPage(buffer);
>  
>      Assert(ntuples > 0);
> -    Assert(TransactionIdIsValid(FreezeLimit));
> +    Assert(TransactionIdIsValid(OldestXmin));
>  
>      START_CRIT_SECTION();
>  
> @@ -6822,11 +6860,10 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer,
>  
>          /*
>           * latestRemovedXid describes the latest processed XID, whereas
> -         * FreezeLimit is (approximately) the first XID not frozen by VACUUM.
> -         * Back up caller's FreezeLimit to avoid false conflicts when
> -         * FreezeLimit is precisely equal to VACUUM's OldestXmin cutoff.
> +         * OldestXmin is the first XID not frozen by VACUUM.  Back up caller's
> +         * OldestXmin to avoid false conflicts.
>           */
> -        latestRemovedXid = FreezeLimit;
> +        latestRemovedXid = OldestXmin;
>          TransactionIdRetreat(latestRemovedXid);
>  
>          xlrec.latestRemovedXid = latestRemovedXid;

Won't using OldestXmin instead of FreezeLimit potentially cause additional
conflicts? Is there any reason to not compute an accurate value?


> @@ -1634,27 +1639,23 @@ retry:
>              continue;
>          }
>  
> -        /*
> -         * LP_DEAD items are processed outside of the loop.
> -         *
> -         * Note that we deliberately don't set hastup=true in the case of an
> -         * LP_DEAD item here, which is not how count_nondeletable_pages() does
> -         * it -- it only considers pages empty/truncatable when they have no
> -         * items at all (except LP_UNUSED items).
> -         *
> -         * Our assumption is that any LP_DEAD items we encounter here will
> -         * become LP_UNUSED inside lazy_vacuum_heap_page() before we actually
> -         * call count_nondeletable_pages().  In any case our opinion of
> -         * whether or not a page 'hastup' (which is how our caller sets its
> -         * vacrel->nonempty_pages value) is inherently race-prone.  It must be
> -         * treated as advisory/unreliable, so we might as well be slightly
> -         * optimistic.
> -         */
>          if (ItemIdIsDead(itemid))
>          {
> +            /*
> +             * Delay unsetting all_visible until after we have decided on
> +             * whether this page should be frozen.  We need to test "is this
> +             * page all_visible, assuming any LP_DEAD items are set LP_UNUSED
> +             * in final heap pass?" to reach a decision.  all_visible will be
> +             * unset before we return, as required by lazy_scan_heap caller.
> +             *
> +             * Deliberately don't set hastup for LP_DEAD items.  We make the
> +             * soft assumption that any LP_DEAD items encountered here will
> +             * become LP_UNUSED later on, before count_nondeletable_pages is
> +             * reached.  Whether the page 'hastup' is inherently race-prone.
> +             * It must be treated as unreliable by caller anyway, so we might
> +             * as well be slightly optimistic about it.
> +             */
>              deadoffsets[lpdead_items++] = offnum;
> -            prunestate->all_visible = false;
> -            prunestate->has_lpdead_items = true;
>              continue;
>          }

What does this have to do with the rest of the commit? And why are we doing
this?


> @@ -1782,11 +1783,13 @@ retry:
>          if (heap_prepare_freeze_tuple(tuple.t_data,
>                                        vacrel->relfrozenxid,
>                                        vacrel->relminmxid,
> +                                      vacrel->OldestXmin,
> +                                      vacrel->OldestMxact,
>                                        vacrel->FreezeLimit,
>                                        vacrel->MultiXactCutoff,
>                                        &frozen[tuples_frozen],
>                                        &tuple_totally_frozen,
> -                                      &NewRelfrozenXid, &NewRelminMxid))
> +                                      &xtrack))
>          {
>              /* Save prepared freeze plan for later */
>              frozen[tuples_frozen++].offset = offnum;
> @@ -1807,9 +1810,33 @@ retry:
>       * that will need to be vacuumed in indexes later, or a LP_NORMAL tuple
>       * that remains and needs to be considered for freezing now (LP_UNUSED and
>       * LP_REDIRECT items also remain, but are of no further interest to us).
> +     *
> +     * Freeze the page when heap_prepare_freeze_tuple indicates that at least
> +     * one XID/MXID from before FreezeLimit/MultiXactCutoff is present.
>       */
> -    vacrel->NewRelfrozenXid = NewRelfrozenXid;
> -    vacrel->NewRelminMxid = NewRelminMxid;
> +    if (xtrack.freeze || tuples_frozen == 0)
> +    {
> +        /*
> +         * We're freezing the page.  Our final NewRelfrozenXid doesn't need to
> +         * be affected by the XIDs that are just about to be frozen anyway.

Seems quite confusing to enter a block with described as "We're freezing the
page." when we're not freezing anything (tuples_frozen == 0).


> +         * Note: although we're freezing all eligible tuples on this page, we
> +         * might not need to freeze anything (might be zero eligible tuples).
> +         */
> +        vacrel->NewRelfrozenXid = xtrack.relfrozenxid_out;
> +        vacrel->NewRelminMxid = xtrack.relminmxid_out;
> +        freeze_all_eligible = true;

I don't really get what freeze_all_eligible is trying to do.


>  #ifdef USE_ASSERT_CHECKING
>      /* Note that all_frozen value does not matter when !all_visible */
> -    if (prunestate->all_visible)
> +    if (prunestate->all_visible && lpdead_items == 0)
>      {
>          TransactionId cutoff;
>          bool        all_frozen;
> @@ -1849,8 +1876,7 @@ retry:
>          if (!heap_page_is_all_visible(vacrel, buf, &cutoff, &all_frozen))
>              Assert(false);

Not related to this change, but why isn't this just
Assert(heap_page_is_all_visible(vacrel, buf, &cutoff, &all_frozen))?



> From 8f3b6237affda15101ffb0b88787bfd6bb92e32f Mon Sep 17 00:00:00 2001
> From: Peter Geoghegan <pg@bowt.ie>
> Date: Mon, 18 Jul 2022 14:35:44 -0700
> Subject: [PATCH v6 2/6] Teach VACUUM to use visibility map snapshot.
> 
> Acquire an in-memory immutable "snapshot" of the target rel's visibility
> map at the start of each VACUUM, and use the snapshot to determine when
> and how VACUUM will skip pages.

This should include a description of the memory usage effects.


> This has significant advantages over the previous approach of using the
> authoritative VM fork to decide on which pages to skip.  The number of
> heap pages processed will no longer increase when some other backend
> concurrently modifies a skippable page, since VACUUM will continue to
> see the page as skippable (which is correct because the page really is
> still skippable "relative to VACUUM's OldestXmin cutoff").

Why is it an advantage for the number of pages to not increase?


> It also
> gives VACUUM reliable information about how many pages will be scanned,
> before its physical heap scan even begins.  That makes it easier to
> model the costs that VACUUM incurs using a top-down, up-front approach.
> 
> Non-aggressive VACUUMs now make an up-front choice about VM skipping
> strategy: they decide whether to prioritize early advancement of
> relfrozenxid (eager behavior) over avoiding work by skipping all-visible
> pages (lazy behavior).  Nothing about the details of how lazy_scan_prune
> freezes changes just yet, though a later commit will add the concept of
> freezing strategies.
> 
> Non-aggressive VACUUMs now explicitly commit to (or decide against)
> early relfrozenxid advancement up-front.

Why?


> VACUUM will now either scan
> every all-visible page, or none at all.  This replaces lazy_scan_skip's
> SKIP_PAGES_THRESHOLD behavior, which was intended to enable early
> relfrozenxid advancement (see commit bf136cf6), but left many of the
> details to chance.

The main goal according to bf136cf6 was to avoid defeating OS readahead, so I
think it should be mentioned here.

To me this is something that ought to be changed separately from the rest of
this commit.


> TODO: We don't spill VM snapshots to disk just yet (resource management
> aspects of VM snapshots still need work).  For now a VM snapshot is just
> a copy of the VM pages stored in local buffers allocated by palloc().

HEAPBLOCKS_PER_PAGE is 32672 with the defaults. The maximum relation size is
2**32 - 1 blocks. So the max FSM size is 131458 pages, a bit more than 1GB. Is
that correct?

For large relations that are already nearly all-frozen this does add a
noticable amount of overhead, whether spilled to disk or not. Of course
they're also not going to be vacuumed super often, but ...

Perhaps worth turning the VM into a range based description for the snapshot,
given it's a readonly datastructure in local memory? And we don't necessarily
need the all-frozen and all-visible in memory, one should suffice? We don't
even need random access, so it could easily be allocated incrementally, rather
than one large allocation.

Hard to imagine anybody having a multi-TB table without "runs" of
all-visible/all-frozen. I don't think it'd be worth worrying about patterns
that'd be inefficient in a range representation.



> +    /*
> +     * VACUUM must scan all pages that might have XIDs < OldestXmin in tuple
> +     * headers to be able to safely advance relfrozenxid later on.  There is
> +     * no good reason to scan any additional pages. (Actually we might opt to
> +     * skip all-visible pages.  Either way we won't scan pages for no reason.)
> +     *
> +     * Now that OldestXmin and rel_pages are acquired, acquire an immutable
> +     * snapshot of the visibility map as well.  lazy_scan_skip works off of
> +     * the vmsnap, not the authoritative VM, which can continue to change.
> +     * Pages that lazy_scan_heap will scan are fixed and known in advance.

Hm. It's a bit sad to compute the snapshot after determining OldestXmin.

We probably should refresh OldestXmin periodically. That won't allow us to get
a more aggressive relfrozenxid, but it'd allow to remove more gunk.


> +     *
> +     * The exact number of pages that lazy_scan_heap will scan also depends on
> +     * our choice of skipping strategy.  VACUUM can either choose to skip any
> +     * all-visible pages lazily, or choose to scan those same pages instead.

What does it mean to "skip lazily"?





> +        /*
> +         * Visibility map page copied to local buffer for caller's snapshot.
> +         * Caller requires an exact count of all-visible and all-frozen blocks
> +         * in the heap relation.  Handle that now.

This part of the comment seems like it actually belongs further down?


> +         * Must "truncate" our local copy of the VM to avoid incorrectly
> +         * counting heap pages >= rel_pages as all-visible/all-frozen.  Handle
> +         * this by clearing irrelevant bits on the last VM page copied.
> +         */

Hm - why would those bits already be set?


> +        map = PageGetContents(localvmpage);
> +        if (mapBlock == mapBlockLast)
> +        {
> +            /* byte and bit for first heap page not to be scanned by VACUUM */
> +            uint32        truncByte = HEAPBLK_TO_MAPBYTE(rel_pages);
> +            uint8        truncOffset = HEAPBLK_TO_OFFSET(rel_pages);
> +
> +            if (truncByte != 0 || truncOffset != 0)
> +            {
> +                /* Clear any bits set for heap pages >= rel_pages */
> +                MemSet(&map[truncByte + 1], 0, MAPSIZE - (truncByte + 1));
> +                map[truncByte] &= (1 << truncOffset) - 1;
> +            }
> +
> +            /* Now it's safe to tally bits from this final VM page below */
> +        }
> +
> +        /* Tally the all-visible and all-frozen counts from this page */
> +        umap = (uint64 *) map;
> +        for (int i = 0; i < MAPSIZE / sizeof(uint64); i++)
> +        {
> +            *all_visible += pg_popcount64(umap[i] & VISIBLE_MASK64);
> +            *all_frozen += pg_popcount64(umap[i] & FROZEN_MASK64);
> +        }
> +    }
> +
> +    return vmsnap;
> +}



> From 4f5969932451869f0f28295933c28de49a22fdf2 Mon Sep 17 00:00:00 2001
> From: Peter Geoghegan <pg@bowt.ie>
> Date: Mon, 18 Jul 2022 15:13:27 -0700
> Subject: [PATCH v6 3/6] Add eager freezing strategy to VACUUM.
> 
> Avoid large build-ups of all-visible pages by making non-aggressive
> VACUUMs freeze pages proactively for VACUUMs/tables where eager
> vacuuming is deemed appropriate.  Use of the eager strategy (an
> alternative to the classic lazy freezing strategy) is controlled by a
> new GUC, vacuum_freeze_strategy_threshold (and an associated
> autovacuum_* reloption).  Tables whose rel_pages are >= the cutoff will
> have VACUUM use the eager freezing strategy.

What's the logic behind a hard threshold?  Suddenly freezing everything on a
huge relation seems problematic. I realize that never getting all that far
behind is part of the theory, but I don't think that's always going to work.

Wouldn't a better strategy be to freeze a percentage of the relation on every
non-aggressive vacuum? That way the amount of work for an eventual aggressive
vacuum will shrink, without causing individual vacuums to take extremely long.


> When the eager strategy is in use, lazy_scan_prune will trigger freezing
> a page's tuples at the point that it notices that it will at least
> become all-visible -- it can be made all-frozen instead.  We still
> respect FreezeLimit, though: the presence of any XID < FreezeLimit also
> triggers page-level freezing (just as it would with the lazy strategy).

The other thing that I think would be to good to use is a) whether the page is
already in s_b, and b) whether the page already is dirty. The cost of freezing
shrinks significantly if it doesn't cause an additional read + write. And that
additional IO IMO one of the major concerns with freezing much more
aggressively in OLTPish workloads where a lot of the rows won't ever get old
enough to need freezing.




> From f2066c8ca5ba1b6f31257a36bb3dd065ecb1e3d4 Mon Sep 17 00:00:00 2001
> From: Peter Geoghegan <pg@bowt.ie>
> Date: Mon, 5 Sep 2022 17:46:34 -0700
> Subject: [PATCH v6 4/6] Make VACUUM's aggressive behaviors continuous.
> 
> The concept of aggressive/scan_all VACUUM dates back to the introduction
> of the visibility map in Postgres 8.4.  Before then, every lazy VACUUM
> was "equally aggressive": each operation froze whatever tuples before
> the age-wise cutoff needed to be frozen.  And each table's relfrozenxid
> was updated at the end.  In short, the previous behavior was much less
> efficient, but did at least have one thing going for it: it was much
> easier to understand at a high level.
>
> VACUUM no longer applies a separate mode of operation (aggressive mode).
> There are still antiwraparound autovacuums, but they're now little more
> than another way that autovacuum.c can launch an autovacuum worker to
> run VACUUM.

The most significant aspect of anti-wrap autvacuums right now is that they
don't auto-cancel. Is that still used? If so, what's the threshold?

IME one of the most common reasons for autovac not keeping up is that the
application occasionally acquires conflicting locks on one of the big
tables. Before reaching anti-wrap age all autovacuums on that table get
cancelled before it gets to update relfrozenxid. Once in that situation
autovac really focusses only on that relation...


> Now every VACUUM might need to wait for a cleanup lock, though few will.
> It can only happen when required to advance relfrozenxid to no less than
> half way between the existing relfrozenxid and nextXID.

Where's that "halfway" bit coming from?

Isn't "half way between the relfrozenxid and nextXID" a problem for instances
with longrunning transactions? Wouldn't this mean that wait for every page if
relfrozenxid can't be advanced much because of a longrunning query or such?



> From 51a863190f70c8baa6d04e3ffd06473843f3326d Mon Sep 17 00:00:00 2001
> From: Peter Geoghegan <pg@bowt.ie>
> Date: Sun, 31 Jul 2022 13:53:19 -0700
> Subject: [PATCH v6 5/6] Avoid allocating MultiXacts during VACUUM.
> 
> Pass down vacuumlazy.c's OldestXmin cutoff to FreezeMultiXactId(), and
> teach it the difference between OldestXmin and FreezeLimit.  Use this
> high-level context to intelligently avoid allocating new MultiXactIds
> during VACUUM operations.  We should always prefer to avoid allocating
> new MultiXacts during VACUUM on general principle.  VACUUM is the only
> mechanism that can claw back MultixactId space, so allowing VACUUM to
> consume MultiXactId space (for any reason) adds to the risk that the
> system will trigger the multiStopLimit wraparound protection mechanism.

Strictly speaking that's not quite true, you can also drop/truncate tables ;)


Greetings,

Andres Freund

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

19 November 2022, 04:06:57

On Tue, Nov 15, 2022 at 9:20 PM Andres Freund <andres@anarazel.de> wrote:
> > Subject: [PATCH v6 1/6] Add page-level freezing to VACUUM.

Attached is v7, which incorporates much of your feedback. Thanks for the review!

> > +/*
> > + * State used by VACUUM to track what the oldest extant XID/MXID will become
> > + * when determing whether and how to freeze a page's heap tuples via calls to
> > + * heap_prepare_freeze_tuple.
>
> Perhaps this could say something like "what the oldest extant XID/MXID
> currently is and what it would be if we decide to freeze the page" or such?

Fixed.

> > + * The relfrozenxid_out and relminmxid_out fields are the current target
> > + * relfrozenxid and relminmxid for VACUUM caller's heap rel.  Any and all
>
> "VACUUM caller's heap rel." could stand to be rephrased.

Fixed.

> > + * unfrozen XIDs or MXIDs that remain in caller's rel after VACUUM finishes
> > + * _must_ have values >= the final relfrozenxid/relminmxid values in pg_class.
> > + * This includes XIDs that remain as MultiXact members from any tuple's xmax.
> > + * Each heap_prepare_freeze_tuple call pushes back relfrozenxid_out and/or
> > + * relminmxid_out as needed to avoid unsafe values in rel's authoritative
> > + * pg_class tuple.
> > + *
> > + * Alternative "no freeze" variants of relfrozenxid_nofreeze_out and
> > + * relminmxid_nofreeze_out must also be maintained for !freeze pages.
> > + */
>
> relfrozenxid_nofreeze_out isn't really a "no freeze variant" :)

Why not? I think that that's exactly what it is. We maintain these
alternative "oldest extant XID" values so that vacuumlazy.c's
lazy_scan_prune function can "opt out" of freezing. This is exactly
the same as what we do in lazy_scan_noprune, both conceptually and at
the implementation level.

> I think it might be better to just always maintain the nofreeze state.

Not sure. Even if there is very little to gain in cycles by not
maintaining the "nofreeze" cutoffs needlessly, it's still a pure waste
of cycles that can easily be avoided. So it just feels natural to not
waste those cycles -- it may even make the design clearer.

> > +typedef struct HeapPageFreeze
> > +{
> > +     /* Is heap_prepare_freeze_tuple caller required to freeze page? */
> > +     bool            freeze;
>
> s/freeze/freeze_required/?

Fixed.

> Given the number of parameters to heap_prepare_freeze_tuple, why don't we pass
> in more of them in via HeapPageFreeze?

HeapPageFreeze is supposed to be mutable state used for one single
page, though. Seems like we should use a separate immutable struct for
this instead.

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.

> What does 'xtrack' stand for? Xid Tracking?

Yes.

> >   * VACUUM caller must assemble HeapFreezeTuple entries for every tuple that we
> >   * returned true for when called.  A later heap_freeze_execute_prepared call
> > - * will execute freezing for caller's page as a whole.
> > + * will execute freezing for caller's page as a whole.  Caller should also
> > + * initialize xtrack fields for page as a whole before calling here with first
> > + * tuple for the page.  See page_frozenxid_tracker comments.
>
> s/should/need to/?

Changed it to "must".

> page_frozenxid_tracker appears to be a dangling pointer.

I think that you mean that the code comments reference an obsolete
type name -- fixed.

> > +      * VACUUM calls limit_xid "FreezeLimit", and cutoff_xid "OldestXmin".
> > +      * (limit_multi is "MultiXactCutoff", and cutoff_multi "OldestMxact".)
>
> Hm. Perhaps we should just rename them if it requires this kind of
> explanation? They're really not good names.

Agreed -- this can be taken care of as part of using a new VACUUM
operation level struct that is passed as immutable state, which I went
into a moment ago. That centralizes the definitions, which makes it
far easier to understand which cutoff is which. For now I've kept the
names as they were.

> Could use TransactionIdOlder().

I suppose, but the way I've done it feels a bit more natural to me,
and appears more often elsewhere. Not sure.

> > @@ -6563,8 +6564,11 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple,
> >                        */
> >                       Assert(!freeze_xmax);
> >                       Assert(TransactionIdIsValid(newxmax));
> > -                     if (TransactionIdPrecedes(newxmax, *relfrozenxid_out))
> > -                             *relfrozenxid_out = newxmax;
> > +                     Assert(heap_tuple_would_freeze(tuple, limit_xid, limit_multi,
> > +
&xtrack->relfrozenxid_nofreeze_out,
> > +
&xtrack->relminmxid_nofreeze_out));
> > +                     if (TransactionIdPrecedes(newxmax, xtrack->relfrozenxid_out))
> > +                             xtrack->relfrozenxid_out = newxmax;
>
> Perhaps the Assert(heap_tuple_would_freeze()) bit could be handled once at the
> end of the routine, for all paths?

The problem with that is that we cannot Assert() when we're removing a
Multi via FRM_INVALIDATE_XMAX processing in certain cases (I tried it
this way myself, and the assertion fails there). This can happen when
the call to FreezeMultiXactId() for the xmax determined that we should
do FRM_INVALIDATE_XMAX processing for the xmax due to the Multi being
"isLockOnly" and preceding "OldestVisibleMXactId[MyBackendId])". Which
is relatively common.

I fixed this by moving the assert further down, while still only
checking the FRM_RETURN_IS_XID and FRM_RETURN_IS_MULTI cases.

> Oh - I totally didn't realize that ->freeze is an out parameter. Seems a bit
> odd to have the other fields suffied with _out but not this one?

Fixed this by not having an "_out" suffix for any of these mutable
fields from HeapPageFreeze. Now everything is consistent. (The "_out"
convention is totally redundant, now that we have the HeapPageFreeze
struct, which makes it obvious that it is is all mutable state.)

> Won't using OldestXmin instead of FreezeLimit potentially cause additional
> conflicts? Is there any reason to not compute an accurate value?

This is a concern that I share. I was hoping that I'd be able to get
away with using OldestXmin just for this, because it's simpler that
way. But I had my doubts about it already.

I wonder why it's correct to use FreezeLimit for this on HEAD, though.
What about those FRM_INVALIDATE_XMAX cases that I just mentioned we
couldn't Assert() on? That case effectively removes XIDs that might be
well after FreezeLimit. Granted it might be safe in practice, but it's
far from obvious why it is safe.

Perhaps we can fix this in a not-too-invasive way by reusing
LVPagePruneState.visibility_cutoff_xid for FREEZE_PAGE conflicts (not
just VISIBLE conflicts) in cases where that was possible (while still
using OldestXmin as a fallback in much rarer cases). In practice we're
only triggering freezing eagerly because the page is already expected
to be set all-visible (the whole point is that we'd prefer if it was
set all-frozen instead of all-visible).

(I've not done this in v7, but it's on my TODO list.)

Note that the patch already maintains
LVPagePruneState.visibility_cutoff_xid when there are some LP_DEAD
items on the page, because we temporarily ignore those LP_DEAD items
when considering the eager freezing stuff......

> >               if (ItemIdIsDead(itemid))
> >               {

> >                       deadoffsets[lpdead_items++] = offnum;
> > -                     prunestate->all_visible = false;
> > -                     prunestate->has_lpdead_items = true;
> >                       continue;
> >               }
>
> What does this have to do with the rest of the commit? And why are we doing
> this?

....which is what you're asking about here.

The eager freezing strategy triggers page-level freezing for any page
that is about to become all-visible, so that it can be set all-frozen
instead. But that's not entirely straightforward when there happens to
be some LP_DEAD items on the heap page. There are really two ways that
a page can become all-visible during VACUUM, and we want to account
for that here. With eager freezing we want to make the pages become
all-frozen instead of just all-visible, regardless of which heap pass
(first pass or second pass) the page is set to become all-visible (and
maybe even all-frozen).

The comments that you mention were moved around a bit in passing.

Note that we still set prunestate->all_visible to false inside
lazy_scan_prune when we see remaining LP_DEAD stub items. We just do
it later on, after we've decided on freezing stuff. (Obviously it
wouldn't be okay to return to lazy_scan_heap without unsetting
prunestate->all_visible if there are LP_DEAD items.)

> Seems quite confusing to enter a block with described as "We're freezing the
> page." when we're not freezing anything (tuples_frozen == 0).

> I don't really get what freeze_all_eligible is trying to do.

freeze_all_eligible (and the "tuples_frozen == 0" behavior) are both
there because we can mark a page as all-frozen in the VM without
freezing any of its tuples first. When that happens, we must make sure
that "prunestate->all_frozen" is set to true, so that we'll actually
set the all-frozen bit. At the same time, we need to be careful about
the case where we *could* set the page all-frozen if we decided to
freeze all eligible tuples -- we need to handle the case where we
choose against freezing (and so can't set the all-frozen bit in the
VM, and so must actually set "prunestate->all_frozen" to false).

This is all kinda tricky because we're simultaneously dealing with the
actual state of the page, and the anticipated state of the page in the
near future. Closely related concepts, but distinct in important ways.

> >  #ifdef USE_ASSERT_CHECKING
> >       /* Note that all_frozen value does not matter when !all_visible */
> > -     if (prunestate->all_visible)
> > +     if (prunestate->all_visible && lpdead_items == 0)
> >       {
> >               TransactionId cutoff;
> >               bool            all_frozen;
> > @@ -1849,8 +1876,7 @@ retry:
> >               if (!heap_page_is_all_visible(vacrel, buf, &cutoff, &all_frozen))
> >                       Assert(false);
>
> Not related to this change, but why isn't this just
> Assert(heap_page_is_all_visible(vacrel, buf, &cutoff, &all_frozen))?

It's just a matter of personal preference. I prefer to have a clear
block of related code that contains multiple related assertions. You
would probably have declared PG_USED_FOR_ASSERTS_ONLY variables at the
top of lazy_scan_prune instead. FWIW if you did it the other way the
assertion would actually have to include a "!prunestate->all_visible"
test that short circuits the heap_page_is_all_visible() call from the
Assert().

> > Subject: [PATCH v6 2/6] Teach VACUUM to use visibility map snapshot.

> This should include a description of the memory usage effects.

The visibilitymap.c side of this is the least worked out part of the
patch series, by far. I have deliberately put off work on the data
structure itself, preferring to focus on the vacuumlazy.c side of
things for the time being. But I still agree -- fixed by acknowledging
that that particular aspect of resource management is unresolved.

I did have an open TODO before in the commit message, which is now
improved based on your feedback: it now fully owns the fact that we
really ignore the impact on memory usage right now. Just because that
part is very WIP (much more so than every other part).

> > This has significant advantages over the previous approach of using the
> > authoritative VM fork to decide on which pages to skip.  The number of
> > heap pages processed will no longer increase when some other backend
> > concurrently modifies a skippable page, since VACUUM will continue to
> > see the page as skippable (which is correct because the page really is
> > still skippable "relative to VACUUM's OldestXmin cutoff").
>
> Why is it an advantage for the number of pages to not increase?

The commit message goes into that immediately after the last line that
you quoted.  :-)

Having an immutable structure will help us, both in the short term,
for this particular project, and the long term. for other VACUUM
enhancements.

We need to have something that drives the cost model in vacuumlazy.c
for the skipping strategy stuff -- we need to have advanced
information about costs that drive the decision making process. Thanks
to VM snapshots, the cost model is able to reason about the cost of
relfrozenxid advancement precisely, in terms of "extra" scanned_pages
implied by advancing relfrozenxid during this VACUUM. That level of
precision is pretty nice IMV. It's not strictly necessary, but it's
nice to be able to make a precise accurate comparison between each of
the two skipping strategies.

Did you happen to look at the 6th and final patch? It's trivial, but
can have a big impact. It sizes dead_items while capping its sized
based on scanned_pages, not based on rel_pages. That's obviously
guaranteed to be correct.  Note also that the 2nd patch teaches VACUUM
VERBOSE to report the final number of scanned_pages right at the
start, before scanning anything -- so it's a useful basis for much
better progress reporting in pg_stat_progress_vacuum. Stuff like that
also becomes very easy with VM snapshots.

Then there is the more ambitious stuff, that's not in scope for this
project. Example: Perhaps Sawada san will be able to take the concept
of visibility map snapshots, and combine it with his Radix tree design
-- which could presumably benefit from advanced knowledge of which
pages can be scanned. This is information that is reliable, by
definition. In fact I think that it would make a lot of sense for this
visibility map snapshot data structure to be exactly the same
structure used to store dead_items. They really are kind of the same
thing. The design can reason precisely about which heap pages can ever
end up having any LP_DEAD items. (It's already trivial to use the VM
snapshot infrastructure as a precheck cache for dead_items lookups.)

> > Non-aggressive VACUUMs now explicitly commit to (or decide against)
> > early relfrozenxid advancement up-front.
>
> Why?

We can advance relfrozenxid because it's cheap to, or because it's
urgent (according to autovacuum_freeze_max_age). This is kind of true
on HEAD already due to the autovacuum_freeze_max_age "escalate to
aggressive" thing -- but we can do much better than that. Why not
decide to advance relfrozenxid when (say) it's only *starting* to get
urgent when it happens to be relatively cheap (though not dirt cheap)?
We make relfrozenxid advancement a deliberate decision that weighs
*all* available information, and has a sense of the needs of the table
over time.

The user experience is important here. Going back to a model where
there is really just one kind of lazy VACUUM makes a lot of sense. We
should have much more approximate guarantees about relfrozenxid
advancement, since that's what gives us the flexibility to find a
cheaper (or more stable) way of keeping up over time. It matters that
we keep up over time, but it doesn't matter if we fall behind on
relfrozenxid advancement -- at least not if we don't also fall behind
on the work of freezing physical heap pages.

> > VACUUM will now either scan
> > every all-visible page, or none at all.  This replaces lazy_scan_skip's
> > SKIP_PAGES_THRESHOLD behavior, which was intended to enable early
> > relfrozenxid advancement (see commit bf136cf6), but left many of the
> > details to chance.
>
> The main goal according to bf136cf6 was to avoid defeating OS readahead, so I
> think it should be mentioned here.

Agreed. Fixed.

> To me this is something that ought to be changed separately from the rest of
> this commit.

Maybe, but I'd say it depends on the final approach taken -- the
visibilitymap.c aspects of the patch are the least settled. I am
seriously considering adding prefetching to the vm snapshot structure,
which would make it very much a direct replacement for
SKIP_PAGES_THRESHOLD.

Separately, I'm curious about what you think of VM snapshots from an
aio point of view. Seems like it would be ideal for prefetching for
aio?

> > TODO: We don't spill VM snapshots to disk just yet (resource management
> > aspects of VM snapshots still need work).  For now a VM snapshot is just
> > a copy of the VM pages stored in local buffers allocated by palloc().
>
> HEAPBLOCKS_PER_PAGE is 32672 with the defaults. The maximum relation size is
> 2**32 - 1 blocks. So the max FSM size is 131458 pages, a bit more than 1GB. Is
> that correct?

I think that you meant "max VM size". That sounds correct to me.

> For large relations that are already nearly all-frozen this does add a
> noticable amount of overhead, whether spilled to disk or not. Of course
> they're also not going to be vacuumed super often, but ...

I wouldn't be surprised if the patch didn't work with relations that
approach 32 TiB in size. As I said, the visibilitymap.c data structure
is the least worked out piece of the project.

> Perhaps worth turning the VM into a range based description for the snapshot,
> given it's a readonly datastructure in local memory? And we don't necessarily
> need the all-frozen and all-visible in memory, one should suffice? We don't
> even need random access, so it could easily be allocated incrementally, rather
> than one large allocation.

Definitely think that we should do simple run-length encoding, stuff
like that. Just as long as it allows vacuumlazy.c to work off of a
true snapshot, with scanned_pages known right from the start. The
consumer side of things has been my focus so far.

> Hm. It's a bit sad to compute the snapshot after determining OldestXmin.
>
> We probably should refresh OldestXmin periodically. That won't allow us to get
> a more aggressive relfrozenxid, but it'd allow to remove more gunk.

That may well be a good idea, but I think that it's also a good idea
to just not scan heap pages that we know won't have XIDs < OldestXmin
(OldestXmin at the start of the VACUUM). That visibly makes the
problem of "recently dead" tuples that cannot be cleaned up a lot
better, without requiring that we do anything with OldestXmin.

I also think that there is something to be said for not updating the
FSM for pages that were all-visible at the beginning of the VACUUM
operation. VACUUM is currently quite happy to update the FSM with its
own confused idea about how much free space there really is on heap
pages with recently dead (dead but not yet removable) tuples. That's
really bad, but really subtle.

> What does it mean to "skip lazily"?

Skipping even all-visible pages, prioritizing avoiding work over
advancing relfrozenxid. This is a cost-based decision. As I mentioned
a moment ago, that's one immediate use of VM snapshots (it gives us
precise information to base our decision on, that simply *cannot*
become invalid later on).

> > +             /*
> > +              * Visibility map page copied to local buffer for caller's snapshot.
> > +              * Caller requires an exact count of all-visible and all-frozen blocks
> > +              * in the heap relation.  Handle that now.
>
> This part of the comment seems like it actually belongs further down?

No, it just looks a bit like that because of the "truncate in-memory
VM" code stanza. It's actually the right order.

> > +              * Must "truncate" our local copy of the VM to avoid incorrectly
> > +              * counting heap pages >= rel_pages as all-visible/all-frozen.  Handle
> > +              * this by clearing irrelevant bits on the last VM page copied.
> > +              */
>
> Hm - why would those bits already be set?

No real reason, we "truncate" like this defensively. This will
probably look quite different before too long.

> > Subject: [PATCH v6 3/6] Add eager freezing strategy to VACUUM.

> What's the logic behind a hard threshold?  Suddenly freezing everything on a
> huge relation seems problematic. I realize that never getting all that far
> behind is part of the theory, but I don't think that's always going to work.

It's a vast improvement on what we do currently, especially in
append-only tables.

There is simply no limit on how many physical heap pages will have to
be frozen when there is an aggressive mode VACUUM. It could be
terabytes, since table age predicts precisely nothing about costs.
With the patch we have a useful limit for the first time, that uses
physical units (the only kind of units that make any sense).

Admittedly we should really have special instrumentation that reports
when VACUUM must do "catch up freezing" when the
vacuum_freeze_strategy_threshold threshold is first crossed, to help
users to make better choices in this area. And maybe
vacuum_freeze_strategy_threshold should be lower by default, so it's
not as noticeable. (The GUC partly exists as a compatibility option, a
bridge to the old lazy behavior.)

Freezing just became approximately 5x cheaper with the freeze plan
deduplication work (commit 9e540599). To say nothing about how
vacuuming indexes became a lot cheaper in recent releases. So to some
extent we can afford to be more proactive here. There are some very
nonlinear cost profiles involved here due to write amplification
effects. So having a strong bias against write amplification seems
totally reasonable to me -- we can potentially "get it wrong" and
still come out ahead, because we at least had the right idea about
costs.

I don't deny that there are clear downsides, though. I am convinced
that it's worth it -- performance stability is what users actually
complain about in almost all cases. Why should performance stability
be 100% free?

> Wouldn't a better strategy be to freeze a percentage of the relation on every
> non-aggressive vacuum? That way the amount of work for an eventual aggressive
> vacuum will shrink, without causing individual vacuums to take extremely long.

I think that it's better to avoid aggressive mode altogether. By
committing to advancing relfrozenxid by *some* amount in ~all VACUUMs
against larger tables, we can notice when we don't actually need to do
very much freezing to keep relfrozenxid current, due to workload
characteristics. It depends on workload, of course. But if we don't
try to do this we'll never notice that it's possible to do it.

Why should we necessarily need to freeze very much, after a while? Why
shouldn't most newly frozen pages stay frozen ~forever after a little
while?

> The other thing that I think would be to good to use is a) whether the page is
> already in s_b, and b) whether the page already is dirty. The cost of freezing
> shrinks significantly if it doesn't cause an additional read + write. And that
> additional IO IMO one of the major concerns with freezing much more
> aggressively in OLTPish workloads where a lot of the rows won't ever get old
> enough to need freezing.

Maybe, but I think that systematic effects are more important. We
freeze eagerly during this VACUUM in part because it makes
relfrozenxid advancement possible in the next VACUUM.

Note that eager freezing doesn't freeze the page unless it's already
going to set it all-visible. That's another way in which we ameliorate
the problem of freezing when it makes little sense to -- even with
eager freezing strategy, we *don't* freeze heap pages where it
obviously makes little sense to. Which makes a huge difference on its
own.

There is good reason to believe that most individual heap pages are
very cold data, even in OLTP apps. To a large degree Postgres is
successful because it is good at inexpensively storing data that will
possibly never be accessed:

https://www.microsoft.com/en-us/research/video/cost-performance-in-modern-data-stores-how-data-cashing-systems-succeed/

Speaking of OLTP apps:

in many cases VACUUM will prune just to remove one or two heap-only
tuples, maybe even generating an FPI in the process. But the removed
tuple wasn't actually doing any harm -- an opportunistic prune could
have done the same thing later on, once we'd built up some more
garbage tuples. So the only reason to prune is to freeze the page. And
yet right now we don't recognize this and freeze the page to get
*some* benefit out of the arguably needlessly prune. This is quite
common, in fact.

> The most significant aspect of anti-wrap autvacuums right now is that they
> don't auto-cancel. Is that still used? If so, what's the threshold?

This patch set doesn't change anything about antiwraparound
autovacuums -- though it does completely eliminate aggressive mode (so
it's a little like Postgres 8.4).

There is a separate thread discussing the antiwraparound side of this, actually:

https://postgr.es/m/CAH2-Wz=S-R_2rO49Hm94Nuvhu9_twRGbTm6uwDRmRu-Sqn_t3w@mail.gmail.com

I think that I will need to invent a new type of autovacuum that's
similar to antiwraparound autovacuum, but doesn't have the
cancellation behavior -- that is more or less prerequisite to
committing this patch series. We can accept some risk of relfrozenxid
falling behind if that doesn't create any real risk of antiwraparound
autovacuums.

We can retain antiwraparound autovacuum, which should kick in only
when the new kind of autovacuum has failed to advance relfrozenxid,
having had the opportunity. Maybe antiwraparound autovacuum should be
triggered when age(relfrozenxid) is twice the value of
autovacuum_freeze_max_age. The new kind of autovacuum would trigger at
the same table age that triggers antiwraparound autovacuum with the
current design.

So antiwraparound autovacuum would work in the same way, but would be
much less common -- even for totally static tables. We'd at least be
sure that the auto cancellation behavior was *proportionate* to the
problem at hand, because we'll always have tried and ultimately failed
to advance relfrozenxid without activating the auto cancellation
behavior. We wouldn't trigger a very disruptive behavior routinely,
without any very good reason.

> > Now every VACUUM might need to wait for a cleanup lock, though few will.
> > It can only happen when required to advance relfrozenxid to no less than
> > half way between the existing relfrozenxid and nextXID.
>
> Where's that "halfway" bit coming from?

We don't use FreezeLimit within lazy_scan_noprune in the patch that
gets rid of aggressive mode VACUUM. We use something called minXid in
its place. So a different timeline to freezing (even for tables where
we always use the lazy freezing strategy).

The new minXid cutoff (used by lazy_scan_noprune) comes from this
point in vacuum_set_xid_limits():

+ *minXid = nextXID - (freeze_table_age / 2);
+ if (!TransactionIdIsNormal(*minXid))
+     *minXid = FirstNormalTransactionId;

So that's what I meant by "half way".

(Note that minXid is guaranteed to be <= FreezeLimit, which is itself
guaranteed to be <= OldestXmin, no matter what.)

> Isn't "half way between the relfrozenxid and nextXID" a problem for instances
> with longrunning transactions?

Should we do less relfrozenxid advancement because there is a long
running transaction, though? It's obviously seriously bad when things
are blocked by a long running transaction, but I don't see the
connection between that and how we wait for cleanup locks. Waiting for
cleanup locks is always really, really bad, and can be avoided in
almost all cases.

I suspect that I still haven't been aggressive enough in how minXid is
set, BTW -- we should be avoiding waiting for a cleanup lock like the
plague. So "half way" isn't enough. Maybe we should have a LOG message
in cases where it actually proves necessary to wait, because it's just
asking for trouble (at least when we're running in autovacuum).

> Wouldn't this mean that wait for every page if
> relfrozenxid can't be advanced much because of a longrunning query or such?

Old XIDs always start out as young XIDs. Which we're now quite willing
to freeze when conditions look good.

Page level freezing always freezes all eligible XIDs on the page when
triggered, no matter what the details may be. This means that the
oldest XID on a heap page is more or less always an XID that's after
whatever OldestXmin was for the last VACUUM that ran and froze the
page, whenever that happened, and regardless of the mix of XID ages
was on the page at that time.

As a consequence, lone XIDs that are far older than other XIDs on the
same page become much rarer than what you'd see with the current
design -- they have to "survive" multiple VACUUMs, not just one
VACUUM. The best predictor of XID age becomes the time that VACUUM
last froze the page as a whole -- so workload characteristics and
natural variations are much much less likely to lead to problems from
waiting for cleanup locks. (Of course it also helps that we'll try
really hard to do that, and almost always prefer lazy_scan_noprune
processing.)

There is some sense in which we're trying to create a virtuous cycle
here. If we are always in a position to advance relfrozenxid by *some*
amount each VACUUM, however small, then we will have many individual
opportunities (spaced out over multiple VACUUM operations) to freeze
tuples on any heap tuples that (for whatever reason) are harder to get
a cleanup lock on, and then catch up on relfrozenxid by a huge amount
whenever we "get lucky". We have to "keep an open mind" to ever have
any chance of "getting lucky" in this sense, though.

> > VACUUM is the only
> > mechanism that can claw back MultixactId space, so allowing VACUUM to
> > consume MultiXactId space (for any reason) adds to the risk that the
> > system will trigger the multiStopLimit wraparound protection mechanism.
>
> Strictly speaking that's not quite true, you can also drop/truncate tables ;)

Fixed.

--
Peter Geoghegan

Attachment

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

24 November 2022, 02:06:52

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

On Tue, Dec 6, 2022 at 1:45 PM Peter Geoghegan <pg@bowt.ie> wrote:
> v9 will also address some of the concerns you raised in your review
> that weren't covered by v8, especially about the VM snapshotting
> infrastructure. But also your concerns about the transition from lazy
> strategies to eager strategies.

Attached is v9. Highlights:

* VM snapshot infrastructure now spills using temp files when required
(only in larger tables).

v9 is the first version that has a credible approach to resource
management, which was something I put off until recently. We only use
a fixed amount of memory now, which should be acceptable from the
viewpoint of VACUUM resource management. The temp files use the
BufFile infrastructure in a relatively straightforward way.

* VM snapshot infrastructure now uses explicit prefetching.

Our approach is straightforward, and perhaps even obvious: we prefetch
at the point that VACUUM requests the next block in line. There is a
configurable prefetch distance, controlled by
maintenance_io_concurrency. We "stage" a couple of thousand
BlockNumbers in VACUUM's vmsnap by bulk-reading from the vmsnap's
local copy of the visibility map -- these staged blocks are returned
to VACUUM to scan, with interlaced prefetching of later blocks from
the same local BlockNumber array.

The addition of prefetching ought to be enough to avoid regressions
that might otherwise result from the removal of SKIP_PAGES_THRESHOLD
from vacuumlazy.c (see commit bf136cf6 from around the time the
visibility map first went in for the full context). While I definitely
need to do more performance validation work around prefetching
(especially on high latency network-attached storage), I imagine that
it won't be too hard to get into shape for commit. It's certainly not
committable yet, but it's vastly better than v8.

The visibility map snapshot interface (presented by visibilitymap.h)
also changed in v9, mostly to support prefetching. We now have an
iterator style interface (so vacuumlazy.c cannot request random
access). This iterator interface is implemented by visibilitymap.c
using logic similar to the current lazy_scan_skip() logic from
vacuumlazy.c (which is gone).

All told, visibilitymap.c knows quite a bit more than it used to about
high level requirements from vacuumlazy.c. For example it has explicit
awareness of VM skipping strategies.

* Page-level freezing commit now freezes a page whenever VACUUM
detects that pruning ran and generated an FPI.

Following a suggestion by Andres, page-level freezing is now always
triggered when pruning needs an FPI. Note that this optimization gets
applied regardless of freezing strategy (unless you turn off
full_page_writes, I suppose).

This optimization is added by the second patch
(v9-0002-Add-page-level-freezing-to-VACUUM.patch).

* Fixed the doc build.

* Much improved criteria for deciding on freezing and vmsnap skipping
strategies in vacuumlazy.c lazy_scan_strategy function -- improved
"cost model".

VACUUM should now give users a far smoother "transition" from lazy
processing to eager processing. A table that starts out small (smaller
than vacuum_freeze_strategy_threshold), but gradually grows, and
eventually becomes fairly large (perhaps to a multiple of
vacuum_freeze_strategy_threshold in size) will now experience a far
more gradual transition, with catch-up freezing spread out multiple
VACUUM operations. We avoid big jumps in the overhead of freezing,
where one particular VACUUM operation does all required "catch-up
freezing" in one go.

My approach is to "stagger" the timeline for switching freezing
strategy and vmsnap skipping strategy. We now change over from lazy to
eager freezing strategy when the table size threshold (controlled by
vacuum_freeze_strategy_threshold) is first crossed, just like in v8.
But unlike v8, v9 will switch over to eager skipping in some later
VACUUM operation (barring edge cases). This is implemented in a fairly
simple way: we now apply a "separate" threshold that is based on
vacuum_freeze_strategy_threshold: a threshold that's *twice* the
current value of the vacuum_freeze_strategy_threshold GUC/reloption
threshold.

My approach of "staggering" multiple distinct behaviors to avoid
having them all kick in during the same VACUUM operation isn't new to
v9. The behavior around waiting for cleanup locks (added by
v9-0005-Finish-removing-aggressive-mode-VACUUM.patch) is another
example of the same general idea.

In general I think that VACUUM shouldn't switch to more aggressive
behaviors all at the same time, in the same VACUUM. Each distinct
aggressive behavior has totally different properties, so there is no
reason why VACUUM should start to apply each and every one of them at
the same time. Some "aggressive" behaviors have the potential to make
things quite a lot worse, in fact. The cure must not be worse than the
disease.

* Related to the previous item (about the "cost model" that chooses a
strategy), we now have a much more sophisticated approach when it
comes to when and how we decide to advance relfrozenxid in smaller
tables (tables whose size is < vacuum_freeze_strategy_threshold). This
improves things for tables that start out small, and stay small.
Tables where we're unlikely to want to advance relfrozenxid in every
single VACUUM (better to be lazy with such a table), but still want to
be clever about advancing relfrozenxid "opportunistically".

The way that VACUUM weighs both table age and the added cost of
relfrozenxid advancement is more sophisticated in v9. The goal is to
make it more likely that VACUUM will stumble upon opportunities to
advance relfrozenxid when it happens to be cheap, which can happen for
many reasons. All of which have a great deal to do with workload
characteristics.

As in v8, v9 makes VACUUM willing to advance relfrozenxid without
concern for table age, whenever it notices that the cost of doing so
happens to be very cheap (in practice this means that the number of
"extra" heap pages scanned is < 5% of rel_pages). However, in v9 we
now go further by scaling this threshold through interpolation, based
on table age.

We have the same "5% of rel_pages" threshold when table age is less
than half way towards the point that autovacuum.c will launch an
antiwraparound autovacuum -- when we still have only minimal concern
about table age. But the rel_pages-wise threshold starts to grow once
table age gets past that "half way towards antiwrap AV" point. We
interpolate the rel_pages-wise threshold using a new approach in v9.

At first the rel_pages-wise threshold grows quite slowly (relative to
the rate at which table age approaches the point of forcing an
antiwraparound AV). For example, when we're 60% of the way towards
needing an antiwraparound AV, and VACUUM runs, we'll eagerly advance
relfrozenxid provided that the "extra" cost of doing so happens to be
less than ~22% of rel_pages. It "accelerates" from there (assuming
fixed rel_pages).

VACUUM will now tend to take advantage of individual table
characteristics that make it relatively cheap to advance relfrozenxid.
Bear in mind that these characteristics are not fixed for the same
table. The "extra" cost of advancing relfrozenxid during this VACUUM
(whether measured in absolute terms, of as a proportion of the net
amount of work just to do simple vacuuming) just isn't predictable
with real workloads. Especially not with the FPI opportunistic
freezing stuff from the second patch (the "freeze when heap pruning
gets an FPI" thing) in place. We should expect significant "natural
variation" among tables, and within the same table over time -- this
is a good thing.

For example, imagine a table that experiences a bunch of random
deletes, which leads to a  VACUUM that must visit most heap pages (say
85% of rel_pages). Let's suppose that those deletes are a once-off
thing. The added cost of advancing relfrozenxid in the next VACUUM
still isn't trivial (assuming the remaining 15% of pages are
all-visible). But it is probably still worth doing if table age is at
least starting to become a concern. It might actually be a lot cheaper
to advance relfrozenxid early.

* Numerous structural improvements, lots of code polishing.

The patches have been reordered in a way that should make review a bit
easier. Now the commit messages are written in a way that clearly
anticipates the removal of aggressive mode VACUUM, which the last
patch actually finishes. Most of the earlier commits are presented as
preparation for completely removing aggressive mode VACUUM.

The first patch (which refactors how VACUUM passes around cutoffs like
FreezeLimit and OldestXmin by using a dedicated struct) is much
improved. heap_prepare_freeze_tuple() now takes a more explicit
approach to tracking what needs to happen for the tuple's freeze plan.
This allowed me to pepper it with defensive assertions. It's also a
lot clearer IMV. For example, we now have separate freeze_xmax and
replace_xmax tracker variables.

The second patch in the series (the page-level freezing patch) is also
much improved. I'm much happier with the way that
heap_prepare_freeze_tuple() now explicitly delegates control of
page-level freezing to FreezeMultiXactId() in v9, for example.

Note that I squashed the patch that taught VACUUM to size dead_items
using scanned_pages into the main visibility map patch
(v9-0004-Add-eager-and-lazy-VM-strategies-to-VACUUM.patch). That's why
there are only 5 patches (down from 6) in v9.

--
Peter Geoghegan

On Tue, Dec 13, 2022 at 12:29 AM John Naylor
<john.naylor@enterprisedb.com> wrote:
> If the number of unfrozen heap pages is the thing we care about, perhaps that, and not the total size of the table,
shouldbe the parameter that drives freezing strategy?

That's not the only thing we care about, though. And to the extent we
care about it, we mostly care about the consequences of either
freezing or not freezing eagerly. Concentration of unfrozen pages in
one particular table is a lot more of a concern than the same number
of heap pages being spread out across multiple tables. Those tables
can all be independently vacuumed, and come with their own
relfrozenxid, that can be advanced independently, and are very likely
to be frozen as part of a vacuum that needed to happen anyway.

Pages become frozen pages because VACUUM freezes those pages. Same
with all-visible pages, which could in principle have been made
all-frozen instead, had VACUUM opted to do it that way back when it
processed the page. So VACUUM is not a passive, neutral observer here.
What happens over time and across multiple VACUUM operations is very
relevant. VACUUM needs to pick up where it left off last time, at
least with larger tables, where the time between VACUUMs is naturally
very high, and where each individual VACUUM has to process a huge
number of individual pages. It's not really practical to take a "wait
and see" approach with big tables.

At the very least, a given VACUUM operation has to choose its freezing
strategy based on how it expects the table will look when it's done
vacuuming the table, and how that will impact the next VACUUM against
the same table. Without that, then vacuuming an append-only table will
fall into a pattern of setting pages all-visible in one vacuum, and
then freezing those same pages all-frozen in the very next vacuum
because there are too many. Which makes little sense; we're far better
off freezing the pages at the earliest opportunity instead.

We're going to have to write a WAL record for the visibility map
anyway, so doing everything at the same time has a lot to recommend
it. Even if it turns out to be quite wrong, we may still come out
ahead in terms of absolute volume of WAL written, and especially in
terms of performance stability. To a limited extent we need to reason
about what will happen in the near future. But we also need to reason
about which kinds of mispredictions we cannot afford to make, and
which kinds are okay. Some mistakes hurt a lot more than others.

--
Peter Geoghegan

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

14 December 2022, 02:07:10

On Tue, Dec 13, 2022 at 9:16 AM Peter Geoghegan <pg@bowt.ie> wrote:
> That's not the only thing we care about, though. And to the extent we
> care about it, we mostly care about the consequences of either
> freezing or not freezing eagerly. Concentration of unfrozen pages in
> one particular table is a lot more of a concern than the same number
> of heap pages being spread out across multiple tables. Those tables
> can all be independently vacuumed, and come with their own
> relfrozenxid, that can be advanced independently, and are very likely
> to be frozen as part of a vacuum that needed to happen anyway.

At the suggestion of Jeff, I wrote a Wiki page that shows motivating
examples for the patch series:

https://wiki.postgresql.org/wiki/Freezing/skipping_strategies_patch:_motivating_examples

These are all cases where VACUUM currently doesn't do the right thing
around freezing, in a way that is greatly ameliorated by the patch.
Perhaps this will help other hackers to understand the motivation
behind some of these mechanisms. There are plenty of details that only
make sense in the context of a certain kind of table, with certain
performance characteristics that the design is sensitive to, and seeks
to take advantage of in one way or another.

-- 
Peter Geoghegan

Re: New strategies for freezing, advancing relfrozenxid early

From

Matthias van de Meent

Date:

15 December 2022, 17:50:19

On Wed, 14 Dec 2022 at 00:07, Peter Geoghegan <pg@bowt.ie> wrote:
>
> On Tue, Dec 13, 2022 at 9:16 AM Peter Geoghegan <pg@bowt.ie> wrote:
> > That's not the only thing we care about, though. And to the extent we
> > care about it, we mostly care about the consequences of either
> > freezing or not freezing eagerly. Concentration of unfrozen pages in
> > one particular table is a lot more of a concern than the same number
> > of heap pages being spread out across multiple tables. Those tables
> > can all be independently vacuumed, and come with their own
> > relfrozenxid, that can be advanced independently, and are very likely
> > to be frozen as part of a vacuum that needed to happen anyway.
>
> At the suggestion of Jeff, I wrote a Wiki page that shows motivating
> examples for the patch series:
>
> https://wiki.postgresql.org/wiki/Freezing/skipping_strategies_patch:_motivating_examples
>
> These are all cases where VACUUM currently doesn't do the right thing
> around freezing, in a way that is greatly ameliorated by the patch.
> Perhaps this will help other hackers to understand the motivation
> behind some of these mechanisms. There are plenty of details that only
> make sense in the context of a certain kind of table, with certain
> performance characteristics that the design is sensitive to, and seeks
> to take advantage of in one way or another.

In this mentioned wiki page, section "Simple append-only", the
following is written:

> Our "transition from lazy to eager strategies" concludes with an autovacuum that actually advanced relfrozenxid
eagerly:
>> automatic vacuum of table "regression.public.pgbench_history": index scans: 0
>> pages: 0 removed, 1078444 remain, 561143 scanned (52.03% of total)
>> [...]
>> frozen: 560841 pages from table (52.00% of total) had 88051825 tuples frozen
>> [...]
>> WAL usage: 1121683 records, 557662 full page images, 4632208091 bytes

I think that this 'transition from lazy to eager' could benefit from a
limit on how many all_visible blocks each autovacuum iteration can
freeze. This first run of (auto)vacuum after the 8GB threshold seems
to appear as a significant IO event (both in WAL and relation
read/write traffic) with 50% of the table updated and WAL-logged. I
think this should be limited to some degree, such as only freeze
all_visible blocks up to 10% of the table's blocks in eager vacuum, so
that the load is spread across a larger time frame and more VACUUM
runs.


Kind regards,

Matthias van de Meent.

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

15 December 2022, 21:53:04

On Thu, Dec 15, 2022 at 6:50 AM Matthias van de Meent
<boekewurm+postgres@gmail.com> wrote:
> This first run of (auto)vacuum after the 8GB threshold seems
> to appear as a significant IO event (both in WAL and relation
> read/write traffic) with 50% of the table updated and WAL-logged. I
> think this should be limited to some degree, such as only freeze
> all_visible blocks up to 10% of the table's blocks in eager vacuum, so
> that the load is spread across a larger time frame and more VACUUM
> runs.

I agree that the burden of catch-up freezing is excessive here (in
fact I already wrote something to that effect on the wiki page). The
likely solution can be simple enough.

In v9 of the patch, we switch over to eager freezing when table size
crosses 4GB (since that is the value of the
vacuum_freeze_strategy_threshold GUC). The catch up freezing that you
draw attention to here occurs when table size exceeds 8GB, which is a
separate physical table size threshold that forces eager relfrozenxid
advancement. The second threshold is hard-coded to 2x the first one.

I think that this issue can be addressed by making the second
threshold 4x or even 8x vacuum_freeze_strategy_threshold, not just 2x.
That would mean that we'd have to freeze just as many pages whenever
we did the catch-up freezing -- so no change in the added *absolute*
cost of freezing. But, the *relative* cost would be much lower, simply
because catch-up freezing would take place when the table was much
larger. So it would be a lot less noticeable.

Note that we might never reach the second table size threshold before
we must advance relfrozenxid, in any case. The catch-up freezing might
actually take place because table age created pressure to advance
relfrozenxid. It's useful to have a purely physical/table-size
threshold like this, especially in bulk loading scenarios. But it's
not like table age doesn't have any influence at all, anymore. The
cost model weighs physical units/costs as well as table age, and in
general the most likely trigger for advancing relfrozenxid is usually
some combination of the two, not any one factor on its own [1].

[1]
https://wiki.postgresql.org/wiki/Freezing/skipping_strategies_patch:_motivating_examples#Opportunistically_advancing_relfrozenxid_with_bursty.2C_real-world_workloads
-- 
Peter Geoghegan

Re: New strategies for freezing, advancing relfrozenxid early

From

Justin Pryzby

Date:

15 December 2022, 22:11:14

The patches (003 and 005) are missing a word
should use to decide whether to its eager freezing strategy.

On the wiki, missing a word:
builds on related added

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

16 December 2022, 07:13:07

On Thu, Dec 15, 2022 at 11:11 AM Justin Pryzby <pryzby@telsasoft.com> wrote:
> The patches (003 and 005) are missing a word
> should use to decide whether to its eager freezing strategy.

I mangled this during rebasing for v9, which reordered the commits.
Will be fixed in v10.

> On the wiki, missing a word:
> builds on related added

Fixed.

Thanks
-- 
Peter Geoghegan

Re: New strategies for freezing, advancing relfrozenxid early

From

John Naylor

Date:

16 December 2022, 10:48:17

On Wed, Dec 14, 2022 at 6:07 AM Peter Geoghegan <pg@bowt.ie> wrote:
>
> At the suggestion of Jeff, I wrote a Wiki page that shows motivating
> examples for the patch series:
>
> https://wiki.postgresql.org/wiki/Freezing/skipping_strategies_patch:_motivating_examples
>
> These are all cases where VACUUM currently doesn't do the right thing
> around freezing, in a way that is greatly ameliorated by the patch.
> Perhaps this will help other hackers to understand the motivation
> behind some of these mechanisms. There are plenty of details that only
> make sense in the context of a certain kind of table, with certain
> performance characteristics that the design is sensitive to, and seeks
> to take advantage of in one way or another.

Thanks for this. This is the kind of concrete, data-based evidence that I find much more convincing, or at least easy to reason about. I'd actually recommend in the future to open discussion with this kind of analysis -- even before coding, it's possible to indicate what a design is *intended* to achieve. And reviewers can likewise bring up cases of their own in a concrete fashion.

On Wed, Dec 14, 2022 at 12:16 AM Peter Geoghegan <pg@bowt.ie> wrote:

> At the very least, a given VACUUM operation has to choose its freezing
> strategy based on how it expects the table will look when it's done
> vacuuming the table, and how that will impact the next VACUUM against
> the same table. Without that, then vacuuming an append-only table will
> fall into a pattern of setting pages all-visible in one vacuum, and
> then freezing those same pages all-frozen in the very next vacuum
> because there are too many. Which makes little sense; we're far better
> off freezing the pages at the earliest opportunity instead.

That makes sense, but I wonder if we can actually be more specific: One motivating example mentioned is the append-only table. If we detected that case, which I assume we can because autovacuum_vacuum_insert_* GUCs exist, we could use that information as one way to drive eager freezing independently of size. At least in theory -- it's very possible size will be a necessary part of the decision, but it's less clear that it's as useful as a user-tunable knob.

If we then ignored the append-only case when evaluating a freezing policy, maybe other ideas will fall out. I don't have a well-thought out idea about policy or knobs, but it's worth thinking about.

Aside from that, I've only given the patches a brief reading. Having seen the VM snapshot in practice (under "Scanned pages, visibility map snapshot" in the wiki page), it's neat to see fewer pages being scanned. Prefetching not only seems superior to SKIP_PAGES_THRESHOLD, but anticipates asynchronous IO. Keeping only one VM snapshot page in memory makes perfect sense.

I do have a cosmetic, but broad-reaching, nitpick about terms regarding "skipping strategy". That's phrased as a kind of negative -- what we're *not* doing. Many times I had to pause and compute in my head what we're *doing*, i.e. the "scanning strategy". For example, I wonder if the VM strategies would be easier to read as:

VMSNAP_SKIP_ALL_VISIBLE -> VMSNAP_SCAN_LAZY
VMSNAP_SKIP_ALL_FROZEN -> VMSNAP_SCAN_EAGER
VMSNAP_SKIP_NONE -> VMSNAP_SCAN_ALL

Notice here they're listed in order of increasing eagerness.

--
John Naylor
EDB: http://www.enterprisedb.com

Re: New strategies for freezing, advancing relfrozenxid early

From

Nikita Malakhov

Date:

16 December 2022, 10:59:39

Hi!

I've found this discussion very interesting, in view of vacuuming

TOAST tables is always a problem because these tables tend to

bloat very quickly with dead data - just to remind, all TOAST-able

columns of the relation use the same TOAST table which is one

for the relation, and TOASTed data are not updated - there are

only insert and delete operations.

Have you tested it with large and constantly used TOAST tables?

How would it work with the current TOAST implementation?

We propose a different approach to the TOAST mechanics [1],

and a new vacuum would be very promising.

Thank you!

[1] https://commitfest.postgresql.org/41/3490/

On Fri, Dec 16, 2022 at 10:48 AM John Naylor <john.naylor@enterprisedb.com> wrote:

On Wed, Dec 14, 2022 at 6:07 AM Peter Geoghegan <pg@bowt.ie> wrote:
>
> At the suggestion of Jeff, I wrote a Wiki page that shows motivating
> examples for the patch series:
>
> https://wiki.postgresql.org/wiki/Freezing/skipping_strategies_patch:_motivating_examples
>
> These are all cases where VACUUM currently doesn't do the right thing
> around freezing, in a way that is greatly ameliorated by the patch.
> Perhaps this will help other hackers to understand the motivation
> behind some of these mechanisms. There are plenty of details that only
> make sense in the context of a certain kind of table, with certain
> performance characteristics that the design is sensitive to, and seeks
> to take advantage of in one way or another.

Thanks for this. This is the kind of concrete, data-based evidence that I find much more convincing, or at least easy to reason about. I'd actually recommend in the future to open discussion with this kind of analysis -- even before coding, it's possible to indicate what a design is *intended* to achieve. And reviewers can likewise bring up cases of their own in a concrete fashion.

On Wed, Dec 14, 2022 at 12:16 AM Peter Geoghegan <pg@bowt.ie> wrote:

> At the very least, a given VACUUM operation has to choose its freezing
> strategy based on how it expects the table will look when it's done
> vacuuming the table, and how that will impact the next VACUUM against
> the same table. Without that, then vacuuming an append-only table will
> fall into a pattern of setting pages all-visible in one vacuum, and
> then freezing those same pages all-frozen in the very next vacuum
> because there are too many. Which makes little sense; we're far better
> off freezing the pages at the earliest opportunity instead.

That makes sense, but I wonder if we can actually be more specific: One motivating example mentioned is the append-only table. If we detected that case, which I assume we can because autovacuum_vacuum_insert_* GUCs exist, we could use that information as one way to drive eager freezing independently of size. At least in theory -- it's very possible size will be a necessary part of the decision, but it's less clear that it's as useful as a user-tunable knob.

If we then ignored the append-only case when evaluating a freezing policy, maybe other ideas will fall out. I don't have a well-thought out idea about policy or knobs, but it's worth thinking about.

Aside from that, I've only given the patches a brief reading. Having seen the VM snapshot in practice (under "Scanned pages, visibility map snapshot" in the wiki page), it's neat to see fewer pages being scanned. Prefetching not only seems superior to SKIP_PAGES_THRESHOLD, but anticipates asynchronous IO. Keeping only one VM snapshot page in memory makes perfect sense.

I do have a cosmetic, but broad-reaching, nitpick about terms regarding "skipping strategy". That's phrased as a kind of negative -- what we're *not* doing. Many times I had to pause and compute in my head what we're *doing*, i.e. the "scanning strategy". For example, I wonder if the VM strategies would be easier to read as:

VMSNAP_SKIP_ALL_VISIBLE -> VMSNAP_SCAN_LAZY
VMSNAP_SKIP_ALL_FROZEN -> VMSNAP_SCAN_EAGER
VMSNAP_SKIP_NONE -> VMSNAP_SCAN_ALL

Notice here they're listed in order of increasing eagerness.

--
John Naylor
EDB: http://www.enterprisedb.com

Regards,

Nikita Malakhov

Postgres Professional

https://postgrespro.ru/

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

17 December 2022, 00:53:56

On Thu, Dec 15, 2022 at 11:48 PM John Naylor
<john.naylor@enterprisedb.com> wrote:
> Thanks for this. This is the kind of concrete, data-based evidence that I find much more convincing, or at least easy
toreason about.

I'm glad to hear that it helped. It's always difficult to judge where
other people are coming from, especially when it's not clear how much
context is shared. Face time would have helped here, too.

> One motivating example mentioned is the append-only table. If we detected that case, which I assume we can because
autovacuum_vacuum_insert_*GUCs exist, we could use that information as one way to drive eager freezing independently of
size.At least in theory -- it's very possible size will be a necessary part of the decision, but it's less clear that
it'sas useful as a user-tunable knob.

I am not strongly opposed to that idea, though I have my doubts about
it. I have thought about it already, and it wouldn't be hard to get
the information to vacuumlazy.c (I plan on doing it as part of related
work on antiwraparound autovacuum, in fact [1]). I'm skeptical of the
general idea that autovacuum.c has enough reliable information to give
detailed recommendations as to how vacuumlazy.c should process the
table.

I have pointed out several major flaws with the autovacuum.c dead
tuple accounting in the past [2][3], but I also think that there are
significant problems with the tuples inserted accounting. Basically, I
think that there are effects which are arguably an example of the
inspection paradox [4]. Insert-based autovacuums occur on a timeline
determined by the "inserted since last autovacuum" statistics. These
statistics are (in part) maintained by autovacuum/VACUUM itself. Which
has no specific understanding of how it might end up chasing its own
tail.

Let me be more concrete about what I mean about autovacuum chasing its
own tail. The autovacuum_vacuum_insert_threshold mechanism works by
triggering an autovacuum whenever the number of tuples inserted since
the last autovacuum/VACUUM reaches a certain threshold -- usually some
fixed proportion of pg_class.reltuples. But the
tuples-inserted-since-last-VACUUM counter gets reset at the end of
VACUUM, not at the start. Whereas VACUUM itself processes only the
subset of pages that needed to be vacuumed at the start of the VACUUM.
There is no attempt to compensate for that disparity. This *isn't*
really a measure of "unvacuumed tuples" (you'd need to compensate to
get that).

This "at the start vs at the end" difference won't matter at all with
smaller tables. And even in larger tables we might hope that the
effect would kind of average out. But what about cases where one
particular VACUUM operation takes an unusually long time, out of a
sequence of successive VACUUMs that run against the same table? For
example, the sequence that you see on the Wiki page, when Postgres
HEAD autovacuum does an aggressive VACUUM on one occasion, which takes
dramatically longer [5].

Notice that the sequence in [5] shows that the patch does one more
autovacuum operation in total, compared to HEAD/master. That's a lot
more -- we're talking about VACUUMs that each take 40+ minutes. That
can be explained by the fact that VACUUM (quite naturally) resets the
"tuples inserted since last VACUUM" at the end of that unusually long
running aggressive autovacuum -- just like any other VACUUM would.
That seems very weird to me. If (say) we happened to have a much
higher vacuum_freeze_table_age setting, then we wouldn't have had an
aggressive VACUUM until much later on (or never, because the benchmark
would just end). And the VACUUM that was aggressive would have been a
regular VACUUM instead, and would therefore have completed far sooner,
and would therefore have had a *totally* different cadence, compared
to what we actually saw -- it becomes distorted in a way that outlasts
the aggressive VACUUM.

With a far higher vacuum_freeze_table_age, we might have even managed
to do two regular autovacuums in the same period that it took a single
aggressive VACUUM to run in (that's not too far from what actually
happened with the patch). The *second* regular autovacuum would then
end up resetting the "inserted since last VACUUM" counter to 0 at the
same time as the long running aggressive VACUUM actually did so (same
wall clock time, same time since the start of the benchmark). Notice
that we'll have done much less useful work (on cleaning up bloat and
setting newer pages all-visible) with the "one long aggressive mode
VACUUM" setup/scenario -- we'll be way behind -- but the statistics
will nevertheless look about the same as they do in the "two fast
autovacuums instead of one slow autovacuum" counterfactual scenario.

In short, autovacuum.c fails to appreciate that a lot of stuff about
the table changes when VACUUM runs. Time hasn't stood still -- the
table was modified and extended throughout. So autovacuum.c hasn't
compensated for how VACUUM actually performed, and, in effect, forgets
how far it has fallen behind. It should be eager to start the nex
autovacuum very quickly, having fallen behind, but it isn't eager.
This is all the more reason to get rid of aggressive mode, but that's
not my point -- my point is that the statistics driving things seem
quite dubious, in all sorts of ways.

> Aside from that, I've only given the patches a brief reading.

Thanks for taking a look.

> Having seen the VM snapshot in practice (under "Scanned pages, visibility map snapshot" in the wiki page), it's neat
tosee fewer pages being scanned. Prefetching not only seems superior to SKIP_PAGES_THRESHOLD, but anticipates
asynchronousIO.

All of that is true, but more than anything else the VM snapshot
concept appeals to me because it seems to make VACUUMs of large tables
more similar to VACUUMs of small tables. Particularly when one
individual VACUUM happens to take an unusually long amount of time,
for whatever reason (best example right now is aggressive mode, but
there are other ways in which VACUUM can take far longer than
expected). That approach seems much more logical. I also think that
it'll make it easier to teach VACUUM to "pick up where the last VACUUM
left off" in the future.

I understand why you haven't seriously investigated using the same
information for the Radix tree dead_items project. I certainly don't
object. But I still think that having one integrated data structure
(VM snapshots + dead_items) is worth exploring in the future. It's
something that I think is quite promising.

> I do have a cosmetic, but broad-reaching, nitpick about terms regarding "skipping strategy". That's phrased as a kind
ofnegative -- what we're *not* doing. Many times I had to pause and compute in my head what we're *doing*, i.e. the
"scanningstrategy". For example, I wonder if the VM strategies would be easier to read as:
>
> VMSNAP_SKIP_ALL_VISIBLE -> VMSNAP_SCAN_LAZY
> VMSNAP_SKIP_ALL_FROZEN -> VMSNAP_SCAN_EAGER
> VMSNAP_SKIP_NONE -> VMSNAP_SCAN_ALL
>
> Notice here they're listed in order of increasing eagerness.

I agree that the terminology around skipping strategies is confusing,
and plan to address that in the next version. I'll consider using this
scheme for v10.

[1] https://commitfest.postgresql.org/41/4027/
[2] https://postgr.es/m/CAH2-Wz=MGFwJEpEjVzXwEjY5yx=UuNPzA6Bt4DSMasrGLUq9YA@mail.gmail.com
[3] https://postgr.es/m/CAH2-WznrZC-oHkB+QZQS65o+8_Jtj6RXadjh+8EBqjrD1f8FQQ@mail.gmail.com
[4] https://towardsdatascience.com/the-inspection-paradox-is-everywhere-2ef1c2e9d709
[5]
https://wiki.postgresql.org/wiki/Freezing/skipping_strategies_patch:_motivating_examples#Scanned_pages.2C_visibility_map_snapshot
--
Peter Geoghegan

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

17 December 2022, 05:44:11

On Thu, Dec 15, 2022 at 11:59 PM Nikita Malakhov <hukutoc@gmail.com> wrote:
> I've found this discussion very interesting, in view of vacuuming
> TOAST tables is always a problem because these tables tend to
> bloat very quickly with dead data - just to remind, all TOAST-able
> columns of the relation use the same TOAST table which is one
> for the relation, and TOASTed data are not updated - there are
> only insert and delete operations.

I don't think that it would be any different to any other table that
happened to have lots of inserts and deletes, such as the table
described here:

https://wiki.postgresql.org/wiki/Freezing/skipping_strategies_patch:_motivating_examples#Mixed_inserts_and_deletes

In the real world, a table like this would probably consist of some
completely static data, combined with other data that is constantly
deleted and re-inserted -- probably only a small fraction of the table
at any one time. I would expect such a table to work quite well,
because the static pages would all become frozen (at least after a
while), leaving behind only the tuples that are deleted quickly, most
of the time. VACUUM would have a decent chance of noticing that it
will be cheap to advance relfrozenxid in earlier VACUUM operations, as
bloat is cleaned up -- even a VACUUM that happens long before the
point that autovacuum.c will launch an antiwraparound autovacuum has a
decent chance of it. That's not a new idea, really; the
pgbench_branches example from the Wiki page looks like that already,
and even works on Postgres 15.

Here is the part that's new: the pressure to advance relfrozenxid
grows gradually, as table age grows. If table age is still very young,
then we'll only do it if the number of "extra" scanned pages is < 5%
of rel_pages -- only when the added cost is very low (again, like the
pgbench_branches example, mostly). Once table age gets about halfway
towards the point that antiwraparound autovacuuming is required,
VACUUM then starts caring less about costs. It gradually worries less
about the costs, and more about the need to advance it. Ideally it
will happen before antiwraparound autovacuum is actually required.

I'm not sure how much this would help with bloat. I suspect that it
could make a big difference with the right workload. If you always
need frequent autovacuums, just to deal with bloat, then there is
never a good time to run an aggressive antiwraparound autovacuum. An
aggressive AV will probably end up taking much longer than the typical
autovacuum that deals with bloat. While the aggressive AV will remove
as much bloat as any other AV, in theory, that might not help much. If
the aggressive AV takes as long as (say) 5 regular autovacuums would
have taken, and if you really needed those 5 separate autovacuums to
run, just to deal with the bloat, then that's a real problem.  The
aggressive AV effectively causes bloat with such a workload.

-- 
Peter Geoghegan

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

19 December 2022, 01:20:49

On Thu, Dec 15, 2022 at 10:53 AM Peter Geoghegan <pg@bowt.ie> wrote:
> I agree that the burden of catch-up freezing is excessive here (in
> fact I already wrote something to that effect on the wiki page). The
> likely solution can be simple enough.

Attached is v10, which fixes this issue, but using a different
approach to the one I sketched here.

This revision also changes the terminology around VM skipping: we now
call the strategies there "scanning strategies", per feedback from
Jeff and John. This does seem a lot clearer.

Also cleaned up the docs a little bit, which were messed up by a
rebasing issue in v9.

I ended up fixing the aforementioned "too much catch-up freezing"
issue by just getting rid of the whole concept of a second table-size
threshold that forces the eager scanning strategy. I now believe that
it's fine to just rely on the generic logic that determines scanning
strategy based on a combination of table age and the added cost of
eager scanning. It'll work in a way that doesn't result in too much of
a freezing spike during any one VACUUM operation, without waiting
until an antiwraparound autovacuum to advance relfrozenxid (it'll
happen far earlier than that, though still quite a lot later than what
you'd see with v9, so as to avoid that big spike in freezing that was
possible in pgbench_history-like tables [1]).

This means that vacuum_freeze_strategy_threshold is now strictly
concerned with freezing. A table that is always frozen eagerly will
inevitably fall into a pattern of advancing relfrozenxid in every
VACUUM operation, but that isn't something that needs to be documented
or anything. We don't need to introduce a special case here.

The other notable change for v10 is in the final patch, which removes
aggressive mode altogether. v10 now makes lazy_scan_noprune less
willing to give up on setting relfrozenxid to a relatively recent XID.
Now lazy_scan_noprune is willing to wait a short while for a cleanup
lock on a heap page (a few tens of milliseconds) when doing so might
be all it takes to preserve VACUUM's ability to advance relfrozenxid
all the way up to FreezeLimit, which is the traditional guarantee made
by aggressive mode VACUUM.

This makes lazy_scan_noprune "under promise and over deliver". It now
only promises to advance relfrozenxid up to MinXid in the very worst
case -- even if that means waiting indefinitely long for a cleanup
lock. That's not a very strong promise, because advancing relfrozenxid
up to MinXid is only barely adequate. At the same time,
lazy_scan_noprune is willing to go to extra trouble to
get a recent enough FreezeLimit -- it'll wait for a few 10s of milliseconds.
It's just not willing to wait indefinitely. This seems likely to give us the
best of both worlds.

This was based in part on something that Andres said about cleanup
locks a while back. He had a concern about cases where even MinXid was
before OldestXmin. To some degree that's addressed here, because I've
also changed the way that MinXid is determined, so that it'll be a
much earlier value. That doesn't have much downside now, because of the
way that lazy_scan_noprune is now "aggressive-ish" when that happens to
make sense.

Not being able to get a cleanup lock on our first attempt is relatively
rare, and when it happens it's often something completely benign. For
example, it might just be that the checkpointer was writing out the
same page at the time, which signifies nothing about it really being
hard to get a cleanup lock -- the checkpointer will have dropped its
conflicting buffer pin almost immediately. It would be a shame to
accept a significantly older final relfrozenxid during an infrequent,
long running antiwraparound autovacuum of larger tables when that
happens -- we should be willing to wait 30 milliseconds (just not 30
minutes, or 30 days).

None of this even comes up for pages whose XIDs are >= FreezeLimit,
which is actually most pages with the patch, even in larger tables.
It's relatively rare for VACUUM to need to process any heap page in
lazy_scan_noprune, but it'll be much rarer still for it to have to do
a "short wait" like this. So "short waits" have a very small downside,
and (at least occasionally) a huge upside.

By inventing a third alternative behavior (to go along with processing
pages via standard lazy_scan_noprune skipping and processing pages in
lazy_scan_prune), VACUUM has the flexibility to respond in a way
that's proportionate to the problem at hand, in one particular heap
page. The new behavior has zero chance of mattering in most individual
tables/workloads, but it's good to have every possible eventuality
covered. I really hate the idea of getting a significantly worse
outcome just because of something that happened in one single heap
page, because the wind changed directions at the wrong time.

[1] https://wiki.postgresql.org/wiki/Freezing/skipping_strategies_patch:_motivating_examples#Patch

--
Peter Geoghegan

Attachment

Re: New strategies for freezing, advancing relfrozenxid early

From

Jeff Davis

Date:

20 December 2022, 12:01:49

On Sun, 2022-12-18 at 14:20 -0800, Peter Geoghegan wrote:
> Attached is v10, which fixes this issue, but using a different
> approach to the one I sketched here.

In 0001, it's fairly straightforward rearrangement and looks like an
improvement to me. I have a few complaints, but they are about pre-
existing code that you moved around, and I like that you didn't
editorialize too much while just moving code around. +1 from me.

--
Jeff Davis
PostgreSQL Contributor Team - AWS

Re: New strategies for freezing, advancing relfrozenxid early

From

Nikita Malakhov

Date:

20 December 2022, 21:04:14

Hi!

I'll try to apply this patch onto my branch with Pluggable TOAST to test

these mechanics with new TOAST. Would reply on the result. It could

be difficult though, because both have a lot of changes that affect

the same code.

>I'm not sure how much this would help with bloat. I suspect that it
>could make a big difference with the right workload. If you always
>need frequent autovacuums, just to deal with bloat, then there is
>never a good time to run an aggressive antiwraparound autovacuum. An
>aggressive AV will probably end up taking much longer than the typical
>autovacuum that deals with bloat. While the aggressive AV will remove
>as much bloat as any other AV, in theory, that might not help much. If
>the aggressive AV takes as long as (say) 5 regular autovacuums would
>have taken, and if you really needed those 5 separate autovacuums to
>run, just to deal with the bloat, then that's a real problem. The
>aggressive AV effectively causes bloat with such a workload.

On Tue, Dec 20, 2022 at 12:01 PM Jeff Davis <pgsql@j-davis.com> wrote:

On Sun, 2022-12-18 at 14:20 -0800, Peter Geoghegan wrote:
> Attached is v10, which fixes this issue, but using a different
> approach to the one I sketched here.

In 0001, it's fairly straightforward rearrangement and looks like an
improvement to me. I have a few complaints, but they are about pre-
existing code that you moved around, and I like that you didn't
editorialize too much while just moving code around. +1 from me.

--
Jeff Davis
PostgreSQL Contributor Team - AWS

Regards,

Nikita Malakhov

Postgres Professional

https://postgrespro.ru/

Re: New strategies for freezing, advancing relfrozenxid early

From

Jeff Davis

Date:

21 December 2022, 04:44:27

On Sun, 2022-12-18 at 14:20 -0800, Peter Geoghegan wrote:
> On Thu, Dec 15, 2022 at 10:53 AM Peter Geoghegan <pg@bowt.ie> wrote:
> > I agree that the burden of catch-up freezing is excessive here (in
> > fact I already wrote something to that effect on the wiki page).
> > The
> > likely solution can be simple enough.
>
> Attached is v10, which fixes this issue, but using a different
> approach to the one I sketched here.

Comments on 0002:

Can you explain the following portion of the diff:

  - else if (MultiXactIdPrecedes(multi, cutoffs->MultiXactCutoff))
  + else if (MultiXactIdPrecedes(multi, cutoffs->OldestMxact))

  ...

  + /* Can't violate the MultiXactCutoff invariant, either */
  + if (!need_replace)
  +     need_replace = MultiXactIdPrecedes(
  +        multi, cutoffs->MultiXactCutoff);

Regarding correctness, it seems like the basic structure and invariants
are the same, and it builds on the changes already in 9e5405993c. Patch
0002 seems *mostly* about making choices within the existing framework.
That gives me more confidence.

That being said, it does push harder against the limits on both sides.
If I understand correctly, that means pages with wider distributions of
xids are going to persist longer, which could expose pre-existing bugs
in new and interesting ways.

Next, the 'freeze_required' field suggests that it's more involved in
the control flow that causes freezing than it actually is. All it does
is communicate how the trackers need to be adjusted. The return value
of heap_prepare_freeze_tuple() (and underneath, the flags set by
FreezeMultiXactId()) are what actually control what happens. It would
be nice to make this more clear somehow.

The comment:

  /*
   * If we freeze xmax, make absolutely sure that it's not an XID that
   * is important.  (Note, a lock-only xmax can be removed independent
   * of committedness, since a committed lock holder has released the
   * lock).
   */

caused me to go down a rabbit hole looking for edge cases where we
might want to freeze an xmax but not an xmin; e.g. tup.xmax <
OldestXmin < tup.xmin or the related case where tup.xmax < RecentXmin <
tup.xmin. I didn't find a problem, so that's good news.

I also tried some pgbench activity along with concurrent vacuums (and
vacuum freezes) along with periodic verify_heapam(). No problems there.

Did you already describe the testing you've done for 0001+0002
specfiically? It's not radically new logic, but it would be good to try
to catch minor state-handling errors.

--
Jeff Davis
PostgreSQL Contributor Team - AWS

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

21 December 2022, 06:15:33

On Tue, Dec 20, 2022 at 5:44 PM Jeff Davis <pgsql@j-davis.com> wrote:
> Comments on 0002:
>
> Can you explain the following portion of the diff:
>
>
>   - else if (MultiXactIdPrecedes(multi, cutoffs->MultiXactCutoff))
>   + else if (MultiXactIdPrecedes(multi, cutoffs->OldestMxact))
>
>   ...
>
>   + /* Can't violate the MultiXactCutoff invariant, either */
>   + if (!need_replace)
>   +     need_replace = MultiXactIdPrecedes(
>   +        multi, cutoffs->MultiXactCutoff);

Don't forget the historic context: before Postgres 15's commit
0b018fab, VACUUM's final relfrozenxid always came from FreezeLimit.
Almost all of this code predates that work. So the general idea that
you can make a "should I freeze or should I ratchet back my
relfrozenxid tracker instead?" trade-off at the level of individual
tuples and pages is still a very new one. Right now it's only applied
within lazy_scan_noprune(), but 0002 leverages the same principles
here.

Before now, these heapam.c freezing routines had cutoffs called
cutoff_xid and cutoff_multi. These had values that actually came from
vacuumlazy.c's FreezeLimit and MultiXactCutoff cutoffs (which was
rather unclear). But cutoff_xid and cutoff_multi were *also* used as
inexact proxies for OldestXmin and OldestMxact (also kind of unclear,
but true). For example, there are some sanity checks in heapam.c that
kind of pretend that cutoff_xid is OldestXmin, even though it usually
isn't the same value (it can be, but only during VACUUM FREEZE, or
when the min freeze age is 0 in some other way).

So 0002 teaches the same heapam.c code about everything -- about all
of the different cutoffs, and about the true requirements of VACUUM
around relfrozenxid advancement. In fact, 0002 makes vacuumlazy.c cede
a lot of control of "XID stuff" to the same heapam.c code, freezing it
up to think about freezing as something that works at the level of
physical pages. This is key to allowing vacuumlazy.c to reason about
freezing at the level of the whole table. It thinks about physical
blocks, leaving logical XIDs up to heapam.c code.

This business that you asked about in FreezeMultiXactId() is needed so
that we can allow vacuumlazy.c to "think in terms of physical pages",
while at the same time avoiding allocating new Multis in VACUUM --
which requires "thinking about individual xmax fields" instead -- a
somewhat conflicting goal. We're really trying to have it both ways
(we get page-level freezing, with a little tuple level freezing on the
side, sufficient to to avoid allocating new Multis during VACUUMs in
roughly the same way as we do right now).

In most cases "freezing a page" removes all XIDs < OldestXmin, and all
MXIDs < OldestMxact. It doesn't quite work that way in certain rare
cases involving MultiXacts, though. It is convenient to define "freeze
the page" in a way that gives heapam.c's FreezeMultiXactId() the
leeway to put off the work of processing an individual tuple's xmax,
whenever it happens to be a MultiXactId that would require an
expensive second pass to process aggressively (allocating a new Multi
during VACUUM is especially worth avoiding here).

Our definition of "freeze the page" is a bit creative, at least if
you're used to thinking about it in terms of strict XID-wise cutoffs
like OldestXmin/FreezeLimit. But even if you do think of it in terms
of XIDs, the practical difference is extremely small in practice.

FreezeMultiXactId() effectively makes a decision on how to proceed
with processing at the level of each individual xmax field.  Its no-op
multi processing "freezes" an xmax in the event of a costly-to-process
xmax on a page when (for whatever reason) page-level freezing is
triggered. If, on the other hand, page-level freezing isn't triggered
for the page, then page-level no-op processing takes care of the multi
for us instead. Either way, the remaining Multi will ratchet back
VACUUM's relfrozenxid and/or relminmxid trackers as required, and we
won't need an expensive second pass over the multi (unless we really
have no choice, for example during a VACUUM FREEZE, where
OldestXmin==FreezeLimit).

> Regarding correctness, it seems like the basic structure and invariants
> are the same, and it builds on the changes already in 9e5405993c. Patch
> 0002 seems *mostly* about making choices within the existing framework.
> That gives me more confidence.

You're right that it's the same basic invariants as before, of course.
Turns out that those invariants can be pushed quite far.

Though note that I kind of invented a new invariant (not really, sort
of). Well, it's a postcondition, which is a sort of invariant: any
scanned heap page that can be cleanup locked must never have any
remaining XIDs < FreezeLimit, nor can any MXIDs < MultiXactCutoff
remain. But a cleanup-locked page does *not* need to get rid of all
XIDs < OldestXmin, nor MXIDs < OldestMxact.

This flexibility is mostly useful because it allows lazy_scan_prune to
just decide to not freeze. But, to a much lesser degree, it's useful
because of the edge case with multis -- in general we might just need
the same leeway when lazy_scan_prune "freezes the page".

> That being said, it does push harder against the limits on both sides.
> If I understand correctly, that means pages with wider distributions of
> xids are going to persist longer, which could expose pre-existing bugs
> in new and interesting ways.

I don't think it's fundamentally different to what we're already doing
in lazy_scan_noprune. It's just more complicated, because you have to
tease apart slightly different definitions of freezing to understand
code around FreezeMultiXactId(). This is more or less needed to
provide maximum flexibility, where we delay decisions about what to do
until the very last moment.

> Next, the 'freeze_required' field suggests that it's more involved in
> the control flow that causes freezing than it actually is. All it does
> is communicate how the trackers need to be adjusted. The return value
> of heap_prepare_freeze_tuple() (and underneath, the flags set by
> FreezeMultiXactId()) are what actually control what happens. It would
> be nice to make this more clear somehow.

I'm not sure what you mean. Page-level freezing *doesn't* have to go
ahead when freeze_required is not ever set to true for any tuple on
the page (which is most of the time, in practice). lazy_scan_prune
gets to make a choice about freezing the page, when the choice is
available.

Note also that the FRM_NOOP case happens when a call to
FreezeMultiXactId() takes place that won't leave behind a freeze plan
for the tuple (unless its xmin happens to necessitate a freeze plan
for the same tuple). And yet, it will do useful work, needed iff the
"freeze the page" path is ultimately taken by lazy_scan_prune --
FreezeMultiXactId() itself will ratchet back
FreezePageRelfrozenXid/NewRelfrozenXid as needed to make everything
safe.

> The comment:
>
>   /*
>    * If we freeze xmax, make absolutely sure that it's not an XID that
>    * is important.  (Note, a lock-only xmax can be removed independent
>    * of committedness, since a committed lock holder has released the
>    * lock).
>    */
>
> caused me to go down a rabbit hole looking for edge cases where we
> might want to freeze an xmax but not an xmin; e.g. tup.xmax <
> OldestXmin < tup.xmin or the related case where tup.xmax < RecentXmin <
> tup.xmin. I didn't find a problem, so that's good news.

This is an example of what I meant about the heapam.c code using a
cutoff that actually comes from FreezeLimit, when it would be more
sensible to use OldestXmin instead.

> I also tried some pgbench activity along with concurrent vacuums (and
> vacuum freezes) along with periodic verify_heapam(). No problems there.
>
> Did you already describe the testing you've done for 0001+0002
> specfiically? It's not radically new logic, but it would be good to try
> to catch minor state-handling errors.

Lots of stuff with contrib/amcheck, which, as you must already know,
will notice when an XID/MXID is contained in a table whose
relfrozenxid and/or relminmxid indicates that it shouldn't be there.
(Though VACUUM itself does the same thing, albeit not as effectively.)

Obviously the invariants haven't changed here. In many ways it's a
very small set of changes. But in one or two ways it's a significant
shift. It depends on how you think about it.

--
Peter Geoghegan

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

21 December 2022, 08:26:21

On Tue, Dec 20, 2022 at 7:15 PM Peter Geoghegan <pg@bowt.ie> wrote:
> On Tue, Dec 20, 2022 at 5:44 PM Jeff Davis <pgsql@j-davis.com> wrote:
> > Next, the 'freeze_required' field suggests that it's more involved in
> > the control flow that causes freezing than it actually is. All it does
> > is communicate how the trackers need to be adjusted. The return value
> > of heap_prepare_freeze_tuple() (and underneath, the flags set by
> > FreezeMultiXactId()) are what actually control what happens. It would
> > be nice to make this more clear somehow.
>
> I'm not sure what you mean. Page-level freezing *doesn't* have to go
> ahead when freeze_required is not ever set to true for any tuple on
> the page (which is most of the time, in practice). lazy_scan_prune
> gets to make a choice about freezing the page, when the choice is
> available.

Oh wait, I think I see the point of confusion now.

When freeze_required is set to true, that means that lazy_scan_prune
literally has no choice -- it simply must freeze the page as
instructed by heap_prepare_freeze_tuple/FreezeMultiXactId. It's not
just a strong suggestion -- it's crucial that lazy_scan_prune freezes
the page as instructed.

The "no freeze" trackers (HeapPageFreeze.NoFreezePageRelfrozenXid and
HeapPageFreeze.NoFreezePageRelminMxid) won't have been maintained
properly when freeze_required was set, so lazy_scan_prune can't expect
to use them -- doing so would lead to VACUUM setting incorrect values
in pg_class later on.

Avoiding the work of maintaining those "no freeze" trackers isn't just
a nice-to-have microoptimization -- it is sometimes very important. We
kind of rely on this to be able to avoid getting too many MultiXact
member SLRU buffer misses inside FreezeMultiXactId. There is a comment
above FreezeMultiXactId that advises its caller that it had better not
call heap_tuple_should_freeze when freeze_required is set to true,
because that could easily lead to multixact member SLRU buffer misses
-- misses that FreezeMultiXactId set out to avoid itself.

It could actually be cheaper to freeze than to not freeze, in the case
of a Multi -- member space misses can sometimes be really expensive.
And so FreezeMultiXactId sometimes freezes a Multi even though it's
not strictly required to do so.

Note also that this isn't a new behavior -- it's actually an old one,
for the most part. It kinda doesn't look that way, because we haven't
passed down separate FreezeLimit/OldestXmin cutoffs (and separate
OldestMxact/MultiXactCutoff cutoffs) until now. But we often don't
need that granular information to be able to process Multis before the
multi value is < MultiXactCutoff.

If you look at how FreezeMultiXactId works, in detail, you'll see that
even on Postgres HEAD it can (say) set a tuple's xmax to
InvalidTransactionId long before the multi value is < MultiXactCutoff.
It just needs to detect that the multi is not still running, and
notice that it's HEAP_XMAX_IS_LOCKED_ONLY(). Stuff like that happens
quite a bit. So for the most part "eager processing of Multis as a
special case" is an old behavior, that has only been enhanced a little
bit (the really important, new change in FreezeMultiXactId is how the
FRM_NOOP case works with FreezeLimit, even though OldestXmin is used
nearby -- this is extra confusing because 0002 doesn't change how we
use FreezeLimit -- it actually changes every other use of FreezeLimit
nearby, making it OldestXmin).

-- 
Peter Geoghegan

Re: New strategies for freezing, advancing relfrozenxid early

From

Jeff Davis

Date:

22 December 2022, 03:30:28

On Tue, 2022-12-20 at 21:26 -0800, Peter Geoghegan wrote:
> When freeze_required is set to true, that means that lazy_scan_prune
> literally has no choice -- it simply must freeze the page as
> instructed by heap_prepare_freeze_tuple/FreezeMultiXactId. It's not
> just a strong suggestion -- it's crucial that lazy_scan_prune freezes
> the page as instructed.

The confusing thing to me is perhaps just the name -- to me,
"freeze_required" suggests that if it were set to true, it would cause
freezing to happen. But as far as I can tell, it does not cause
freezing to happen, it causes some other things to happen that are
necessary when freezing happens (updating and using the right
trackers).

A minor point, no need to take action here. Perhaps rename the
variable.

I think 0001+0002 are about ready.

--
Jeff Davis
PostgreSQL Contributor Team - AWS

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

22 December 2022, 03:53:48

On Wed, Dec 21, 2022 at 4:30 PM Jeff Davis <pgsql@j-davis.com> wrote:
> The confusing thing to me is perhaps just the name -- to me,
> "freeze_required" suggests that if it were set to true, it would cause
> freezing to happen. But as far as I can tell, it does not cause
> freezing to happen, it causes some other things to happen that are
> necessary when freezing happens (updating and using the right
> trackers).

freeze_required is about what's required, which tells us nothing about
what will happen when it's not required (could go either way,
depending on how lazy_scan_prune feels about it).

Setting freeze_required=true implies that heap_prepare_freeze_tuple
has stopped doing maintenance of the "no freeze" trackers. When it
sets freeze_required=true, it really *does* force freezing to happen,
in every practical sense. This happens because lazy_scan_prune does
what it's told to do when it's told that freezing is required. Because
of course it does, why wouldn't it?

So...I still don't get what you mean. Why would lazy_scan_prune ever
break its contract with heap_prepare_freeze_tuple? And in what sense
would you say that heap_prepare_freeze_tuple's setting
freeze_required=true doesn't quite amount to "forcing freezing"? Are
you worried about the possibility that lazy_scan_prune will decide to
rebel at some point, and fail to honor its contract with
heap_prepare_freeze_tuple?  :-)

> A minor point, no need to take action here. Perhaps rename the
> variable.

Andres was the one that suggested this name, actually. I initially
just called it "freeze", but I think that Andres had it right.

> I think 0001+0002 are about ready.

Great. I plan on committing 0001 in the next few days. Committing 0002
might take a bit longer.

Thanks
-- 
Peter Geoghegan

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

22 December 2022, 22:39:10

On Wed, Dec 21, 2022 at 4:53 PM Peter Geoghegan <pg@bowt.ie> wrote:
> Great. I plan on committing 0001 in the next few days. Committing 0002
> might take a bit longer.

I pushed the VACUUM cutoffs patch (previously 0001) this morning -
thanks for your help with that one.

Attached is v11, which is mostly just to fix the bitrot caused by
today's commits. Though I did adjust some of the commit messages a
bit. There is also one minor functional change in v11: we now always
use eager freezing strategy in unlogged and temp tables, since it's
virtually guaranteed to be a win there.

With an unlogged or temp table, most of the cost of freezing is just
the cycles spent preparing to freeze, since, of course, there isn't
any WAL overhead to have to worry about (which is the dominant concern
with freezing costs, in general). Deciding *not* to freeze pages that
we can freeze and make all-frozen in the VM from unlogged/temp tables
seems like a case of wasting the cycles spent preparing freeze plans.
Why not just do the tiny additional work of executing the freeze plans
at that point?

It's not like eager freezing strategy comes with an added risk that
VACUUM will allocate new multis that it wouldn't otherwise have to
allocate. Nor does it change cleanup-lock-wait behavior. Clearly this
optimization isn't equivalent to interpreting vacuum_freeze_min_age as
0 in unlogged/temp tables. The whole design of freezing strategies is
supposed to abstract away details like that, freeing up high level
code like lazy_scan_strategy to think about freezing at the level of
the whole table -- the cost model stuff really benefits from being
able to measure debt at the table level, measuring things in terms of
units like total all-frozen pages, rel_pages, etc.

--
Peter Geoghegan

Attachment

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

26 December 2022, 23:53:52

On Thu, Dec 22, 2022 at 11:39 AM Peter Geoghegan <pg@bowt.ie> wrote:
> On Wed, Dec 21, 2022 at 4:53 PM Peter Geoghegan <pg@bowt.ie> wrote:
> > Great. I plan on committing 0001 in the next few days. Committing 0002
> > might take a bit longer.
>
> I pushed the VACUUM cutoffs patch (previously 0001) this morning -
> thanks for your help with that one.

Attached is v12. I think that the page-level freezing patch is now
commitable, and plan on committing it in the next 2-4 days barring any
objections.

Notable changes in v12:

* Simplified some of the logic in FreezeMultiXactId(), which now
doesn't have any needless handling of NewRelfrozenXid style cutoffs
except in the one case that still needs it (its no-op processing
case).

We don't need most of the handling on HEAD anymore because every
possible approach to processing a Multi other than FRM_NOOP will
reliably leave behind a new xmax that is either InvalidTransactionId,
or an XID/MXID >= OldestXmin/OldestMxact. Such values cannot possibly
need to be tracked by the NewRelfrozenXid trackers, since the trackers
are initialized using OldestXmin/OldestMxact to begin with.

* v12 merges together the code for the "freeze the page"
lazy_scan_prune path with the block that actually calls
heap_freeze_execute_prepared().

This should make it clear that pagefrz.freeze_required really does
mean that freezing is required. Hopefully that addresses Jeff's recent
concern. It's certainly an improvement, in any case.

* On a related note, comments around the same point in lazy_scan_prune
as well as comments above the HeapPageFreeze struct now explain a
concept I decided to call "nominal freezing". This is the case where
we "freeze a page" without having any freeze plans to execute.

"nominal freezing" is the new name for a concept I invented many
months ago, which helps to resolve subtle problems with the way that
heap_prepare_freeze_tuple is tasked with doing two different things
for its lazy_scan_prune caller: 1. telling lazy_scan_prune how it
would freeze each tuple (were it to freeze the page), and 2. helping
lazy_scan_prune to determine if the page should become all-frozen in
the VM. The latter is always conditioned on page-level freezing
actually going ahead, since everything else in
heap_prepare_freeze_tuple has to work that way.

We always freeze a page with zero freeze plans (or "nominally freeze"
the page) in lazy_scan_prune (which is nothing new in itself). We
thereby avoid breaking heap_prepare_freeze_tuple's working assumption
that all it needs to focus on what the page will look like after
freezing executes, while also avoiding senselessly throwing away the
ability to set a page all-frozen in the VM in lazy_scan_prune when
it'll cost us nothing extra. That is, by always freezing in the event
of zero freeze plans, we won't senselessly miss out on setting a page
all-frozen in cases where we don't actually have to execute any freeze
plans to make that safe, while the "freeze the page path versus don't
freeze the page path" dichotomy still works as a high level conceptual
abstraction.

-- 
Peter Geoghegan

On Mon, Dec 26, 2022 at 12:53 PM Peter Geoghegan <pg@bowt.ie> wrote:
> Attached is v12. I think that the page-level freezing patch is now
> commitable, and plan on committing it in the next 2-4 days barring any
> objections.

I've pushed the page-level freezing patch, so now I need to produce a
new revision, just to keep CFTester happy.

Attached is v13. No notable changes since v12.

-- 
Peter Geoghegan

On Sat, Dec 31, 2022 at 12:45 PM Peter Geoghegan <pg@bowt.ie> wrote:
> On Sat, Dec 31, 2022 at 11:46 AM Jeff Davis <pgsql@j-davis.com> wrote:
> > "We have no freeze plans to execute, so there's no cost to following
> > the freeze path. This is important in the case where the page is
> > entirely frozen already, so that the page will be marked as such in the
> > VM."
>
> I'm happy to use your wording instead -- I'll come up with a patch for that.

What do you think of the wording adjustments in the attached patch?
It's based on your suggested wording.

--
Peter Geoghegan

Attachment

0001-Tweak-page-level-freezing-comments.patch

Re: New strategies for freezing, advancing relfrozenxid early

From

Jeff Davis

Date:

03 January 2023, 05:26:13

On Mon, 2023-01-02 at 11:45 -0800, Peter Geoghegan wrote:
> What do you think of the wording adjustments in the attached patch?
> It's based on your suggested wording.

Great, thank you.


--
Jeff Davis
PostgreSQL Contributor Team - AWS

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

03 January 2023, 23:30:02

On Mon, Jan 2, 2023 at 6:26 PM Jeff Davis <pgsql@j-davis.com> wrote:
> On Mon, 2023-01-02 at 11:45 -0800, Peter Geoghegan wrote:
> > What do you think of the wording adjustments in the attached patch?
> > It's based on your suggested wording.
>
> Great, thank you.

Pushed that today.

Attached is v14.

v14 simplifies the handling of setting the visibility map at the end
of the blkno-wise loop in lazy_scan_heap(). And,
visibilitymap_snap_next() doesn't tell its caller (lazy_scan_heap)
anything about the visibility status of each returned block -- we no
longer need a all_visible_according_to_vm local variable to help with
setting the visibility map.

This new approach to setting the VM is related to hardening that I
plan on adding, which makes the visibility map robust against certain
race conditions that can lead to setting a page all-frozen but not
all-visible. I go into that here:

https://postgr.es/m/CAH2-WznuNGSzF8v6OsgjaC5aYsb3cZ6HW6MLm30X0d65cmSH6A@mail.gmail.com

(It's the second patch -- the first patch already became yesterday's
commit 6daeeb1f.)

In general I don't think that we should be using
all_visible_according_to_vm for anything, especially not anything
critical -- it is just information about how the page used to be in
the past, after all. This will be more of a problem with visibility
map snapshots, since all_visible_according_to_vm could be information
that is hours old by the time it's actually used by lazy_scan_heap().
But it is an existing issue.

BTW, it would be helpful if I could get a +1 to the visibility map
patch posted on that other thread. It's practically a bug fix -- the
VM shouldn't be able to show contradictory information about any given
heap page (i.e. "page is all-frozen but not all-visible"), no matter
what. Just on general principle.

--
Peter Geoghegan

On Tue, Jan 3, 2023 at 12:30 PM Peter Geoghegan <pg@bowt.ie> wrote:
> Attached is v14.

This has stopped applying due to conflicts with nearby work on VACUUM
from Tom. So I attached a new revision, v15, just to make CFTester
green again.

I didn't have time to incorporate any of the feedback from Matthias
just yet. That will have to wait until v16.

--
Peter Geoghegan

On Sun, Jan 8, 2023 at 5:45 PM Peter Geoghegan <pg@bowt.ie> wrote:
> I didn't have time to incorporate any of the feedback from Matthias
> just yet. That will have to wait until v16.

Attached is v16, which incorporates some of Matthias' feedback.

I've rolled back the major restructuring to the "Routine Vacuuming"
docs that previously appeared in 0003, preferring to take a much more
incremental approach. I do still think that somebody needs to do some
major reworking of that, just in general. That can be done by a
separate patch. There are now only fairly mechanical doc updates in
all 3 patches.

Other changes:

* vacuum_freeze_strategy_threshold is now MB-based, and can be set up to 512TB.

* Various refinements to comments.

--
Peter Geoghegan

Attachment

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

17 January 2023, 04:55:49

On Mon, Jan 16, 2023 at 10:10 AM Peter Geoghegan <pg@bowt.ie> wrote:
> Attached is v16, which incorporates some of Matthias' feedback.

0001 (the freezing strategies patch) is now committable IMV. Or at
least will be once I polish the docs a bit more. I plan on committing
0001 some time next week, barring any objections.

I should point out that 0001 is far shorter and simpler than the
page-level freezing commit that already went in (commit 1de58df4). The
only thing in 0001 that seems like it might be a bit controversial
(when considered on its own) is the addition of the
vacuum_freeze_strategy_threshold GUC/reloption. Note in particular
that vacuum_freeze_strategy_threshold doesn't look like any other
existing GUC; it gets applied as a threshold on the size of the rel's
main fork at the beginning of vacuumlazy.c processing. As far as I
know there are no objections to that approach at this time, but it
does still seem worth drawing attention to now.

0001 also makes unlogged tables and temp tables always use eager
freezing strategy, no matter how the GUC/reloption are set. This seems
*very* easy to justify, since the potential downside of such a policy
is obviously extremely low, even when we make very pessimistic
assumptions. The usual cost we need to worry about when it comes to
freezing is the added WAL overhead -- that clearly won't apply when
we're vacuuming non-permanent tables. That really just leaves the cost
of dirtying extra pages, which in general could have a noticeable
system-level impact in the case of unlogged tables.

Dirtying extra pages when vacuuming an unlogged table is also a
non-issue. Even the eager freezing strategy only freezes "extra" pages
("extra" relative to the lazy strategy behavior) given a page that
will be set all-visible in any case [1]. Such a page will need to have
its page-level PD_ALL_VISIBLE bit set in any case -- which is already
enough to dirty the page. And so there can never be any additional
pages dirtied as a result of the special policy 0001 adds for
non-permanent relations.

[1] https://wiki.postgresql.org/wiki/Freezing/skipping_strategies_patch:_motivating_examples#Patch_2
--
Peter Geoghegan

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

17 January 2023, 05:24:53

On Mon, Jan 16, 2023 at 10:00 AM Peter Geoghegan <pg@bowt.ie> wrote:
> Now we treat the scanning and freezing strategies as two independent
> choices. Of course they're not independent in any practical sense, but
> I think it's slightly simpler and more elegant that way -- it makes
> the GUC vacuum_freeze_strategy_threshold strictly about freezing
> strategy, while still leading to VACUUM advancing relfrozenxid in a
> way that makes sense. It just happens as a second order effect. Why
> add a special case?

This might be a better way to explain it:

The main page-level freezing commit (commit 1de58df4) already added an
optimization that triggers page-level freezing "early" (early relative
to vacuum_freeze_min_age). This happens whenever a page already needs
to have an FPI logged inside lazy_scan_prune -- even when we're using
the lazy freezing strategy. The optimization isn't configurable, and
gets applied regardless of freezing strategy (technically there is no
such thing as freezing strategies on HEAD just yet, though HEAD still
has this optimization).

There will be workloads where the FPI optimization will result in
freezing many more pages -- especially when data checksums are in use
(since then we could easily need to log an FPI just so pruning can set
a hint bit). As a result, certain VACUUMs that use the lazy freezing
strategy will freeze in almost the same way as an equivalent VACUUM
using the eager freezing strategy. Such a "nominally lazy but actually
quite eager" VACUUM operation should get the same benefit in terms of
relfrozenxid advancement as it would if it really had used the eager
freezing strategy instead. It's fairly obvious that we'll get the same
benefit in relfrozenxid advancement (comparable relfrozenxid results
for comparable freezing work), since the way that VACUUM decides on
its scanning strategy is not conditioned on freezing strategy (whether
by the ongoing VACUUM or any other VACUUM against the same table).

All that matters is the conditions in the table (in particular the
added cost of opting for eager scanning over lazy scanning) as
indicated by the visibility map at the start of each VACUUM -- how
those conditions came about really isn't interesting at that point.
And so lazy_scan_strategy doesn't care about them when it chooses
VACUUM's scanning strategy.

There are even tables/workloads where relfrozenxid will be able to
jump forward by a huge amount whenever VACUUM choosing the eager
scanning strategy, despite the fact that VACUUM generally does little
or no freezing to make that possible:

https://wiki.postgresql.org/wiki/Freezing/skipping_strategies_patch:_motivating_examples#Patch_3

--
Peter Geoghegan

Re: New strategies for freezing, advancing relfrozenxid early

From

Dilip Kumar

Date:

17 January 2023, 07:13:12

On Mon, Jan 16, 2023 at 11:31 PM Peter Geoghegan <pg@bowt.ie> wrote:
>
> > I think '(nextXID - cutoffs->relfrozenxid) / freeze_table_age' should
> > be the actual fraction right?  What is the point of adding 0.5 to the
> > divisor?  If there is a logical reason, maybe we can explain in the
> > comments.
>
> It's just a way of avoiding division by zero.

oh, correct :)

> > While looking into the logic of 'lazy_scan_strategy', I think the idea
> > looks very good but the only thing is that
> > we have kept eager freeze and eager scan completely independent.
> > Don't you think that if a table is chosen for an eager scan
> > then we should force the eager freezing as well?
>
> Earlier versions of the patch kind of worked that way.
> lazy_scan_strategy would actually use twice the GUC setting to
> determine scanning strategy. That approach could make our "transition
> from lazy to eager strategies" involve an excessive amount of
> "catch-up freezing" in the VACUUM operation that advanced relfrozenxid
> for the first time, which you see an example of here:
>
> https://wiki.postgresql.org/wiki/Freezing/skipping_strategies_patch:_motivating_examples#Patch
>
> Now we treat the scanning and freezing strategies as two independent
> choices. Of course they're not independent in any practical sense, but
> I think it's slightly simpler and more elegant that way -- it makes
> the GUC vacuum_freeze_strategy_threshold strictly about freezing
> strategy, while still leading to VACUUM advancing relfrozenxid in a
> way that makes sense. It just happens as a second order effect. Why
> add a special case?

I think that it makes sense to keep 'vacuum_freeze_strategy_threshold'
strictly for freezing.  But the point is that the eager scanning
strategy is driven by table freezing needs of the table (tableagefrac)
that make sense, but if we have selected the eager freezing based on
the table age and its freezing need then why don't we force the eager
freezing as well if we have selected eager scanning, after all the
eager scanning is selected for satisfying the freezing need.  But
OTOH, the eager scanning might get selected if it appears that we
might not have to scan too many extra pages compared to lazy scan so
in those cases forcing the eager freezing might not be wise.  So maybe
it is a good idea to keep them the way you have in your patch.

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

17 January 2023, 07:35:05

On Mon, Jan 16, 2023 at 8:13 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> I think that it makes sense to keep 'vacuum_freeze_strategy_threshold'
> strictly for freezing.  But the point is that the eager scanning
> strategy is driven by table freezing needs of the table (tableagefrac)
> that make sense, but if we have selected the eager freezing based on
> the table age and its freezing need then why don't we force the eager
> freezing as well if we have selected eager scanning, after all the
> eager scanning is selected for satisfying the freezing need.

Don't think of eager scanning as the new name for aggressive mode --
it's a fairly different concept, because we care about costs now.
Eager scanning can be chosen just because it's very cheap relative to
the alternative of lazy scanning, even when relfrozenxid is still very
recent. (This kind of behavior isn't really new [1], but the exact
implementation from the patch is new.)

Tables such as pgbench_branches and pgbench_tellers will reliably use
eager scanning strategy, no matter how any GUC has been set -- just
because the added cost is always zero (relative to lazy scanning). It
really doesn't matter how far along tableagefrac here, ever. These
same tables will never use eager freezing strategy, unless the
vacuum_freeze_strategy_threshold GUC is misconfigured. (This is
another example of how scanning strategy and freezing strategy may
differ for the same table.)

You do have a good point, though. I think that I know what you mean.
Note that antiwraparound autovacuums (or VACUUMs of tables very near
to that point) *will* always use both the eager freezing strategy and
the eager scanning strategy -- which is probably close to what you
meant.

The important point is that there can be more than one reason to
prefer one strategy to another -- and the reasons can be rather
different. Occasionally it'll be a combination of two factors together
that push things in favor of one strategy over the other -- even
though either factor on its own would not have resulted in the same
choice.

[1]
https://wiki.postgresql.org/wiki/Freezing/skipping_strategies_patch:_motivating_examples#Constantly_updated_tables_.28usually_smaller_tables.29
-- 
Peter Geoghegan

Re: New strategies for freezing, advancing relfrozenxid early

From

Dilip Kumar

Date:

18 January 2023, 11:17:44

On Tue, Jan 17, 2023 at 10:05 AM Peter Geoghegan <pg@bowt.ie> wrote:
>
> On Mon, Jan 16, 2023 at 8:13 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > I think that it makes sense to keep 'vacuum_freeze_strategy_threshold'
> > strictly for freezing.  But the point is that the eager scanning
> > strategy is driven by table freezing needs of the table (tableagefrac)
> > that make sense, but if we have selected the eager freezing based on
> > the table age and its freezing need then why don't we force the eager
> > freezing as well if we have selected eager scanning, after all the
> > eager scanning is selected for satisfying the freezing need.
>
> Don't think of eager scanning as the new name for aggressive mode --
> it's a fairly different concept, because we care about costs now.
> Eager scanning can be chosen just because it's very cheap relative to
> the alternative of lazy scanning, even when relfrozenxid is still very
> recent. (This kind of behavior isn't really new [1], but the exact
> implementation from the patch is new.)
>
> Tables such as pgbench_branches and pgbench_tellers will reliably use
> eager scanning strategy, no matter how any GUC has been set -- just
> because the added cost is always zero (relative to lazy scanning). It
> really doesn't matter how far along tableagefrac here, ever. These
> same tables will never use eager freezing strategy, unless the
> vacuum_freeze_strategy_threshold GUC is misconfigured. (This is
> another example of how scanning strategy and freezing strategy may
> differ for the same table.)

Yes, I agree with that.  Thanks for explaining in detail.

> You do have a good point, though. I think that I know what you mean.
> Note that antiwraparound autovacuums (or VACUUMs of tables very near
> to that point) *will* always use both the eager freezing strategy and
> the eager scanning strategy -- which is probably close to what you
> meant.

Right

> The important point is that there can be more than one reason to
> prefer one strategy to another -- and the reasons can be rather
> different. Occasionally it'll be a combination of two factors together
> that push things in favor of one strategy over the other -- even
> though either factor on its own would not have resulted in the same
> choice.

Yes, that makes sense to me.

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

Re: New strategies for freezing, advancing relfrozenxid early

From

Dilip Kumar

Date:

23 January 2023, 14:17:09

On Wed, Jan 18, 2023 at 1:47 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Tue, Jan 17, 2023 at 10:05 AM Peter Geoghegan <pg@bowt.ie> wrote:

My final set of comments for 0002

1.
+struct vmsnapshot
+{
+    /* Target heap rel */
+    Relation    rel;
+    /* Scanning strategy used by VACUUM operation */
+    vmstrategy    strat;
+    /* Per-strategy final scanned_pages */
+    BlockNumber rel_pages;
+    BlockNumber scanned_pages_lazy;
+    BlockNumber scanned_pages_eager;

I do not understand much use of maintaining these two
'scanned_pages_lazy' and 'scanned_pages_eager' variables.  I think
just maintaining 'scanned_pages' should be sufficient.  I do not see
in patches also they are really used.  lazy_scan_strategy() is using
these variables but this is getting values of these out parameters
from visibilitymap_snap_acquire().  And visibilitymap_snap_strategy()
is also using this, but it seems there we just need the final result
of 'scanned_pages' instead of these two variables.

2.

+#define MAX_PAGES_YOUNG_TABLEAGE    0.05    /* 5% of rel_pages */
+#define MAX_PAGES_OLD_TABLEAGE        0.70    /* 70% of rel_pages */

Why is the logic behind 5% and 70% are those based on some
experiments?  Should those be tuning parameters so that with real
world use cases if we realise that it would be good if the eager scan
is getting selected more frequently or less frequently then we can
tune those parameters?

3.
+    /*
+     * VACUUM's DISABLE_PAGE_SKIPPING option overrides our decision by forcing
+     * VACUUM to scan every page (VACUUM effectively distrusts rel's VM)
+     */
+    if (force_scan_all)
+        vacrel->vmstrat = VMSNAP_SCAN_ALL;

I think this should be moved as first if case, I mean why to do all
the calculations based on the 'tableagefrac' and
'TABLEAGEFRAC_XXPOINT' if we are forced to scan them all.  I agree the
extra computation we are doing might not really matter compared to the
vacuum work we are going to perform but still seems logical to me to
do the simple check first.

4. Should we move prefetching as a separate patch, instead of merging
with the scanning strategy?

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

23 January 2023, 21:01:27

On Mon, Jan 23, 2023 at 3:17 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> My final set of comments for 0002

Thanks for the review!

> I do not understand much use of maintaining these two
> 'scanned_pages_lazy' and 'scanned_pages_eager' variables.  I think
> just maintaining 'scanned_pages' should be sufficient.  I do not see
> in patches also they are really used.

I agree that the visibility map snapshot struct could stand to be
cleaned up -- some of that state may not be needed, and it wouldn't be
that hard to use memory a little more economically, particularly with
very small tables. It's on my TODO list already.

> +#define MAX_PAGES_YOUNG_TABLEAGE    0.05    /* 5% of rel_pages */
> +#define MAX_PAGES_OLD_TABLEAGE        0.70    /* 70% of rel_pages */
>
> Why is the logic behind 5% and 70% are those based on some
> experiments?  Should those be tuning parameters so that with real
> world use cases if we realise that it would be good if the eager scan
> is getting selected more frequently or less frequently then we can
> tune those parameters?

The specific multipliers constants chosen (for
MAX_PAGES_YOUNG_TABLEAGE and MAX_PAGES_OLD_TABLEAGE) were based on
both experiments and intuition. The precise values could be somewhat
different without it really mattering, though. For example, with a
table like pgbench_history (which is a really important case for the
patch in general), there won't be any all-visible pages at all (at
least after a short while), so it won't matter what these constants
are -- eager scanning will always be chosen.

I don't think that they should be parameters. The useful parameter for
users remains vacuum_freeze_table_age/autovacuum_freeze_max_age (note
that vacuum_freeze_table_age usually gets its value from
autovacuum_freeze_max_age due to changes in 0002). Like today,
vacuum_freeze_table_age forces VACUUM to scan all not-all-frozen pages
so that relfrozenxid can be advanced. Unlike today, it forces eager
scanning (not aggressive mode). But even long before eager scanning is
*forced*, pressure to use eager scanning gradually builds. That
pressure will usually cause some VACUUM to use eager scanning before
it's strictly necessary. Overall,
vacuum_freeze_table_age/autovacuum_freeze_max_age now provide loose
guidance.

It really has to be loose in this sense in order for
lazy_scan_strategy() to have the freedom to do the right thing based
on the characteristics of the table as a whole, according to its
visibility map snapshot. This allows lazy_scan_strategy() to stumble
upon once-off opportunities to advance relfrozenxid inexpensively,
including cases where it could never happen with the current model.
These opportunities are side-effects of workload characteristics that
can be hard to predict [1][2].

> I think this should be moved as first if case, I mean why to do all
> the calculations based on the 'tableagefrac' and
> 'TABLEAGEFRAC_XXPOINT' if we are forced to scan them all.  I agree the
> extra computation we are doing might not really matter compared to the
> vacuum work we are going to perform but still seems logical to me to
> do the simple check first.

This is only needed for DISABLE_PAGE_SKIPPING, which is an escape
hatch option that is never supposed to be needed. I don't think that
it's worth going to the trouble of indenting the code more just so
this is avoided -- it really is an afterthought. Besides, the compiler
might well be doing this for us.

> 4. Should we move prefetching as a separate patch, instead of merging
> with the scanning strategy?

I don't think that breaking that out would be an improvement. A lot of
the prefetching stuff informs how the visibility map code is
structured.

[1] https://wiki.postgresql.org/wiki/Freezing/skipping_strategies_patch:_motivating_examples#Patch_3
[2]
https://wiki.postgresql.org/wiki/Freezing/skipping_strategies_patch:_motivating_examples#Opportunistically_advancing_relfrozenxid_with_bursty.2C_real-world_workloads
--
Peter Geoghegan

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

25 January 2023, 01:49:38

On Mon, Jan 16, 2023 at 5:55 PM Peter Geoghegan <pg@bowt.ie> wrote:
> 0001 (the freezing strategies patch) is now committable IMV. Or at
> least will be once I polish the docs a bit more. I plan on committing
> 0001 some time next week, barring any objections.

I plan on committing 0001 (the freezing strategies commit) tomorrow
morning, US Pacific time.

Attached is v17. There are no significant differences compared to v17.
I decided to post a new version now, ahead of commit, to show how I've
cleaned up the docs in 0001 -- docs describing the new GUC, freeze
strategies, and so on.

--
Peter Geoghegan

On Thu, Jan 26, 2023 at 10:11 AM Andres Freund <andres@anarazel.de> wrote:

> I am. Just not every tradeoff. I just don't see any useful tradeoffs purely
> based on the relation size.

I expressed reservations about relation size six weeks ago:

On Wed, Dec 14, 2022 at 12:16 AM Peter Geoghegan <pg@bowt.ie> wrote:
>
> On Tue, Dec 13, 2022 at 12:29 AM John Naylor
> <john.naylor@enterprisedb.com> wrote:
> > If the number of unfrozen heap pages is the thing we care about, perhaps that, and not the total size of the table, should be the parameter that drives freezing strategy?
>
> That's not the only thing we care about, though.

That was followed by several paragraphs that never got around to explaining why table size should drive freezing strategy. Review is a feedback mechanism alerting the patch author to possible problems. Listening to feedback is like vacuum, in a way: If it hurts, you're not doing it enough.

--
John Naylor
EDB: http://www.enterprisedb.com

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

26 January 2023, 07:24:35

On Wed, Jan 25, 2023 at 7:41 PM Robert Haas <robertmhaas@gmail.com> wrote:
> Both Andres and I have repeatedly expressed concern about how much is
> being changed in the behavior of vacuum, and how quickly, and IMHO on
> the basis of very limited evidence that the changes are improvements.
> The fact that Andres was very quickly able to find cases where the
> patch produces large regression is just more evidence of that. It's
> also hard to even understand what has been changed, because the
> descriptions are so theoretical.

Did you actually read the motivating examples Wiki page?

> I think we're on a very dangerous path here. I want VACUUM to be
> better as the next person, but I really don't believe that's the
> direction we're headed. I think if we release like this, we're going
> to experience more VACUUM pain, not less. And worse still, I don't
> think anyone other than Peter and Andres is going to understand why
> it's happening.

I think that the only sensible course of action at this point is for
me to revert the page-level freezing commit from today, and abandon
all outstanding work on VACUUM. I will still stand by the basic
page-level freezing work, at least to the extent that I am able to.
Honestly, just typing that makes me feel a big sense of relief.

I am a proud, stubborn man. While the experience of working on the
earlier related stuff for Postgres 15 was itself enough to make me
seriously reassess my choice to work on VACUUM in general, I still
wanted to finish off what I'd started. I don't see how that'll be
possible now -- I'm just not in a position to be in the center of
another controversy, and I just don't seem to be able to avoid them
here, as a practical matter. I will resolve to be a less stubborn
person. I don't have the constitution for it anymore.

-- 
Peter Geoghegan

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

26 January 2023, 07:36:11

On Wed, Jan 25, 2023 at 8:12 PM John Naylor
<john.naylor@enterprisedb.com> wrote:
> That was followed by several paragraphs that never got around to explaining why table size should drive freezing
strategy.

You were talking about the system level view of freeze debt, and how
the table view might not be a sufficient proxy for that. What does
that have to do with anything that we've discussed on this thread
recently?

> Review is a feedback mechanism alerting the patch author to possible problems. Listening to feedback is like vacuum,
ina way: If it hurts, you're not doing it enough.

An elegant analogy.

-- 
Peter Geoghegan

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

26 January 2023, 09:38:49

On Wed, Jan 25, 2023 at 8:24 PM Peter Geoghegan <pg@bowt.ie> wrote:
> > I think we're on a very dangerous path here. I want VACUUM to be
> > better as the next person, but I really don't believe that's the
> > direction we're headed. I think if we release like this, we're going
> > to experience more VACUUM pain, not less. And worse still, I don't
> > think anyone other than Peter and Andres is going to understand why
> > it's happening.
>
> I think that the only sensible course of action at this point is for
> me to revert the page-level freezing commit from today, and abandon
> all outstanding work on VACUUM. I will still stand by the basic
> page-level freezing work, at least to the extent that I am able to.

I have now reverted today's commit. I have also withdrawn all
remaining work from the patch series as a whole, which is reflected in
the CF app. Separately, I have withdrawn 2 other VACUUM related
patches of mine via the CF app: the antiwraparound autovacuum patch
series, plus a patch that did some further work on freezing
MultiXacts.

I have no intention of picking any of these patches back up again. I
also intend to completely avoid new work on both VACUUM and
autovacuum, not including ambulkdelete() code run by index access
methods. I will continue to do maintenance and bugfix work when it
happens to involve VACUUM, though.

For the record, in case it matters: I certainly have no objection to
anybody else picking up any of this unfinished work for themselves, in
part or in full.

--
Peter Geoghegan

Re: New strategies for freezing, advancing relfrozenxid early

From

Robert Haas

Date:

26 January 2023, 16:41:06

On Wed, Jan 25, 2023 at 11:25 PM Peter Geoghegan <pg@bowt.ie> wrote:
> On Wed, Jan 25, 2023 at 7:41 PM Robert Haas <robertmhaas@gmail.com> wrote:
> > Both Andres and I have repeatedly expressed concern about how much is
> > being changed in the behavior of vacuum, and how quickly, and IMHO on
> > the basis of very limited evidence that the changes are improvements.
> > The fact that Andres was very quickly able to find cases where the
> > patch produces large regression is just more evidence of that. It's
> > also hard to even understand what has been changed, because the
> > descriptions are so theoretical.
>
> Did you actually read the motivating examples Wiki page?

I don't know. I've read a lot of stuff that you've written on this
topic, which has taken a significant amount of time, and I still don't
understand a lot of what you're changing, and I don't agree with all
of the things that I do understand. I can't state with confidence that
the motivating examples wiki page was or was not among the things that
I read. But, you know, when people start running PostgreSQL 16, and
have some problem, they're not going to read the motivating examples
wiki page. They're going to read the documentation. If they can't find
the answer there, they (or some hacker that they contact) will
probably read the code comments and the relevant commit messages.
Those either clearly explain what was changed in a way that somebody
can understand, or they don't. If they don't, *the commits are not
good enough*, regardless of what other information may exist in any
other place.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

Re: New strategies for freezing, advancing relfrozenxid early

From

Robert Haas

Date:

26 January 2023, 17:20:57

On Wed, Jan 25, 2023 at 10:56 PM Andres Freund <andres@anarazel.de> wrote:
> but that's only true because page level freezing neutered
> vacuum_freeze_min_age. Compared to <16, it's a *huge* change.

Do you think that page-level freezing
(1de58df4fec7325d91f5a8345757314be7ac05da) was improvidently
committed?

I have always been a bit skeptical of vacuum_freeze_min_age as a
mechanism. It's certainly true that it is a waste of energy to freeze
tuples that will soon be removed anyway, but on the other hand,
repeatedly dirtying the same page for various different freezing and
visibility related reasons *really sucks*, and even repeatedly reading
the page because we kept deciding not to do anything yet isn't great.
It seems possible that the page-level freezing mechanism could help
with that quite a bit, and I think that the heuristic that patch
proposes is basically reasonable: if there's at least one tuple on the
page that is old enough to justify freezing, it doesn't seem like a
bad bet to freeze all the others that can be frozen at the same time,
at least if it means that we can mark the page all-visible or
all-frozen. If it doesn't, then I'm not so sure; maybe we're best off
deferring as much work as possible to a time when we *can* mark the
page all-visible or all-frozen.

In short, I think that neutering vacuum_freeze_min_age at least to
some degree might be a good thing, but that's not to say that I'm
altogether confident in that patch, either.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

26 January 2023, 19:24:21

On Wed, Jan 25, 2023 at 7:56 PM Andres Freund <andres@anarazel.de> wrote:
> > https://wiki.postgresql.org/wiki/Freezing/skipping_strategies_patch:_motivating_examples#Patch_2
> >
> > The difference between this and VACUUM FREEZE is described here:
> >
> > "Note how we freeze most pages, but still leave a significant number
> > unfrozen each time, despite using an eager approach to freezing
> > (2981204 scanned - 2355230 frozen = 625974 pages scanned but left
> > unfrozen). Again, this is because we don't freeze pages unless they're
> > already eligible to be set all-visible.
>
> The only reason there is a substantial difference is because of pgbench's
> uniform access pattern. Most real-world applications don't have that.

It's not pgbench! It's TPC-C. It's actually an adversarial case for
the patch series.

-- 
Peter Geoghegan

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

26 January 2023, 19:35:04

On Thu, Jan 26, 2023 at 5:41 AM Robert Haas <robertmhaas@gmail.com> wrote:
> On Wed, Jan 25, 2023 at 11:25 PM Peter Geoghegan <pg@bowt.ie> wrote:
> > On Wed, Jan 25, 2023 at 7:41 PM Robert Haas <robertmhaas@gmail.com> wrote:
> > > Both Andres and I have repeatedly expressed concern about how much is
> > > being changed in the behavior of vacuum, and how quickly, and IMHO on
> > > the basis of very limited evidence that the changes are improvements.
> > > The fact that Andres was very quickly able to find cases where the
> > > patch produces large regression is just more evidence of that. It's
> > > also hard to even understand what has been changed, because the
> > > descriptions are so theoretical.
> >
> > Did you actually read the motivating examples Wiki page?
>
> I don't know. I've read a lot of stuff that you've written on this
> topic, which has taken a significant amount of time, and I still don't
> understand a lot of what you're changing, and I don't agree with all
> of the things that I do understand.

You complained about the descriptions being theoretical. But there's
nothing theoretical about the fact that we more or less do *all*
freezing in an eventual aggressive VACUUM in many important cases,
including very simple cases like pgbench_history -- the simplest
possible append-only table case. We'll merrily rewrite the entire
table, all at once, for no good reason at all. Consistently, reliably.
It's so incredibly obvious that this makes zero sense! And yet I don't
think you've ever engaged with such basic points as that one.

-- 
Peter Geoghegan

Re: New strategies for freezing, advancing relfrozenxid early

From

Andres Freund

Date:

26 January 2023, 19:35:45

Hi,

On 2023-01-26 09:20:57 -0500, Robert Haas wrote:
> On Wed, Jan 25, 2023 at 10:56 PM Andres Freund <andres@anarazel.de> wrote:
> > but that's only true because page level freezing neutered
> > vacuum_freeze_min_age. Compared to <16, it's a *huge* change.
> 
> Do you think that page-level freezing
> (1de58df4fec7325d91f5a8345757314be7ac05da) was improvidently
> committed?

I think it's probably ok, but perhaps deserves a bit more thought about when
to "opportunistically" freeze. Perhaps to make it *more* aggressive than it's
now.

With "opportunistic freezing" I mean freezing the page, even though we don't
*have* to freeze any of the tuples.

The overall condition gating freezing is:
    if (pagefrz.freeze_required || tuples_frozen == 0 ||
        (prunestate->all_visible && prunestate->all_frozen &&
         fpi_before != pgWalUsage.wal_fpi))

fpi_before is set before the heap_page_prune() call.

To me the
  fpi_before != pgWalUsage.wal_fpi"
part doesn't make a whole lot of sense. For one, it won't at all work if
full_page_writes=off. But more importantly, it also means we'll not freeze
when VACUUMing a recently modified page, even if pruning already emitted a WAL
record and we'd not emit an FPI if we freezed the page now.

To me a condition that checked if the buffer is already dirty and if another
XLogInsert() would be likely to generate an FPI would make more sense. The
rare race case of a checkpoint starting concurrently doesn't matter IMO.

A minor complaint I have about the code is that the "tuples_frozen == 0" path
imo is confusing. We go into the "freeze" path, which then inside has another
if for the tuples_frozen == 0 part. I get that this deduplicates the
NewRelFrozenXid handling, but it still looks odd.

> I have always been a bit skeptical of vacuum_freeze_min_age as a
> mechanism. It's certainly true that it is a waste of energy to freeze
> tuples that will soon be removed anyway, but on the other hand,
> repeatedly dirtying the same page for various different freezing and
> visibility related reasons *really sucks*, and even repeatedly reading
> the page because we kept deciding not to do anything yet isn't great.
> It seems possible that the page-level freezing mechanism could help
> with that quite a bit, and I think that the heuristic that patch
> proposes is basically reasonable: if there's at least one tuple on the
> page that is old enough to justify freezing, it doesn't seem like a
> bad bet to freeze all the others that can be frozen at the same time,
> at least if it means that we can mark the page all-visible or
> all-frozen. If it doesn't, then I'm not so sure; maybe we're best off
> deferring as much work as possible to a time when we *can* mark the
> page all-visible or all-frozen.

Agreed. Freezing everything if we need to freeze some things seems quite safe
to me.

> In short, I think that neutering vacuum_freeze_min_age at least to
> some degree might be a good thing, but that's not to say that I'm
> altogether confident in that patch, either.

I am not too woried about the neutering in the page level freezing patch.

The combination of the page level work with the eager strategy is where the
sensibly-more-aggressive freeze_min_age got turbocharged to an imo dangerous
degree.

Greetings,

Andres Freund

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

26 January 2023, 19:54:55

On Thu, Jan 26, 2023 at 8:35 AM Andres Freund <andres@anarazel.de> wrote:
> I think it's probably ok, but perhaps deserves a bit more thought about when
> to "opportunistically" freeze. Perhaps to make it *more* aggressive than it's
> now.
>
> With "opportunistic freezing" I mean freezing the page, even though we don't
> *have* to freeze any of the tuples.
>
> The overall condition gating freezing is:
>         if (pagefrz.freeze_required || tuples_frozen == 0 ||
>                 (prunestate->all_visible && prunestate->all_frozen &&
>                  fpi_before != pgWalUsage.wal_fpi))
>
> fpi_before is set before the heap_page_prune() call.

Have you considered page-level checksums, and how the impact on hint
bits needs to be accounted for here?

All RDS customers use page-level checksums. And I've noticed that it's
very common for the number of FPIs to only be very slightly less than
the number of pages dirtied. Much of which is just hint bits. The
"fpi_before != pgWalUsage.wal_fpi" test catches that.

> To me a condition that checked if the buffer is already dirty and if another
> XLogInsert() would be likely to generate an FPI would make more sense. The
> rare race case of a checkpoint starting concurrently doesn't matter IMO.

That's going to be very significantly more aggressive. For example
it'll impact small tables very differently.

--
Peter Geoghegan

Re: New strategies for freezing, advancing relfrozenxid early

From

Andres Freund

Date:

26 January 2023, 20:53:34

Hi,

On 2023-01-26 08:54:55 -0800, Peter Geoghegan wrote:
> On Thu, Jan 26, 2023 at 8:35 AM Andres Freund <andres@anarazel.de> wrote:
> > I think it's probably ok, but perhaps deserves a bit more thought about when
> > to "opportunistically" freeze. Perhaps to make it *more* aggressive than it's
> > now.
> >
> > With "opportunistic freezing" I mean freezing the page, even though we don't
> > *have* to freeze any of the tuples.
> >
> > The overall condition gating freezing is:
> >         if (pagefrz.freeze_required || tuples_frozen == 0 ||
> >                 (prunestate->all_visible && prunestate->all_frozen &&
> >                  fpi_before != pgWalUsage.wal_fpi))
> >
> > fpi_before is set before the heap_page_prune() call.
> 
> Have you considered page-level checksums, and how the impact on hint
> bits needs to be accounted for here?
> 
> All RDS customers use page-level checksums. And I've noticed that it's
> very common for the number of FPIs to only be very slightly less than
> the number of pages dirtied. Much of which is just hint bits. The
> "fpi_before != pgWalUsage.wal_fpi" test catches that.

I assume the case you're thinking of is that pruning did *not* do any changes,
but in the process of figuring out that nothing needed to be pruned, we did a
MarkBufferDirtyHint(), and as part of that emitted an FPI?

> > To me a condition that checked if the buffer is already dirty and if another
> > XLogInsert() would be likely to generate an FPI would make more sense. The
> > rare race case of a checkpoint starting concurrently doesn't matter IMO.
> 
> That's going to be very significantly more aggressive. For example
> it'll impact small tables very differently.

Maybe it would be too aggressive, not sure. The cost of a freeze WAL record is
relatively small, with one important exception below, if we are 99.99% sure
that it's not going to require an FPI and isn't going to dirty the page.

The exception is that a newer LSN on the page can cause the ringbuffer
replacement to trigger more more aggressive WAL flushing. No meaningful
difference if we modified the page during pruning, or if the page was already
in s_b (since it likely won't be written out via the ringbuffer in that case),
but if checksums are off and we just hint-dirtied the page, it could be a
significant issue.

Thus a modification of the above logic could be to opportunistically freeze if
a ) it won't cause an FPI and either
b1) the page was already dirty before pruning, as we'll not do a ringbuffer
    replacement in that case
or
b2) We wrote a WAL record during pruning, as the difference in flush position
    is marginal

An even more aggressive version would be to replace b1) with logic that'd
allow newly dirtying the page if it wasn't read through the ringbuffer. But
newly dirtying the page feels like it'd be more dangerous.

A less aggressive version would be to check if any WAL records were emitted
during heap_page_prune() (instead of FPIs) and whether we'd emit an FPI if we
modified the page again. Similar to what we do now, except not requiring an
FPI to have been emitted.

But to me it seems a bit odd that VACUUM now is more aggressive if checksums /
wal_log_hint bits is on, than without them. Which I think is how using either
of pgWalUsage.wal_fpi, pgWalUsage.wal_records ends up working?

Greetings,

Andres Freund

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

26 January 2023, 21:44:45

On Thu, Jan 26, 2023 at 9:53 AM Andres Freund <andres@anarazel.de> wrote:
> I assume the case you're thinking of is that pruning did *not* do any changes,
> but in the process of figuring out that nothing needed to be pruned, we did a
> MarkBufferDirtyHint(), and as part of that emitted an FPI?

Yes.

> > That's going to be very significantly more aggressive. For example
> > it'll impact small tables very differently.
>
> Maybe it would be too aggressive, not sure. The cost of a freeze WAL record is
> relatively small, with one important exception below, if we are 99.99% sure
> that it's not going to require an FPI and isn't going to dirty the page.
>
> The exception is that a newer LSN on the page can cause the ringbuffer
> replacement to trigger more more aggressive WAL flushing. No meaningful
> difference if we modified the page during pruning, or if the page was already
> in s_b (since it likely won't be written out via the ringbuffer in that case),
> but if checksums are off and we just hint-dirtied the page, it could be a
> significant issue.

Most of the overhead of FREEZE WAL records (with freeze plan
deduplication and page-level freezing in) is generic WAL record header
overhead. Your recent adversarial test case is going to choke on that,
too. At least if you set checkpoint_timeout to 1 minute again.

> Thus a modification of the above logic could be to opportunistically freeze if
> a ) it won't cause an FPI and either
> b1) the page was already dirty before pruning, as we'll not do a ringbuffer
>     replacement in that case
> or
> b2) We wrote a WAL record during pruning, as the difference in flush position
>     is marginal
>
> An even more aggressive version would be to replace b1) with logic that'd
> allow newly dirtying the page if it wasn't read through the ringbuffer. But
> newly dirtying the page feels like it'd be more dangerous.

In many cases we'll have to dirty the page anyway, just to set
PD_ALL_VISIBLE. The whole way the logic works is conditioned (whether
triggered by an FPI or triggered by my now-reverted GUC) on being able
to set the whole page all-frozen in the VM.

> A less aggressive version would be to check if any WAL records were emitted
> during heap_page_prune() (instead of FPIs) and whether we'd emit an FPI if we
> modified the page again. Similar to what we do now, except not requiring an
> FPI to have been emitted.

Also way more aggressive. Not nearly enough on its own.

> But to me it seems a bit odd that VACUUM now is more aggressive if checksums /
> wal_log_hint bits is on, than without them. Which I think is how using either
> of pgWalUsage.wal_fpi, pgWalUsage.wal_records ends up working?

Which part is the odd part? Is it odd that page-level freezing works
that way, or is it odd that page-level checksums work that way?

In any case this seems like an odd thing for you to say, having
eviscerated a patch that really just made the same behavior trigger
independently of FPIs in some tables, controlled via a GUC.

-- 
Peter Geoghegan

Re: New strategies for freezing, advancing relfrozenxid early

From

Matthias van de Meent

Date:

26 January 2023, 22:26:00

On Thu, 26 Jan 2023 at 19:45, Peter Geoghegan <pg@bowt.ie> wrote:
>
> On Thu, Jan 26, 2023 at 9:53 AM Andres Freund <andres@anarazel.de> wrote:
> > I assume the case you're thinking of is that pruning did *not* do any changes,
> > but in the process of figuring out that nothing needed to be pruned, we did a
> > MarkBufferDirtyHint(), and as part of that emitted an FPI?
>
> Yes.
>
> > > That's going to be very significantly more aggressive. For example
> > > it'll impact small tables very differently.
> >
> > Maybe it would be too aggressive, not sure. The cost of a freeze WAL record is
> > relatively small, with one important exception below, if we are 99.99% sure
> > that it's not going to require an FPI and isn't going to dirty the page.
> >
> > The exception is that a newer LSN on the page can cause the ringbuffer
> > replacement to trigger more more aggressive WAL flushing. No meaningful
> > difference if we modified the page during pruning, or if the page was already
> > in s_b (since it likely won't be written out via the ringbuffer in that case),
> > but if checksums are off and we just hint-dirtied the page, it could be a
> > significant issue.
>
> Most of the overhead of FREEZE WAL records (with freeze plan
> deduplication and page-level freezing in) is generic WAL record header
> overhead. Your recent adversarial test case is going to choke on that,
> too. At least if you set checkpoint_timeout to 1 minute again.

Could someone explain to me why we don't currently (optionally)
include the functionality of page freezing in the PRUNE records? I
think they're quite closely related (in that they both execute in
VACUUM and are required for long-term system stability), and are even
more related now that we have opportunistic page-level freezing. I
think adding a "freeze this page as well"-flag in PRUNE records would
go a long way to reducing the WAL overhead of aggressive and more
opportunistic freezing.

-Matthias

Re: New strategies for freezing, advancing relfrozenxid early

From

Robert Haas

Date:

26 January 2023, 22:27:53

On Thu, Jan 26, 2023 at 11:35 AM Peter Geoghegan <pg@bowt.ie> wrote:
> You complained about the descriptions being theoretical. But there's
> nothing theoretical about the fact that we more or less do *all*
> freezing in an eventual aggressive VACUUM in many important cases,
> including very simple cases like pgbench_history -- the simplest
> possible append-only table case. We'll merrily rewrite the entire
> table, all at once, for no good reason at all. Consistently, reliably.
> It's so incredibly obvious that this makes zero sense! And yet I don't
> think you've ever engaged with such basic points as that one.

I'm aware that that's a problem, and I agree that it sucks. I think
that what this patch does is make vacuum more aggressively, and I
expect that would help this problem. I haven't said much about that
because I don't think it's controversial. However, the patch also has
a cost, and that's what I think is controversial.

I think it's pretty much impossible to freeze more aggressively
without losing in some scenario or other. If waiting longer to freeze
would have resulted in the data getting updated again or deleted
before we froze it, then waiting longer reduces the total amount of
freezing work that ever has to be done. Freezing more aggressively
inevitably gives up some amount of that potential benefit in order to
try to secure some other benefit. It's a trade-off.

I think that the goal of a patch that makes vacuum more (or less)
aggressive should be to make the cases where we lose as obscure as
possible, and the cases where we win as broad as possible. I think
that, in order to be a good patch, it needs to be relatively difficult
to find cases where we incur a big loss. If it's easy to find a big
loss, then I think it's better to stick with the current behavior,
even if it's also easy to find a big gain. There's nothing wonderful
about the current behavior, but (to paraphrase what I think Andres has
already said several times) it's better to keep shipping code with the
same bad behavior than to put out a new major release with behaviors
that are just as bad, but different.

I feel like your emails sometimes seem to suppose that I think that
you're a bad person, or a bad developer, or that you have no good
ideas, or that you have no good ideas about this topic, or that this
topic is not important, or that we don't need to do better than we are
currently doing. I think none of those things. However, I'm also not
prepared to go all the way to the other end of the spectrum and say
that all of your ideas and everything in this patch are great. I don't
think either of those things, either.

I certainly think that freezing more aggressively in some scenarios
could be a great idea, but it seems like the patch's theory is to be
very nearly maximally aggressive in every vacuum run if the table size
is greater than some threshold, and I don't think that's right at all.
I'm not exactly sure what information we should use to decide how
aggressive to be, but I am pretty sure that the size of the table is
not it.  It's true that, for a small table, the cost of having to
eventually vacuum the whole table at once isn't going to be very high,
whereas for a large table, it will be. That line of reasoning makes a
size threshold sound reasonable. However, the amount of extra work
that we can potentially do by vacuuming more aggressively *also*
increases with the table size, which to me means using that a
criterion actually isn't sensible at all.

One idea that I've had about how to solve this problem is to try to
make vacuum try to aggressively freeze some portion of the table on
each pass, and to behave less aggressively on the rest of the table so
that, hopefully, no single vacuum does too much work. Unfortunately, I
don't really know how to do that effectively. If we knew that the
table was going to see 10 vacuums before we hit
autovacuum_freeze_max_age, we could try to have each one do 10% of the
amount of freezing that was going to need to be done rather than
letting any single vacuum do all of it, but we don't have that sort of
information. Also, even if we did have that sort of information, the
idea only works if the pages that we freeze sooner are ones that we're
not about to update or delete again, and we don't have any idea what
is likely there. In theory we could have some system that tracks how
recently each page range in a table has been modified, and direct our
freezing activity toward the ones less-recently modified on the theory
that they're not so likely to be modified again in the near future,
but in reality we have no such system. So I don't really feel like I
know what the right answer is here, yet.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

26 January 2023, 22:56:35

On Thu, Jan 26, 2023 at 11:28 AM Robert Haas <robertmhaas@gmail.com> wrote:
> I think it's pretty much impossible to freeze more aggressively
> without losing in some scenario or other. If waiting longer to freeze
> would have resulted in the data getting updated again or deleted
> before we froze it, then waiting longer reduces the total amount of
> freezing work that ever has to be done. Freezing more aggressively
> inevitably gives up some amount of that potential benefit in order to
> try to secure some other benefit. It's a trade-off.

There is no question about that.

> I think that the goal of a patch that makes vacuum more (or less)
> aggressive should be to make the cases where we lose as obscure as
> possible, and the cases where we win as broad as possible. I think
> that, in order to be a good patch, it needs to be relatively difficult
> to find cases where we incur a big loss. If it's easy to find a big
> loss, then I think it's better to stick with the current behavior,
> even if it's also easy to find a big gain.

Again, this seems totally uncontroversial. It's just incredibly vague,
and not at all actionable.

Relatively difficult for Andres, or for somebody else? What are the
real parameters here? Obviously there are no clear answers available.

> However, I'm also not
> prepared to go all the way to the other end of the spectrum and say
> that all of your ideas and everything in this patch are great. I don't
> think either of those things, either.

It doesn't matter. I'm done with it. This is not a negotiation about
what gets in and what doesn't get in.

All that I aim to do now is to draw some kind of line under the basic
page-level freezing work, since of course I'm still responsible for
that. And perhaps to defend my personal reputation.

> I certainly think that freezing more aggressively in some scenarios
> could be a great idea, but it seems like the patch's theory is to be
> very nearly maximally aggressive in every vacuum run if the table size
> is greater than some threshold, and I don't think that's right at all.

We'll systematically avoid accumulating debt past a certain point --
that's its purpose. That is, we'll avoid accumulating all-visible
pages that eventually need to be frozen.

> I'm not exactly sure what information we should use to decide how
> aggressive to be, but I am pretty sure that the size of the table is
> not it.  It's true that, for a small table, the cost of having to
> eventually vacuum the whole table at once isn't going to be very high,
> whereas for a large table, it will be. That line of reasoning makes a
> size threshold sound reasonable. However, the amount of extra work
> that we can potentially do by vacuuming more aggressively *also*
> increases with the table size, which to me means using that a
> criterion actually isn't sensible at all.

The overwhelming cost is usually FPIs in any case. If you're not
mostly focussing on that, you're focussing on the wrong thing. At
least with larger tables. You just have to focus on the picture over
time, across multiple VACUUM operations.

> One idea that I've had about how to solve this problem is to try to
> make vacuum try to aggressively freeze some portion of the table on
> each pass, and to behave less aggressively on the rest of the table so
> that, hopefully, no single vacuum does too much work. Unfortunately, I
> don't really know how to do that effectively.

That has been proposed a couple of times in the context of this
thread. It won't work, because the way autovacuum works in general
(and likely always will work) doesn't allow it. With an append-only
table, each VACUUM will naturally have to scan significantly more
pages than the last one, forever (barring antiwraparound vacuums). Why
wouldn't it continue that way? I mean it might not (the table might
stop growing altogether), but then it doesn't matter much what we do.

If you're not behaving very proactively at the level of each VACUUM
operation, then the picture over time is that you're *already* falling
behind. At least with an append-only table. You have to think of the
sequence of operations, not just one.

> In theory we could have some system that tracks how
> recently each page range in a table has been modified, and direct our
> freezing activity toward the ones less-recently modified on the theory
> that they're not so likely to be modified again in the near future,
> but in reality we have no such system. So I don't really feel like I
> know what the right answer is here, yet.

So we need to come up with a way of getting reliable information from
the future, about an application that we have no particular
understanding of. As opposed to just eating the cost to some degree,
and making it configurable.

-- 
Peter Geoghegan

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

26 January 2023, 23:32:01

On Thu, Jan 26, 2023 at 11:26 AM Matthias van de Meent
<boekewurm+postgres@gmail.com> wrote:
> Could someone explain to me why we don't currently (optionally)
> include the functionality of page freezing in the PRUNE records? I
> think they're quite closely related (in that they both execute in
> VACUUM and are required for long-term system stability), and are even
> more related now that we have opportunistic page-level freezing. I
> think adding a "freeze this page as well"-flag in PRUNE records would
> go a long way to reducing the WAL overhead of aggressive and more
> opportunistic freezing.

Yeah, we've talked about doing that in the past year. It's quite
possible. It would make quite a lot of sense, because the actual
overhead of the WAL record for freezing tends to come from the generic
WAL record header stuff itself. If there was only one record for both,
then you'd only need to include the relfilenode and block number (and
so on) once.

It would be tricky to handle Multis, so what you'd probably do is just
freezing xmin, and possibly aborted and locker XIDs in xmax. So you
wouldn't completely get rid of the main freeze record, but would be
able to avoid it in many important cases.

-- 
Peter Geoghegan

Re: New strategies for freezing, advancing relfrozenxid early

From

Andres Freund

Date:

26 January 2023, 23:45:14

Hi,

On 2023-01-26 10:44:45 -0800, Peter Geoghegan wrote:
> On Thu, Jan 26, 2023 at 9:53 AM Andres Freund <andres@anarazel.de> wrote:
> > > That's going to be very significantly more aggressive. For example
> > > it'll impact small tables very differently.
> >
> > Maybe it would be too aggressive, not sure. The cost of a freeze WAL record is
> > relatively small, with one important exception below, if we are 99.99% sure
> > that it's not going to require an FPI and isn't going to dirty the page.
> >
> > The exception is that a newer LSN on the page can cause the ringbuffer
> > replacement to trigger more more aggressive WAL flushing. No meaningful
> > difference if we modified the page during pruning, or if the page was already
> > in s_b (since it likely won't be written out via the ringbuffer in that case),
> > but if checksums are off and we just hint-dirtied the page, it could be a
> > significant issue.
> 
> Most of the overhead of FREEZE WAL records (with freeze plan
> deduplication and page-level freezing in) is generic WAL record header
> overhead. Your recent adversarial test case is going to choke on that,
> too. At least if you set checkpoint_timeout to 1 minute again.

I don't quite follow. What do you mean with "record header overhead"? Unless
that includes FPIs, I don't think that's that commonly true?

The problematic case I am talking about is when we do *not* emit a WAL record
during pruning (because there's nothing to prune), but want to freeze the
table. If you don't log an FPI, the remaining big overhead is that increasing
the LSN on the page will often cause an XLogFlush() when writing out the
buffer.

I don't see what your reference to checkpoint timeout is about here?

Also, as I mentioned before, the problem isn't specific to checkpoint_timeout
= 1min. It just makes it cheaper to reproduce.

> > Thus a modification of the above logic could be to opportunistically freeze if
> > a ) it won't cause an FPI and either
> > b1) the page was already dirty before pruning, as we'll not do a ringbuffer
> >     replacement in that case
> > or
> > b2) We wrote a WAL record during pruning, as the difference in flush position
> >     is marginal
> >
> > An even more aggressive version would be to replace b1) with logic that'd
> > allow newly dirtying the page if it wasn't read through the ringbuffer. But
> > newly dirtying the page feels like it'd be more dangerous.
> 
> In many cases we'll have to dirty the page anyway, just to set
> PD_ALL_VISIBLE. The whole way the logic works is conditioned (whether
> triggered by an FPI or triggered by my now-reverted GUC) on being able
> to set the whole page all-frozen in the VM.

IIRC setting PD_ALL_VISIBLE doesn't trigger an FPI unless we need to log hint
bits. But freezing does trigger one even without wal_log_hint_bits.

You're right, it makes sense to consider whether we'll emit a
XLOG_HEAP2_VISIBLE anyway.

> > A less aggressive version would be to check if any WAL records were emitted
> > during heap_page_prune() (instead of FPIs) and whether we'd emit an FPI if we
> > modified the page again. Similar to what we do now, except not requiring an
> > FPI to have been emitted.
> 
> Also way more aggressive. Not nearly enough on its own.

In which cases will it be problematically more aggressive?

If we emitted a WAL record during pruning we've already set the LSN of the
page to a very recent LSN. We know the page is dirty. Thus we'll already
trigger an XLogFlush() during ringbuffer replacement. We won't emit an FPI.

> > But to me it seems a bit odd that VACUUM now is more aggressive if checksums /
> > wal_log_hint bits is on, than without them. Which I think is how using either
> > of pgWalUsage.wal_fpi, pgWalUsage.wal_records ends up working?
> 
> Which part is the odd part? Is it odd that page-level freezing works
> that way, or is it odd that page-level checksums work that way?

That page-level freezing works that way.

> In any case this seems like an odd thing for you to say, having
> eviscerated a patch that really just made the same behavior trigger
> independently of FPIs in some tables, controlled via a GUC.

jdksjfkjdlkajsd;lfkjasd;lkfj;alskdfj

That behaviour I critizied was causing a torrent of FPIs and additional
dirtying of pages. My proposed replacement for the current FPI check doesn't,
because a) it only triggers when we wrote a WAL record b) It doesn't trigger
if we would write an FPI.

Greetings,

Andres Freund

Re: New strategies for freezing, advancing relfrozenxid early

From

Robert Haas

Date:

26 January 2023, 23:54:17

On Thu, Jan 26, 2023 at 2:57 PM Peter Geoghegan <pg@bowt.ie> wrote:
> Relatively difficult for Andres, or for somebody else? What are the
> real parameters here? Obviously there are no clear answers available.

Andres is certainly smarter than the average guy, but practically any
scenario that someone can create in a few lines of SQL is something to
which code will be exposed to on some real-world system. If Andres
came along and said, hey, well I found a way to make this patch suck,
and proceeded to describe a scenario that involved a complex set of
tables and multiple workloads running simultaneously and using a
debugger to trigger some race condition and whatever, I'd be like "OK,
but is that really going to happen?". The actual scenario he came up
with is three lines of SQL, and it's nothing remotely obscure. That
kind of thing is going to happen *all the time*.

> The overwhelming cost is usually FPIs in any case. If you're not
> mostly focussing on that, you're focussing on the wrong thing. At
> least with larger tables. You just have to focus on the picture over
> time, across multiple VACUUM operations.

I think that's all mostly true, but the cases where being more
aggressive can cause *extra* FPIs are worthy of just as much attention
as the cases where we can reduce them.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

Re: New strategies for freezing, advancing relfrozenxid early

From

Andres Freund

Date:

26 January 2023, 23:55:10

Hi,

On 2023-01-26 20:26:00 +0100, Matthias van de Meent wrote:
> Could someone explain to me why we don't currently (optionally)
> include the functionality of page freezing in the PRUNE records?

I think we definitely should (and have argued for it a couple times). It's not
just about reducing WAL overhead, it's also about reducing redundant
visibility checks - which are where a very significant portion of the CPU time
for VACUUMing goes to.

Besides performance considerations, it's also just plain weird that
lazy_scan_prune() can end up with a different visibility than
heap_page_prune() (mostly due to concurrent aborts).

The number of WAL records we often end up emitting for a processing a single
page in vacuum is just plain absurd:
- PRUNE
- FREEZE_PAGE
- VISIBLE

There's afaict no justification whatsoever for these to be separate records.

> I think they're quite closely related (in that they both execute in VACUUM
> and are required for long-term system stability), and are even more related
> now that we have opportunistic page-level freezing. I think adding a "freeze
> this page as well"-flag in PRUNE records would go a long way to reducing the
> WAL overhead of aggressive and more opportunistic freezing.

Yep.

I think we should also seriously consider setting all-visible during on-access
pruning, and freezing rows during on-access pruning.

Greetings,

Andres Freund

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

27 January 2023, 00:06:31

On Thu, Jan 26, 2023 at 12:54 PM Robert Haas <robertmhaas@gmail.com> wrote:
> > The overwhelming cost is usually FPIs in any case. If you're not
> > mostly focussing on that, you're focussing on the wrong thing. At
> > least with larger tables. You just have to focus on the picture over
> > time, across multiple VACUUM operations.
>
> I think that's all mostly true, but the cases where being more
> aggressive can cause *extra* FPIs are worthy of just as much attention
> as the cases where we can reduce them.

It's a question of our exposure to real problems, in no small part.
What can we afford to be wrong about? What problem can be fixed by the
user more or less as it emerges, and what problem doesn't have that
quality?

There is very good reason to believe that the large majority of all
data that people store in a system like Postgres is extremely cold
data:

https://www.microsoft.com/en-us/research/video/cost-performance-in-modern-data-stores-how-data-cashing-systems-succeed/
https://brandur.org/fragments/events

Having a separate aggressive step that rewrites an entire large table,
apparently at random, is just a huge burden to users. You've said that
you agree that it sucks, but somehow I still can't shake the feeling
that you don't fully understand just how much it sucks.

-- 
Peter Geoghegan

Re: New strategies for freezing, advancing relfrozenxid early

From

Robert Haas

Date:

27 January 2023, 00:21:54

On Thu, Jan 26, 2023 at 4:06 PM Peter Geoghegan <pg@bowt.ie> wrote:
> There is very good reason to believe that the large majority of all
> data that people store in a system like Postgres is extremely cold
> data:

The systems where I end up troubleshooting problems seem to be, most
typically, busy OLTP systems. I'm not in a position to say whether
that's more or less common than systems with extremely cold data, but
I am in a position to say that my employer will have a lot fewer happy
customers if we regress that use case. Naturally I'm keen to avoid
that.

> Having a separate aggressive step that rewrites an entire large table,
> apparently at random, is just a huge burden to users. You've said that
> you agree that it sucks, but somehow I still can't shake the feeling
> that you don't fully understand just how much it sucks.

Ha!

Well, that's possible. But maybe you don't understand how much your
patch makes other things suck.

I don't think we can really get anywhere here by postulating that the
problem is the other person's lack of understanding, even if such a
postulate should happen to be correct.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

27 January 2023, 00:51:03

On Thu, Jan 26, 2023 at 1:22 PM Robert Haas <robertmhaas@gmail.com> wrote:
> On Thu, Jan 26, 2023 at 4:06 PM Peter Geoghegan <pg@bowt.ie> wrote:
> > There is very good reason to believe that the large majority of all
> > data that people store in a system like Postgres is extremely cold
> > data:
>
> The systems where I end up troubleshooting problems seem to be, most
> typically, busy OLTP systems. I'm not in a position to say whether
> that's more or less common than systems with extremely cold data, but
> I am in a position to say that my employer will have a lot fewer happy
> customers if we regress that use case. Naturally I'm keen to avoid
> that.

This is the kind of remark that makes me think that you don't get it.

The most influential OLTP benchmark of all time is TPC-C, which has
exactly this problem. In spades -- it's enormously disruptive. Which
is one reason why I used it as a showcase for a lot of this work. Plus
practical experience (like the Heroku database in the blog post I
linked to) fully agrees with that benchmark, as far as this stuff goes
-- that was also a busy OLTP database.

Online transaction involves transactions. Right? There is presumably
some kind of ledger, some kind of orders table. Naturally these have
entries that age out fairly predictably. After a while, almost all the
data is cold data. It is usually about that simple.

One of the key strengths of systems like Postgres is the ability to
inexpensively store a relatively large amount of data that has just
about zero chance of being read, let alone modified. While at the same
time having decent OLTP performance for the hot data. Not nearly as
good as an in-memory system, mind you -- and yet in-memory systems
remain largely a niche thing.

-- 
Peter Geoghegan

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

27 January 2023, 02:36:52

On Thu, Jan 26, 2023 at 12:45 PM Andres Freund <andres@anarazel.de> wrote:
> > Most of the overhead of FREEZE WAL records (with freeze plan
> > deduplication and page-level freezing in) is generic WAL record header
> > overhead. Your recent adversarial test case is going to choke on that,
> > too. At least if you set checkpoint_timeout to 1 minute again.
>
> I don't quite follow. What do you mean with "record header overhead"? Unless
> that includes FPIs, I don't think that's that commonly true?

Even if there are no directly observable FPIs, there is still extra
WAL, which can cause FPIs indirectly, just by making checkpoints more
frequent. I feel ridiculous even having to explain this to you.

> The problematic case I am talking about is when we do *not* emit a WAL record
> during pruning (because there's nothing to prune), but want to freeze the
> table. If you don't log an FPI, the remaining big overhead is that increasing
> the LSN on the page will often cause an XLogFlush() when writing out the
> buffer.
>
> I don't see what your reference to checkpoint timeout is about here?
>
> Also, as I mentioned before, the problem isn't specific to checkpoint_timeout
> = 1min. It just makes it cheaper to reproduce.

That's flagrantly intellectually dishonest. Sure, it made it easier to
reproduce. But that's not all it did!

You had *lots* of specific numbers and technical details in your first
email, such as "Time for vacuuming goes up to ~5x. WAL volume to
~9x.". But you did not feel that it was worth bothering with details
like having set checkpoint_timeout to 1 minute, which is a setting
that nobody uses, and obviously had a multiplicative effect. That
detail was unimportant. I had to drag it out of you!

You basically found a way to add WAL overhead to a system/workload
that is already in a write amplification vicious cycle, with latent
tipping point type behavior.

There is a practical point here, that is equally obvious, and yet
somehow still needs to be said: benchmarks like that one are basically
completely free of useful information. If we can't agree on how to
assess such things in general, then what can we agree on when it comes
to what should be done about it, what trade-off to make, when it comes
to any similar question?

> > In many cases we'll have to dirty the page anyway, just to set
> > PD_ALL_VISIBLE. The whole way the logic works is conditioned (whether
> > triggered by an FPI or triggered by my now-reverted GUC) on being able
> > to set the whole page all-frozen in the VM.
>
> IIRC setting PD_ALL_VISIBLE doesn't trigger an FPI unless we need to log hint
> bits. But freezing does trigger one even without wal_log_hint_bits.

That is correct.

> You're right, it makes sense to consider whether we'll emit a
> XLOG_HEAP2_VISIBLE anyway.

As written the page-level freezing FPI mechanism probably doesn't
really stand to benefit much from doing that. Either checksums are
disabled and it's just a hint, or they're enabled and there is a very
high chance that we'll get an FPI inside lazy_scan_prune rather than
right after it is called, when PD_ALL_VISIBLE is set.

That's not perfect, of course, but it doesn't have to be. Perhaps it
should still be improved, just on general principle.

> > > A less aggressive version would be to check if any WAL records were emitted
> > > during heap_page_prune() (instead of FPIs) and whether we'd emit an FPI if we
> > > modified the page again. Similar to what we do now, except not requiring an
> > > FPI to have been emitted.
> >
> > Also way more aggressive. Not nearly enough on its own.
>
> In which cases will it be problematically more aggressive?
>
> If we emitted a WAL record during pruning we've already set the LSN of the
> page to a very recent LSN. We know the page is dirty. Thus we'll already
> trigger an XLogFlush() during ringbuffer replacement. We won't emit an FPI.

You seem to be talking about this as if the only thing that could
matter is the immediate FPI -- the first order effects -- and not any
second order effects. You certainly didn't get to 9x extra WAL
overhead by controlling for that before. Should I take it that you've
decided to assess these things more sensibly now? Out of curiosity:
why the change of heart?

> > > But to me it seems a bit odd that VACUUM now is more aggressive if checksums /
> > > wal_log_hint bits is on, than without them. Which I think is how using either
> > > of pgWalUsage.wal_fpi, pgWalUsage.wal_records ends up working?
> >
> > Which part is the odd part? Is it odd that page-level freezing works
> > that way, or is it odd that page-level checksums work that way?
>
> That page-level freezing works that way.

I think that it will probably cause a little confusion, and should be
specifically documented. But other than that, it seems reasonable
enough to me. I mean, should I not do something that's going to be of
significant help to users with checksums, just because it'll be
somewhat confusing to a small minority of them?

> > In any case this seems like an odd thing for you to say, having
> > eviscerated a patch that really just made the same behavior trigger
> > independently of FPIs in some tables, controlled via a GUC.
>
> jdksjfkjdlkajsd;lfkjasd;lkfj;alskdfj
>
> That behaviour I critizied was causing a torrent of FPIs and additional
> dirtying of pages. My proposed replacement for the current FPI check doesn't,
> because a) it only triggers when we wrote a WAL record b) It doesn't trigger
> if we would write an FPI.

It increases the WAL written in many important cases that
vacuum_freeze_strategy_threshold avoided. Sure, it did have some
problems, but the general idea of adding some high level
context/strategies seems essential to me.

You also seem to be suggesting that your proposed change to how basic
page-level freezing works will make freezing of pages on databases
with page-level checksums similar to an equivalent case without
checksums enabled. Even assuming that that's an important goal, you
won't be much closer to achieving it under your scheme, since hint
bits being set during VACUUM and requiring an FPI still make a huge
difference. Tables like pgbench_history have pages that generally
aren't pruned, that don't need to log an FPI just to set
PD_ALL_VISIBLE once checksums are disabled.

That's the difference that users are going to notice between checksums
enabled vs disabled, if they notice any -- it's the most important one
by far.

--
Peter Geoghegan

Re: New strategies for freezing, advancing relfrozenxid early

From

Andres Freund

Date:

27 January 2023, 04:15:10

Hi,

On 2023-01-26 14:27:53 -0500, Robert Haas wrote:
> One idea that I've had about how to solve this problem is to try to
> make vacuum try to aggressively freeze some portion of the table on
> each pass, and to behave less aggressively on the rest of the table so
> that, hopefully, no single vacuum does too much work.

I agree that this rough direction is worthwhile to purse.

> Unfortunately, I don't really know how to do that effectively. If we knew
> that the table was going to see 10 vacuums before we hit
> autovacuum_freeze_max_age, we could try to have each one do 10% of the
> amount of freezing that was going to need to be done rather than letting any
> single vacuum do all of it, but we don't have that sort of information.

I think, quite fundamentally, it's not possible to bound the amount of work an
anti-wraparound vacuum has to do if we don't have an age based autovacuum
trigger kicking in before autovacuum_freeze_max_age. After all, there might be
no autovacuum before that's autovacuum_freeze_max_age is reached.

But there's just no reason to not have a trigger below
autovacuum_freeze_max_age. That's why I think Peter's patch to split age and
anti-"auto-cancel" autovacuums is an strictly necessary change if we want to
make autovacuum fundamentally suck less. There's a few boring details to
figure out how to set/compute those limits, but I don't think there's anything
fundamentally hard.

I think we also need the number of all-frozen pages in pg_class if we want to
make better scheduling decision. As we already compute the number of
all-visible pages at the end of vacuuming, we can compute the number of
all-frozen pages as well. The space for another integer in pg_class doesn't
bother me one bit.

Let's say we had a autovacuum_vacuum_age trigger of 100m, and
autovacuum_freeze_max_age=500m. We know that we're roughly going to be
vacuuming 5 times before reaching autovacuum_freeze_max_age (very slow
autovacuums are an issue, but if one autovacuum takes 100m+ xids long, there's
not much we can do).

With that we could determine the eager percentage along the lines of:
  frozen_target = Min(age(relfrozenxid), autovacuum_freeze_max_age)/autovacuum_freeze_max_age
  eager_percentage = Min(0, frozen_target * relpages - pg_class.relallfrozen * relpages)

One thing I don't know fully how to handle is how to ensure that we try to
freeze a different part of the table each vacuum. I guess we could store a
page number in pgstats?

This would help address the "cliff" issue of reaching
autovacuum_freeze_max_age. What it would *not*, on its own, would is the
number of times we rewrite pages.

I can guess at a few ways to heuristically identify when tables are "append
mostly" from vacuum's view (a table can be update heavy, but very localized to
recent rows, and still be append mostly from vacuum's view).  There's obvious
cases, e.g. when there are way more inserts than dead rows.  But other cases
are harder.

> Also, even if we did have that sort of information, the idea only works if
> the pages that we freeze sooner are ones that we're not about to update or
> delete again, and we don't have any idea what is likely there.

Perhaps we could use something like
  (age(relfrozenxid) - age(newest_xid_on_page)) / age(relfrozenxid)
as a heuristic?

I have a gut feeling that we should somehow collect/use statistics about the
number of frozen pages, marked as such by the last (or recent?) vacuum, that
had to be "defrosted" by backends. But I don't quite know how to yet.  I think
we could collect statistics about that by storing the LSN of the last vacuum
in the shared stats, and incrementing that counter when defrosting.

A lot of things like that would work a whole lot better if we had statistics
that take older data into account, but weigh it less than more recent
data. But that's hard/expensive to collect.

Greetings,

Andres Freund

Re: New strategies for freezing, advancing relfrozenxid early

From

Andres Freund

Date:

27 January 2023, 05:37:43

Hi,

On 2023-01-26 15:36:52 -0800, Peter Geoghegan wrote:
> On Thu, Jan 26, 2023 at 12:45 PM Andres Freund <andres@anarazel.de> wrote:
> > > Most of the overhead of FREEZE WAL records (with freeze plan
> > > deduplication and page-level freezing in) is generic WAL record header
> > > overhead. Your recent adversarial test case is going to choke on that,
> > > too. At least if you set checkpoint_timeout to 1 minute again.
> >
> > I don't quite follow. What do you mean with "record header overhead"? Unless
> > that includes FPIs, I don't think that's that commonly true?
>
> Even if there are no directly observable FPIs, there is still extra
> WAL, which can cause FPIs indirectly, just by making checkpoints more
> frequent. I feel ridiculous even having to explain this to you.

What does that have to do with "generic WAL record overhead"?


I also don't really see how that is responsive to anything else in my
email. That's just as true for the current gating condition (the issuance of
an FPI during heap_page_prune() / HTSV()).

What I was wondering about is whether we should replace the
  fpi_before != pgWalUsage.wal_fpi
with
  records_before != pgWalUsage.wal_records && !WouldIssueFpi(page)


> > The problematic case I am talking about is when we do *not* emit a WAL record
> > during pruning (because there's nothing to prune), but want to freeze the
> > table. If you don't log an FPI, the remaining big overhead is that increasing
> > the LSN on the page will often cause an XLogFlush() when writing out the
> > buffer.
> >
> > I don't see what your reference to checkpoint timeout is about here?
> >
> > Also, as I mentioned before, the problem isn't specific to checkpoint_timeout
> > = 1min. It just makes it cheaper to reproduce.
>
> That's flagrantly intellectually dishonest. Sure, it made it easier to
> reproduce. But that's not all it did!
>
> You had *lots* of specific numbers and technical details in your first
> email, such as "Time for vacuuming goes up to ~5x. WAL volume to
> ~9x.". But you did not feel that it was worth bothering with details
> like having set checkpoint_timeout to 1 minute, which is a setting
> that nobody uses, and obviously had a multiplicative effect. That
> detail was unimportant. I had to drag it out of you!

The multiples were for checkpoint_timeout=5min, with
 '250s' instead of WHERE ts < now() - '120s'

I started out with checkpoint_timeout=1min, as I wanted to quickly test my
theory. Then I increased checkpoint_timeout back to 5min, adjusted the query
to some randomly guessed value. Happened to get nearly the same results.

I then experimented more with '1min', because it's less annoying to have to
wait for 120s until deletions start, than to wait for 250s. Because it's
quicker to run I thought I'd share the less resource intensive version. A
mistake as I now realize.


This wasn't intended as a carefully designed benchmark, or anything. It was a
quick proof for a problem that I found obvious. And it's not something worth
testing carefully - e.g. the constants in the test are actually quite hardware
specific, because the insert/seconds rate is very machine specific, and it's
completely unnecessarily hardware intensive due to the use of single-row
inserts, instead of batched operations.  It's just a POC.



> You basically found a way to add WAL overhead to a system/workload
> that is already in a write amplification vicious cycle, with latent
> tipping point type behavior.
>
> There is a practical point here, that is equally obvious, and yet
> somehow still needs to be said: benchmarks like that one are basically
> completely free of useful information. If we can't agree on how to
> assess such things in general, then what can we agree on when it comes
> to what should be done about it, what trade-off to make, when it comes
> to any similar question?

It's not at all free of useful information. It reproduces a problem I
predicted repeatedly, that others in the discussion also wondered about, that
you refused to acknowledge or address.

It's not a good benchmark - I completely agree with that much. It was not
designed to carefully benchmark different settings or such. It was designed to
show a problem. And it does that.



> > You're right, it makes sense to consider whether we'll emit a
> > XLOG_HEAP2_VISIBLE anyway.
>
> As written the page-level freezing FPI mechanism probably doesn't
> really stand to benefit much from doing that. Either checksums are
> disabled and it's just a hint, or they're enabled and there is a very
> high chance that we'll get an FPI inside lazy_scan_prune rather than
> right after it is called, when PD_ALL_VISIBLE is set.

I think it might be useful with logged hint bits, consider cases where all the
tuples on the page were already fully hinted. That's not uncommon, I think?


> > > > A less aggressive version would be to check if any WAL records were emitted
> > > > during heap_page_prune() (instead of FPIs) and whether we'd emit an FPI if we
> > > > modified the page again. Similar to what we do now, except not requiring an
> > > > FPI to have been emitted.
> > >
> > > Also way more aggressive. Not nearly enough on its own.
> >
> > In which cases will it be problematically more aggressive?
> >
> > If we emitted a WAL record during pruning we've already set the LSN of the
> > page to a very recent LSN. We know the page is dirty. Thus we'll already
> > trigger an XLogFlush() during ringbuffer replacement. We won't emit an FPI.
>
> You seem to be talking about this as if the only thing that could
> matter is the immediate FPI -- the first order effects -- and not any
> second order effects.

     * Freeze the page when heap_prepare_freeze_tuple indicates that at least
     * one XID/MXID from before FreezeLimit/MultiXactCutoff is present.  Also
     * freeze when pruning generated an FPI, if doing so means that we set the
     * page all-frozen afterwards (might not happen until final heap pass).
     */
    if (pagefrz.freeze_required || tuples_frozen == 0 ||
        (prunestate->all_visible && prunestate->all_frozen &&
         fpi_before != pgWalUsage.wal_fpi))

That's just as true for this.

What I'd like to know is why the second order effects of the above are lesser
than for
    if (pagefrz.freeze_required || tuples_frozen == 0 ||
        (prunestate->all_visible && prunestate->all_frozen &&
         records_before != pgWalUsage.wal_records && !WouldIssueFpi(page)))




> You certainly didn't get to 9x extra WAL
> overhead by controlling for that before. Should I take it that you've
> decided to assess these things more sensibly now? Out of curiosity:
> why the change of heart?

Dude.

What would the point have been to invest a lot of time in a repro for a
predicted problem? It's a problem repro, not a carefully designed benchmark.



> > > In any case this seems like an odd thing for you to say, having
> > > eviscerated a patch that really just made the same behavior trigger
> > > independently of FPIs in some tables, controlled via a GUC.
> >
> > jdksjfkjdlkajsd;lfkjasd;lkfj;alskdfj
> >
> > That behaviour I critizied was causing a torrent of FPIs and additional
> > dirtying of pages. My proposed replacement for the current FPI check doesn't,
> > because a) it only triggers when we wrote a WAL record b) It doesn't trigger
> > if we would write an FPI.
>
> It increases the WAL written in many important cases that
> vacuum_freeze_strategy_threshold avoided. Sure, it did have some
> problems, but the general idea of adding some high level
> context/strategies seems essential to me.

I was discussing changing the conditions for the "oppportunistic pruning"
logic, not about a replacement for the eager freezing strategy.


Greetings,

Andres Freund

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

27 January 2023, 06:01:03

On Thu, Jan 26, 2023 at 6:37 PM Andres Freund <andres@anarazel.de> wrote:
> I also don't really see how that is responsive to anything else in my
> email. That's just as true for the current gating condition (the issuance of
> an FPI during heap_page_prune() / HTSV()).
>
> What I was wondering about is whether we should replace the
>   fpi_before != pgWalUsage.wal_fpi
> with
>   records_before != pgWalUsage.wal_records && !WouldIssueFpi(page)

I understand that. What I'm saying is that that's going to create a
huge problem of its own, unless you separately account for that
problem.

The simplest and obvious example is something like a pgbench_tellers
table. VACUUM will generally run fast enough relative to the workload
that it will set some of those pages all-visible. Now it's going to
freeze them, too. Arguably it shouldn't even be setting the pages
all-visible, but now you make that existing problem much worse.

The important point is that there doesn't seem to be any good way
around thinking about the table as a whole if you're going to freeze
speculatively. This is not the same dynamic as we see with the FPI
thing IMV -- that's not nearly so speculative as what you're talking
about, since it is speculative in roughly the same sense that eager
freezing was speculative (hence the suggestion that something like
vacuum_freeze_strategy_threshold could have a roll to play).

The FPI thing is mostly about the cost now versus the cost later on.
You're gambling that you won't get another FPI later on if you freeze
now. But the cost of a second FPI later on is so much higher than the
added cost of freezing now that that's a very favorable bet, that we
can afford to "lose" many times while still coming out ahead overall.
And even when we lose, you generally still won't have been completely
wrong -- even then there generally will indeed be a second FPI later
on for the same page, to go with everything else. This makes the
wasted freezing even less significant, on a comparative basis!

It's also likely true that an FPI in lazy_scan_prune is a much
stronger signal, but I think that the important dynamic is that we're
reasoning about "costs now vs costs later on". The asymmetry is really
important.

-- 
Peter Geoghegan

Re: New strategies for freezing, advancing relfrozenxid early

From

Andres Freund

Date:

27 January 2023, 08:58:05

Hi,

On 2023-01-26 19:01:03 -0800, Peter Geoghegan wrote:
> On Thu, Jan 26, 2023 at 6:37 PM Andres Freund <andres@anarazel.de> wrote:
> > I also don't really see how that is responsive to anything else in my
> > email. That's just as true for the current gating condition (the issuance of
> > an FPI during heap_page_prune() / HTSV()).
> >
> > What I was wondering about is whether we should replace the
> >   fpi_before != pgWalUsage.wal_fpi
> > with
> >   records_before != pgWalUsage.wal_records && !WouldIssueFpi(page)
>
> I understand that. What I'm saying is that that's going to create a
> huge problem of its own, unless you separately account for that
> problem.

> The simplest and obvious example is something like a pgbench_tellers
> table. VACUUM will generally run fast enough relative to the workload
> that it will set some of those pages all-visible. Now it's going to
> freeze them, too. Arguably it shouldn't even be setting the pages
> all-visible, but now you make that existing problem much worse.

So the benefit of the FPI condition is that it indicates that the page hasn't
been updated all that recently, because, after all, a checkpoint has happened
since?  If that's the intention, it needs a huge honking comment - at least I
can't read that out of:

  Also freeze when pruning generated an FPI, if doing so means that we set the
  page all-frozen afterwards (might not happen until final heap pass).

It doesn't seem like a great proxy to me. ISTM that this means that how
aggressive vacuum is about opportunistically freezing pages depends on config
variables like checkpoint_timeout & max_wal_size (less common opportunistic
freezing), full_page_writes & use of unlogged tables (no opportunistic
freezing), and the largely random scheduling of autovac workers.

I can see it making a difference for pgbench_tellers, but it's a pretty small
difference in overall WAL volume. I can think of more adverse workloads though
- but even there the difference seems not huge, and not predictably
reached. Due to the freeze plan stuff you added, the amount of WAL for
freezing a page is pretty darn small compared to the amount of WAL if compared
to the amount of WAL needed to fill a page with non-frozen tuples.

That's not to say we shouldn't reduce the risk - I agree that both the "any
fpi" and the "any record" condition can have adverse effects!

However, an already dirty page getting frozen is also the one case where
freezing won't have meaningful write amplication effect. So I think it's worth
trying spending effort figuring out how we can make freezing in that situation
have unlikely and small downsides.

The cases with downsides are tables that are very heavily updated througout,
where the page is going to be defrosted again almost immediately. As you say,
the all-visible marking has a similar problem.

Essentially the "any fpi" logic is a very coarse grained way of using the page
LSN as a measurement. As I said, I don't think "has a checkpoint occurred
since the last write" is a good metric to avoid unnecessary freezing - it's
too coarse. But I think using the LSN is the right thought. What about
something like

  lsn_threshold =  insert_lsn - (insert_lsn - lsn_of_last_vacuum) * 0.1
  if (/* other conds */ && PageGetLSN(page) <= lsn_threshold)
     FreezeMe();

I probably got some details wrong, what I am going for with lsn_threshold is
that we'd freeze an already dirty page if it's not been updated within 10% of
the LSN distance to the last VACUUM.

> The important point is that there doesn't seem to be any good way
> around thinking about the table as a whole if you're going to freeze
> speculatively. This is not the same dynamic as we see with the FPI
> thing IMV -- that's not nearly so speculative as what you're talking
> about, since it is speculative in roughly the same sense that eager
> freezing was speculative (hence the suggestion that something like
> vacuum_freeze_strategy_threshold could have a roll to play).

I don't think the speculation is that fundamentally different - a heavily
updated table with a bit of a historic, non-changing portion, makes
vacuum_freeze_strategy_threshold freeze way more aggressively than either "any
record" or "any fpi".

> The FPI thing is mostly about the cost now versus the cost later on.
> You're gambling that you won't get another FPI later on if you freeze
> now. But the cost of a second FPI later on is so much higher than the
> added cost of freezing now that that's a very favorable bet, that we
> can afford to "lose" many times while still coming out ahead overall.

Agreed. And not just avoiding FPIs, avoiding another dirtying of the page! The
latter part is especially huge IMO. Depending on s_b size it can also avoid
another *read* of the page...

> And even when we lose, you generally still won't have been completely
> wrong -- even then there generally will indeed be a second FPI later
> on for the same page, to go with everything else. This makes the
> wasted freezing even less significant, on a comparative basis!

This is precisely why I think that we can afford to be quite aggressive about
freezing already dirty pages...

Greetings,

Andres Freund

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

27 January 2023, 10:11:41

On Thu, Jan 26, 2023 at 9:58 PM Andres Freund <andres@anarazel.de> wrote:
> It doesn't seem like a great proxy to me. ISTM that this means that how
> aggressive vacuum is about opportunistically freezing pages depends on config
> variables like checkpoint_timeout & max_wal_size (less common opportunistic
> freezing), full_page_writes & use of unlogged tables (no opportunistic
> freezing), and the largely random scheduling of autovac workers.

The FPI thing was originally supposed to complement the freezing
strategies stuff, and possibly other rules that live in
lazy_scan_prune. Obviously you can freeze a page by following any rule
that you care to invent -- you can decide by calling random(). Two
rules can coexist during the same VACUUM (actually, they do already).

> Essentially the "any fpi" logic is a very coarse grained way of using the page
> LSN as a measurement. As I said, I don't think "has a checkpoint occurred
> since the last write" is a good metric to avoid unnecessary freezing - it's
> too coarse. But I think using the LSN is the right thought. What about
> something like
>
>   lsn_threshold =  insert_lsn - (insert_lsn - lsn_of_last_vacuum) * 0.1
>   if (/* other conds */ && PageGetLSN(page) <= lsn_threshold)
>      FreezeMe();
>
> I probably got some details wrong, what I am going for with lsn_threshold is
> that we'd freeze an already dirty page if it's not been updated within 10% of
> the LSN distance to the last VACUUM.

It seems to me that you're reinventing something akin to eager
freezing strategy here. At least that's how I define it, since now
you're bringing the high level context into it; what happens with the
table, with VACUUM operations, and so on. Obviously this requires
tracking the metadata that you suppose will be available in some way
or other, in particular things like lsn_of_last_vacuum.

What about unlogged/temporary tables? The obvious thing to do there is
what I did in the patch that was reverted (freeze whenever the page
will thereby become all-frozen), and forget about LSNs. But you have
already objected to that part, specifically.

BTW, you still haven't changed the fact that you get rather different
behavior with checksums/wal_log_hints. I think that that's good, but
you didn't seem to.

> I don't think the speculation is that fundamentally different - a heavily
> updated table with a bit of a historic, non-changing portion, makes
> vacuum_freeze_strategy_threshold freeze way more aggressively than either "any
> record" or "any fpi".

That's true. The point I was making is that both this proposal and
eager freezing are based on some kind of high level picture of the
needs of the table, based on high level metadata. To me that's the
defining characteristic.

> > And even when we lose, you generally still won't have been completely
> > wrong -- even then there generally will indeed be a second FPI later
> > on for the same page, to go with everything else. This makes the
> > wasted freezing even less significant, on a comparative basis!
>
> This is precisely why I think that we can afford to be quite aggressive about
> freezing already dirty pages...

I'm beginning to warm to this idea, now that I understand it a little better.

-- 
Peter Geoghegan

Re: New strategies for freezing, advancing relfrozenxid early

From

Andres Freund

Date:

27 January 2023, 11:51:59

Hi,

On 2023-01-26 23:11:41 -0800, Peter Geoghegan wrote:
> > Essentially the "any fpi" logic is a very coarse grained way of using the page
> > LSN as a measurement. As I said, I don't think "has a checkpoint occurred
> > since the last write" is a good metric to avoid unnecessary freezing - it's
> > too coarse. But I think using the LSN is the right thought. What about
> > something like
> >
> >   lsn_threshold =  insert_lsn - (insert_lsn - lsn_of_last_vacuum) * 0.1
> >   if (/* other conds */ && PageGetLSN(page) <= lsn_threshold)
> >      FreezeMe();
> >
> > I probably got some details wrong, what I am going for with lsn_threshold is
> > that we'd freeze an already dirty page if it's not been updated within 10% of
> > the LSN distance to the last VACUUM.
> 
> It seems to me that you're reinventing something akin to eager
> freezing strategy here. At least that's how I define it, since now
> you're bringing the high level context into it; what happens with the
> table, with VACUUM operations, and so on. Obviously this requires
> tracking the metadata that you suppose will be available in some way
> or other, in particular things like lsn_of_last_vacuum.

I agree with bringing high-level context into the decision about whether to
freeze agressively - my problem with the eager freezing strategy patch isn't
that it did that too much, it's that it didn't do it enough.

But I also don't think what I describe above is really comparable to "table
level" eager freezing though - the potential worst case overhead is a small
fraction of the WAL volume, and there's zero increase in data write volume. I
suspect the absolute worst case of "always freeze dirty pages" is when a
single tuple on the page gets updated immediately after every time we freeze
the page - a single tuple is where the freeze record is the least space
efficient. The smallest update is about the same size as the smallest freeze
record.  For that to amount to a large WAL increase you'd a crazy rate of such
updates interspersed with vacuums. In slightly more realistic cases (i.e. not
column less tuples that constantly get updated and freezing happening all the
time) you end up with a reasonably small WAL rate overhead.

That worst case of "freeze dirty" is bad enough to spend some brain and
compute cycles to prevent. But if we don't always get it right in some
workload, it's not *awful*.

The worst case of the "eager freeze strategy" is a lot larger - it's probably
something like updating one narrow tuple every page, once per checkpoint, so
that each freeze generates an FPI. I think that results in a max overhead of
2x for data writes, and about 150x for WAL volume (ratio of one update record
with an FPI). Obviously that's a pointless workload, but I do think that
analyzing the "outer boundaries" of the regression something can cause, can be
helpful.

I think one way forward with the eager strategy approach would be to have a
very narrow gating condition for now, and then incrementally expand it in
later releases.

One use-case where the eager strategy is particularly useful is
[nearly-]append-only tables - and it's also the one workload that's reasonably
easy to detect using stats. Maybe something like
(dead_tuples_since_last_vacuum / inserts_since_last_vacuum) < 0.05
or so.

That'll definitely leave out loads of workloads where eager freezing would be
useful - but are there semi-reasonable workloads where it'll hurt badly? I
don't *think* so.

> What about unlogged/temporary tables? The obvious thing to do there is
> what I did in the patch that was reverted (freeze whenever the page
> will thereby become all-frozen), and forget about LSNs. But you have
> already objected to that part, specifically.

My main concern about that is the data write amplification it could cause when
page is clean when we start freezing.  But I can't see a large potential
downside to always freezing unlogged/temp tables when the page is already
dirty.

> BTW, you still haven't changed the fact that you get rather different
> behavior with checksums/wal_log_hints. I think that that's good, but
> you didn't seem to.

I think that, if we had something like the recency test I was talking about,
we could afford to alway freeze when the page is already dirty and not very
recently modified. I.e. not even insist on a WAL record having been generated
during pruning/HTSV.  But I need to think through the dangers of that more.

Greetings,

Andres Freund

Re: New strategies for freezing, advancing relfrozenxid early

From

Andres Freund

Date:

27 January 2023, 12:02:53

Hi,

On 2023-01-27 00:51:59 -0800, Andres Freund wrote:
> One use-case where the eager strategy is particularly useful is
> [nearly-]append-only tables - and it's also the one workload that's reasonably
> easy to detect using stats. Maybe something like
> (dead_tuples_since_last_vacuum / inserts_since_last_vacuum) < 0.05
> or so.
> 
> That'll definitely leave out loads of workloads where eager freezing would be
> useful - but are there semi-reasonable workloads where it'll hurt badly? I
> don't *think* so.

That 0.05 could be a GUC + relopt combo, which'd allow users to opt in tables
with known usage pattern into always using eager freezing.

Greetings,

Andres Freund

Re: New strategies for freezing, advancing relfrozenxid early

From

Robert Haas

Date:

27 January 2023, 17:48:43

On Thu, Jan 26, 2023 at 4:51 PM Peter Geoghegan <pg@bowt.ie> wrote:
> This is the kind of remark that makes me think that you don't get it.
>
> The most influential OLTP benchmark of all time is TPC-C, which has
> exactly this problem. In spades -- it's enormously disruptive. Which
> is one reason why I used it as a showcase for a lot of this work. Plus
> practical experience (like the Heroku database in the blog post I
> linked to) fully agrees with that benchmark, as far as this stuff goes
> -- that was also a busy OLTP database.
>
> Online transaction involves transactions. Right? There is presumably
> some kind of ledger, some kind of orders table. Naturally these have
> entries that age out fairly predictably. After a while, almost all the
> data is cold data. It is usually about that simple.
>
> One of the key strengths of systems like Postgres is the ability to
> inexpensively store a relatively large amount of data that has just
> about zero chance of being read, let alone modified. While at the same
> time having decent OLTP performance for the hot data. Not nearly as
> good as an in-memory system, mind you -- and yet in-memory systems
> remain largely a niche thing.

I think it's interesting that TPC-C suffers from the kind of problem
that your patch was intended to address. I hadn't considered that. But
I do not think it detracts from the basic point I was making, which is
that you need to think about the downsides of your patch, not just the
upsides.

If you want to argue that there is *no* OLTP workload that will be
harmed by freezing as aggressively as possible, then that would be a
good argument in favor of your patch, because it would be arguing that
the downside simply doesn't exist, at least for OLTP workloads. The
fact that you can think of *one particular* OLTP workload that can
benefit from the patch is just doubling down on the "my patch has an
upside" argument, which literally no one is disputing.

I don't think you can make such an argument stick, though. OLTP
workloads come in all shapes and sizes. It's pretty common to have
tables where the application inserts a bunch of data, updates it over
and over again like, truncates the table, and starts over. In such a
case, aggressive freezing has to be a loss, because no freezing is
ever needed. It's also surprisingly common to have tables where a
bunch of data is inserted and then, after a bit of processing, a bunch
of rows are updated exactly once, after which the data is not modified
any further. In those kinds of cases, aggressive freezing is a great
idea if it happens after that round of updates but a poor idea if it
happens before that round of updates.

It's also pretty common to have cases where portions of the table
become very hot, get a lot of updates for a while, and then that part
of the table becomes cool and some other part of the table becomes
very hot for a while. I think it's possible that aggressive freezing
might do OK in such environments, actually. It will be a negative if
we aggressively freeze the part of the table that's currently hot, but
I think typically tables that have this access pattern are quite big,
so VACUUM isn't going to sweep through the table all that often. It
will probably freeze a lot more data-that-was-hot-a-bit-ago than it
will freeze data-that-is-hot-this-very-minute. Then again, maybe that
would happen without the patch, too. Maybe this kind of case is a wash
for your patch? I don't know.

Whatever you think of these examples, I don't see how it can be right
to suppose that *in general* freezing very aggressively has no
downsides. If that were true, then we probably wouldn't have
vacuum_freeze_min_age at all. We would always just freeze everything
ASAP. I mean, you could theorize that whoever invented that GUC is an
idiot and that they had absolutely no good reason for introducing it,
but that seems pretty ridiculous. Someone put guards against
overly-aggressive freezing into the system *for a reason* and if you
just go rip them all out, you're going to reintroduce the problems
against which they were intended to guard.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

Re: New strategies for freezing, advancing relfrozenxid early

From

Robert Haas

Date:

27 January 2023, 17:53:29

On Thu, Jan 26, 2023 at 6:37 PM Peter Geoghegan <pg@bowt.ie> wrote:
> > I don't see what your reference to checkpoint timeout is about here?
> >
> > Also, as I mentioned before, the problem isn't specific to checkpoint_timeout
> > = 1min. It just makes it cheaper to reproduce.
>
> That's flagrantly intellectually dishonest.

This kind of ad hominum attack has no place on this mailing list, or
anywhere in the PostgreSQL community.

If you think there's a problem with Andres's test case, or his
analysis of it, you can talk about those problems without accusing him
of intellectual dishonesty.

I don't see anything to indicate that he was being intentionally
dishonest, either. At most he was mistaken. More than likely, not even
that.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

27 January 2023, 19:22:23

On Fri, Jan 27, 2023 at 6:48 AM Robert Haas <robertmhaas@gmail.com> wrote:
> > One of the key strengths of systems like Postgres is the ability to
> > inexpensively store a relatively large amount of data that has just
> > about zero chance of being read, let alone modified. While at the same
> > time having decent OLTP performance for the hot data. Not nearly as
> > good as an in-memory system, mind you -- and yet in-memory systems
> > remain largely a niche thing.
>
> I think it's interesting that TPC-C suffers from the kind of problem
> that your patch was intended to address. I hadn't considered that. But
> I do not think it detracts from the basic point I was making, which is
> that you need to think about the downsides of your patch, not just the
> upsides.
>
> If you want to argue that there is *no* OLTP workload that will be
> harmed by freezing as aggressively as possible, then that would be a
> good argument in favor of your patch, because it would be arguing that
> the downside simply doesn't exist, at least for OLTP workloads. The
> fact that you can think of *one particular* OLTP workload that can
> benefit from the patch is just doubling down on the "my patch has an
> upside" argument, which literally no one is disputing.

You've treated me to another multi paragraph talking down, as if I was
still clinging to my original position, which is of course not the
case. I've literally said I'm done with VACUUM for good, and that I
just want to put a line under this. Yet you still persist in doing
this sort of thing. I'm not fighting you, I'm not fighting Andres.

I was making a point about the need to do something in this area in
general. That's all.

-- 
Peter Geoghegan

Re: New strategies for freezing, advancing relfrozenxid early

From

Robert Haas

Date:

27 January 2023, 20:53:58

On Fri, Jan 27, 2023 at 12:58 AM Andres Freund <andres@anarazel.de> wrote:
> Essentially the "any fpi" logic is a very coarse grained way of using the page
> LSN as a measurement. As I said, I don't think "has a checkpoint occurred
> since the last write" is a good metric to avoid unnecessary freezing - it's
> too coarse. But I think using the LSN is the right thought. What about
> something like
>
>   lsn_threshold =  insert_lsn - (insert_lsn - lsn_of_last_vacuum) * 0.1
>   if (/* other conds */ && PageGetLSN(page) <= lsn_threshold)
>      FreezeMe();
>
> I probably got some details wrong, what I am going for with lsn_threshold is
> that we'd freeze an already dirty page if it's not been updated within 10% of
> the LSN distance to the last VACUUM.

I think this might not be quite the right idea for a couple of reasons.

First, suppose that the table is being processed just by autovacuum
(no manual VACUUM operations) and that the rate of WAL generation is
pretty even, so that LSN age is a good proxy for time. If autovacuum
processes the table once per hour, this will freeze if it hasn't been
updated in the last six minutes. That sounds good. But if autovacuum
processes the table once per day, then this will freeze if it hasn't
been updated in 2.4 hours. That might be OK, but it sounds a little on
the long side. If autovacuum processes the table once per week, then
this will freeze if it hasn't been updated in 16.8 hours. That sounds
too conservative. Conversely, if autovacuum processes the table every
3 minutes, then this will freeze the data if it hasn't been updated in
the last 18 seconds, which sounds awfully aggressive. Maybe I'm wrong
here, but I feel like the absolute amount of wall-clock time we're
talking about here probably matters to some degree. I'm not sure
whether a strict time-based threshold like, say, 10 minutes would be a
good idea, leaving aside the difficulties of implementation. It might
be right to think that if the table is being vacuumed a lot, freezing
more aggressively is smart, and if it's being vacuumed infrequently,
freezing less aggressively is smart, because if the table has enough
activity that it's being vacuumed frequently, that might also be a
sign that we need to freeze more aggressively in order to avoid having
things go sideways. However, I'm not completely sure about that, and I
think it's possible that we need some guardrails to avoid going too
far in either direction.

Second, and more seriously, I think this would, in some circumstances,
lead to tremendously unstable behavior. Suppose somebody does a bunch
of work on a table and then they're like "oh, we should clean up,
VACUUM" and it completes quickly because it's been a while since the
last vacuum and so it doesn't freeze much. Then, for whatever reason,
they decide to run it one more time, and it goes bananas and starts
freezing all kinds of stuff because the LSN distance since the last
vacuum is basically zero. Or equally, you run a manual VACUUM, and you
get completely different behavior depending on how long it's been
since the last autovacuum ran.

In some ways, I think this proposal has many of the same problems as
vacuum_freeze_min_age. In both cases, the instinct is that we should
use something on the page to let us know how long it's been since the
page was modified, and proceed on the theory that if the page has not
been modified recently, it probably isn't about to be modified again.
That's a reasonable instinct, but the rate of XID advancement and the
rate of LSN advancement are both highly variable, even on a system
that's always under some load.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

Re: New strategies for freezing, advancing relfrozenxid early

From

Andres Freund

Date:

27 January 2023, 21:36:22

Hi,

On 2023-01-27 12:53:58 -0500, Robert Haas wrote:
> On Fri, Jan 27, 2023 at 12:58 AM Andres Freund <andres@anarazel.de> wrote:
> > Essentially the "any fpi" logic is a very coarse grained way of using the page
> > LSN as a measurement. As I said, I don't think "has a checkpoint occurred
> > since the last write" is a good metric to avoid unnecessary freezing - it's
> > too coarse. But I think using the LSN is the right thought. What about
> > something like
> >
> >   lsn_threshold =  insert_lsn - (insert_lsn - lsn_of_last_vacuum) * 0.1
> >   if (/* other conds */ && PageGetLSN(page) <= lsn_threshold)
> >      FreezeMe();
> >
> > I probably got some details wrong, what I am going for with lsn_threshold is
> > that we'd freeze an already dirty page if it's not been updated within 10% of
> > the LSN distance to the last VACUUM.
>
> I think this might not be quite the right idea for a couple of reasons.

It's definitely not perfect.  If we had an approximate LSN->time map as
general infrastructure, we could do a lot better. I think it'd be reasonably
easy to maintain that in the autovacuum launcher, for example.

One thing worth calling out here, because it's not obvious from the code
quoted above in isolation, is that what I was trying to refine here was the
decision when to perform opportunistic freezing *of already dirty pages that
do not require an FPI*.

So all that we need to prevent here is freezing very hotly updated data, where
the WAL overhead of the freeze records would be noticable, because we
constantly VACUUM, due to the high turnover.

> First, suppose that the table is being processed just by autovacuum
> (no manual VACUUM operations) and that the rate of WAL generation is
> pretty even, so that LSN age is a good proxy for time. If autovacuum
> processes the table once per hour, this will freeze if it hasn't been
> updated in the last six minutes. That sounds good. But if autovacuum
> processes the table once per day, then this will freeze if it hasn't
> been updated in 2.4 hours. That might be OK, but it sounds a little on
> the long side.

You're right. I was thinking of the "lsn_since_last_vacuum" because I was
posulating it being useful elsewhere in the thread (but for eager strategy
logic) - but here that's really not very relevant.

Given that we're dealing with already dirty pages not requiring an FPI, I
think a much better "reference LSN" would be the LSN of the last checkpoint
(LSN of the last checkpoint record, not the current REDO pointer).

> Second, and more seriously, I think this would, in some circumstances,
> lead to tremendously unstable behavior. Suppose somebody does a bunch
> of work on a table and then they're like "oh, we should clean up,
> VACUUM" and it completes quickly because it's been a while since the
> last vacuum and so it doesn't freeze much. Then, for whatever reason,
> they decide to run it one more time, and it goes bananas and starts
> freezing all kinds of stuff because the LSN distance since the last
> vacuum is basically zero. Or equally, you run a manual VACUUM, and you
> get completely different behavior depending on how long it's been
> since the last autovacuum ran.

I don't think this quite applies to the scenario at hand, because it's
restricted to already dirty pages. And the max increased overhead is also
small due to that - so occasionally getting it wrong is that impactful.

Greetings,

Andres Freund

Re: New strategies for freezing, advancing relfrozenxid early

From

Peter Geoghegan

Date:

27 January 2023, 21:40:10

On Fri, Jan 27, 2023 at 12:52 AM Andres Freund <andres@anarazel.de> wrote:
> I agree with bringing high-level context into the decision about whether to
> freeze agressively - my problem with the eager freezing strategy patch isn't
> that it did that too much, it's that it didn't do it enough.
>
>
> But I also don't think what I describe above is really comparable to "table
> level" eager freezing though - the potential worst case overhead is a small
> fraction of the WAL volume, and there's zero increase in data write volume.

All I meant was that I initially thought that you were trying to
replace the FPI thing with something at the same level of ambition,
that could work in a low context way. But I now see that you're
actually talking about something quite a bit more ambitious for
Postgres 16, which is structurally similar to a freezing strategy,
from a code point of view -- it relies on high-level context for the
VACUUM/table as a whole. I wasn't equating it with the eager freezing
strategy in any other way.

It might also be true that this other thing happens to render the FPI
mechanism redundant. I'm actually not completely sure that it will
just yet. Let me verify my understanding of your proposal:

You mean that we'd take the page LSN before doing anything with the
page, right at the top of lazy_scan_prune, at the same point that
"fpi_before" is initialized currently. Then, if we subsequently
dirtied the page (as determined by its LSN, so as to focus on "dirtied
via WAL logged operation") during pruning, *and* if the "lsn_before"
of the page was from before our cutoff (derived via "  lsn_threshold =
 insert_lsn - (insert_lsn - lsn_of_last_vacuum) * 0.1" or similar),
*and* if the page is eligible to become all-frozen, then we'd freeze
the page.

That's it, right? It's about pages that *we* (VACUUM) dirtied, and
wrote records and/or FPIs for already?

> I suspect the absolute worst case of "always freeze dirty pages" is when a
> single tuple on the page gets updated immediately after every time we freeze
> the page - a single tuple is where the freeze record is the least space
> efficient. The smallest update is about the same size as the smallest freeze
> record.  For that to amount to a large WAL increase you'd a crazy rate of such
> updates interspersed with vacuums. In slightly more realistic cases (i.e. not
> column less tuples that constantly get updated and freezing happening all the
> time) you end up with a reasonably small WAL rate overhead.

Other thing is that we'd be doing this in situations where we already
know that a VISIBLE record is required, which is comparable in size to
a FREEZE_PAGE record with one tuple/plan (around 64 bytes). The
smallest WAL records are mostly just generic WAL record header
overhead.

> Obviously that's a pointless workload, but I do think that
> analyzing the "outer boundaries" of the regression something can cause, can be
> helpful.

I agree about the "outer boundaries" being a useful guide.

> I think one way forward with the eager strategy approach would be to have a
> very narrow gating condition for now, and then incrementally expand it in
> later releases.
>
> One use-case where the eager strategy is particularly useful is
> [nearly-]append-only tables - and it's also the one workload that's reasonably
> easy to detect using stats. Maybe something like
> (dead_tuples_since_last_vacuum / inserts_since_last_vacuum) < 0.05
> or so.
>
> That'll definitely leave out loads of workloads where eager freezing would be
> useful - but are there semi-reasonable workloads where it'll hurt badly? I
> don't *think* so.

I have no further plans to work on eager freezing strategy, or
anything of the sort, in light of recent developments. My goal at this
point is very unambitious: to get the basic page-level freezing work
into a form that makes sense as a standalone thing for Postgres 16. To
put things on a good footing, so that I can permanently bow out of all
work on VACUUM having left everything in good order. That's all.

Now, that might still mean that I'd facilitate future work of this
sort, by getting the right basic structure in place. But my
involvement in any work on freezing or anything of the sort ends here,
both as a patch author and a committer of anybody else's work. I'm
proud of the work I've done on VACUUM, but I'm keen to move on from
it.

> > What about unlogged/temporary tables? The obvious thing to do there is
> > what I did in the patch that was reverted (freeze whenever the page
> > will thereby become all-frozen), and forget about LSNs. But you have
> > already objected to that part, specifically.
>
> My main concern about that is the data write amplification it could cause when
> page is clean when we start freezing.  But I can't see a large potential
> downside to always freezing unlogged/temp tables when the page is already
> dirty.

But we have to dirty the page anyway, just to set PD_ALL_VISIBLE. That
was always a gating condition. Actually, that may have depended on not
having SKIP_PAGES_THRESHOLD, which the vm snapshot infrastructure
would have removed. That's not happening now, so I may need to
reassess. But even with SKIP_PAGES_THRESHOLD, it should be fine.

> > BTW, you still haven't changed the fact that you get rather different
> > behavior with checksums/wal_log_hints. I think that that's good, but
> > you didn't seem to.
>
> I think that, if we had something like the recency test I was talking about,
> we could afford to alway freeze when the page is already dirty and not very
> recently modified. I.e. not even insist on a WAL record having been generated
> during pruning/HTSV.  But I need to think through the dangers of that more.

Now I'm confused. I thought that the recency test you talked about was
purely to be used to do something a bit like the FPI thing, but using
some high level context. Now I don't know what to think.

--
Peter Geoghegan