Re: Testing autovacuum wraparound (including failsafe) - Mailing list pgsql-hackers

On Fri, Apr 23, 2021 at 5:29 PM Andres Freund <andres@anarazel.de> wrote:
> On 2021-04-23 16:12:33 -0700, Peter Geoghegan wrote:
> > The only reason that I chose 4GB for FAILSAFE_MIN_PAGES is because the
> > related VACUUM_FSM_EVERY_PAGES constant was 8GB -- the latter limits
> > how often we'll consider the failsafe in the single-pass/no-indexes
> > case.
>
> I don't really understand why it makes sense to tie FAILSAFE_MIN_PAGES
> and VACUUM_FSM_EVERY_PAGES together? They seem pretty independent to me?

VACUUM_FSM_EVERY_PAGES controls how often VACUUM does work that
usually takes place right after the two pass case finishes a round of
index and heap vacuuming. This is work that we certainly don't want to
do every time we process a single heap page in the one-pass/no-indexes
case. Initially this just meant FSM vacuuming, but it now includes a
failsafe check.

Of course all of the precise details here are fairly arbitrary
(including VACUUM_FSM_EVERY_PAGES, which has been around for a couple
of releases now). The overall goal that I had in mind was to make the
one-pass case's use of the failsafe have analogous behavior to the
two-pass/has-indexes case -- a goal which was itself somewhat
arbitrary.

> The failsafe mode affects the table scan itself by disabling cost
> limiting. As far as I can see the ways it triggers for the table scan (vs
> truncation or index processing) are:
>
> 1) Before vacuuming starts, for heap phases and indexes, if already
>    necessary at that point
> 2) For a table with indexes, before/after each index vacuum, if now
>    necessary
> 3) On a table without indexes, every 8GB, iff there are dead tuples, if now necessary
>
> Why would we want to trigger the failsafe mode during a scan of a table
> with dead tuples and no indexes, but not on a table without dead tuples
> or with indexes but fewer than m_w_m dead tuples? That makes little
> sense to me.

What alternative does make sense to you?

It seemed important to put the failsafe check at points where we do
other analogous work in all cases. We made a pragmatic trade-off. In
theory almost any scheme might not check often enough, and/or might
check too frequently.

> It seems that for the no-index case the warning message is quite off?

I'll fix that up some point soon. FWIW this happened because the
support for one-pass VACUUM was added quite late, at Robert's request.

Another issue with the failsafe commit is that we haven't considered
the autovacuum_multixact_freeze_max_age table reloption -- we only
check the GUC. That might have accidentally been the right thing to
do, though, since the reloption is interpreted as lower than the GUC
in all cases anyway -- arguably the
autovacuum_multixact_freeze_max_age GUC should be all we care about
anyway. I will need to think about this question some more, though.

> > > For 2), I don't really have a better idea than making that configurable
> > > somehow?
> >
> > That could make sense as a developer/testing option, I suppose. I just
> > doubt that it makes sense as anything else.
>
> Yea, I only was thinking of making it configurable to be able to test
> it. If we change the limit to something considerably lower I wouldn't
> see a need for that anymore.

It would probably be okay to just lower it significantly. Not sure if
that's the best approach, though. Will pick it up next week.

-- 
Peter Geoghegan