Re: Decoupling antiwraparound autovacuum from special rules around auto cancellation

Hi,

On 2023-01-18 17:54:27 -0800, Peter Geoghegan wrote:
> From 0afaf310255a068d3c1ca9d2ce6f00118cbff890 Mon Sep 17 00:00:00 2001
> From: Peter Geoghegan <pg(at)bowt(dot)ie>
> Date: Fri, 25 Nov 2022 11:23:20 -0800
> Subject: [PATCH v5 1/2] Add autovacuum trigger instrumentation.
>
> Add new instrumentation that lists a triggering condition in the server
> log whenever an autovacuum is logged. This reports "table age" as the
> triggering criteria when antiwraparound autovacuum runs (the XID age
> trigger case and the MXID age trigger case are represented separately).
> Other cases are reported as autovacuum trigger when the tuple insert
> thresholds or the dead tuple thresholds were crossed.
>
> Author: Peter Geoghegan <pg(at)bowt(dot)ie>
> Reviewed-By: Andres Freund <andres(at)anarazel(dot)de>
> Reviewed-By: Jeff Davis <pgsql(at)j-davis(dot)com>
> Discussion: https://postgr.es/m/CAH2-Wz=S-R_2rO49Hm94Nuvhu9_twRGbTm6uwDRmRu-Sqn_t3w@mail.gmail.com
> ---
> src/include/commands/vacuum.h | 19 +++-
> src/backend/access/heap/vacuumlazy.c | 5 ++
> src/backend/commands/vacuum.c | 31 ++++++-
> src/backend/postmaster/autovacuum.c | 124 ++++++++++++++++++---------
> 4 files changed, 137 insertions(+), 42 deletions(-)
>
> diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
> index 689dbb770..13f70a1f6 100644
> --- a/src/include/commands/vacuum.h
> +++ b/src/include/commands/vacuum.h
> @@ -191,6 +191,21 @@ typedef struct VacAttrStats
> #define VACOPT_SKIP_DATABASE_STATS 0x100 /* skip vac_update_datfrozenxid() */
> #define VACOPT_ONLY_DATABASE_STATS 0x200 /* only vac_update_datfrozenxid() */
>
> +/*
> + * Values used by autovacuum.c to tell vacuumlazy.c about the specific
> + * threshold type that triggered an autovacuum worker.
> + *
> + * AUTOVACUUM_NONE is used when VACUUM isn't running in an autovacuum worker.
> + */
> +typedef enum AutoVacType
> +{
> + AUTOVACUUM_NONE = 0,
> + AUTOVACUUM_TABLE_XID_AGE,
> + AUTOVACUUM_TABLE_MXID_AGE,
> + AUTOVACUUM_DEAD_TUPLES,
> + AUTOVACUUM_INSERTED_TUPLES,
> +} AutoVacType;

Why is there TABLE_ in AUTOVACUUM_TABLE_XID_AGE but not
AUTOVACUUM_DEAD_TUPLES? Both are on tables.

What do you think about naming this VacuumTriggerType and adding an
VAC_TRIG_MANUAL or such?

> /*
> * Values used by index_cleanup and truncate params.
> *
> @@ -222,7 +237,8 @@ typedef struct VacuumParams
> * use default */
> int multixact_freeze_table_age; /* multixact age at which to scan
> * whole table */
> - bool is_wraparound; /* force a for-wraparound vacuum */
> + bool is_wraparound; /* antiwraparound autovacuum? */
> + AutoVacType trigger; /* autovacuum trigger condition, if any */

The comment change for is_wraparound seems a bit pointless, but whatever.

> @@ -2978,7 +2995,10 @@ relation_needs_vacanalyze(Oid relid,
> bool *doanalyze,
> bool *wraparound)
> {

The changes here are still bigger than I'd like, but ...

> - bool force_vacuum;
> + TransactionId relfrozenxid = classForm->relfrozenxid;
> + MultiXactId relminmxid = classForm->relminmxid;
> + AutoVacType trigger = AUTOVACUUM_NONE;
> + bool tableagevac;

Here + below we end up with three booleans that just represent the choices in
our fancy new enum. That seems awkward to me.

> @@ -3169,14 +3212,15 @@ autovacuum_do_vac_analyze(autovac_table *tab, BufferAccessStrategy bstrategy)
> static void
> autovac_report_activity(autovac_table *tab)
> {
> -#define MAX_AUTOVAC_ACTIV_LEN (NAMEDATALEN * 2 + 56)
> +#define MAX_AUTOVAC_ACTIV_LEN (NAMEDATALEN * 2 + 100)
> char activity[MAX_AUTOVAC_ACTIV_LEN];
> int len;
>
> /* Report the command and possible options */
> if (tab->at_params.options & VACOPT_VACUUM)
> snprintf(activity, MAX_AUTOVAC_ACTIV_LEN,
> - "autovacuum: VACUUM%s",
> + "autovacuum for %s: VACUUM%s",
> + vac_autovacuum_trigger_msg(tab->at_params.trigger),
> tab->at_params.options & VACOPT_ANALYZE ? " ANALYZE" : "");
> else
> snprintf(activity, MAX_AUTOVAC_ACTIV_LEN,

Somehow the added "for ..." sounds a bit awkward. "autovacuum for table XID
age". Maybe "autovacuum due to ..."?

Greetings,

Andres Freund