Re: do only critical work during single-user vacuum?

On Thu, Jan 27, 2022 at 8:28 PM Justin Pryzby <pryzby@telsasoft.com> wrote:

> I'm sure you meant "&" here (fixed in attached patch to appease the cfbot):
> +               if (options | VACOPT_MINIMAL)

Thanks for catching that! That copy-pasto was also masking my failure
to process the option properly -- fixed in the attached as v5.

> It should either refuse to run if a list of tables is specified with MINIMAL,
> or it should filter that list by XID condition.

I went with the former for simplicity. As a single-purpose option, it
makes sense.

> As for the name, it could be MINIMAL or FAILSAFE or EMERGENCY or ??
> I think the name should actually be a bit more descriptive, and maybe say XID,
> like MINIMAL_XID or XID_EMERGENCY...

I went with EMERGENCY in this version to reinforce its purpose in the
mind of the user (and reader of this code).

> Normally, options are independent, but VACUUM (MINIMAL) is a "shortcut" to a
> hardcoded set of options: freeze on, truncate off, cleanup off.  So it refuses
> to be combined with other options - good.
>
> This is effectively a shortcut to hypothetical parameters for selecting tables
> by XID/MXID age.  In the future, someone could debate adding user-facing knobs
> for table selection by age.

I used the params struct in v5 for the emergency cutoff ages. Even
with the values hard-coded, it seems cleaner to keep them here.

> I still wonder if the relations should be processed in order of decreasing age.
> An admin might have increased autovacuum_freeze_max_age up to 2e9, and your
> query might return thousands of tables, with a wide range of sizes and ages.
>
> Processing them in order of decreasing age would allow the admin to quickly
> vacuum the oldest tables, and optionally interrupt vacuum to get out of single
> user mode ASAP - even if their just want to run VACUUM(MINIMAL) in a normal
> backend when services aren't offline.  Processing them out of order might be
> pretty surprising - they might run vacuum for an hour (or overnight), cancel
> it, attempt to start the DB in normal mode, and conclude that it made no
> visible progress.

While that seems like a nice property to have, it does complicate
things, so can be left for follow-on work.

Also in v5:

- It mentions the new command in the error hint in
GetNewTransactionId(). I'm not sure if multi-word commands should be
quoted like this.
- A first draft of documentation

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