Re: [PATCH] Clarify the behavior of the system when approaching XID wraparound - Mailing list pgsql-hackers

From John Naylor
Subject Re: [PATCH] Clarify the behavior of the system when approaching XID wraparound
Date
Msg-id CAFBsxsHsA_TKKimrbH+LgESC6Gq=U5e6e6Hi1_aMEQn25MVqmw@mail.gmail.com
Whole thread Raw
In response to Re: [PATCH] Clarify the behavior of the system when approaching XID wraparound  (Aleksander Alekseev <aleksander@timescale.com>)
Responses Re: [PATCH] Clarify the behavior of the system when approaching XID wraparound  (Pavel Borisov <pashkin.elfe@gmail.com>)
List pgsql-hackers
On Mon, Apr 3, 2023 at 7:33 PM Aleksander Alekseev <aleksander@timescale.com> wrote:

> > Yes, the exact same text as it appeared in the [2] thread above, which prompted Robert's comment I quoted. I take the point to mean: All of these things need to be taken care of *first*, before vacuuming, so the hint should order things so that it is clear.
> >
> > > Please let me know if you think
> > > we should also add a suggestion to kill long-running sessions, etc.
> >
> > +1 for also adding that.
>
> OK, done. I included this change as a separate patch. It can be
> squashed with another one if necessary.

Okay, great. For v4-0003:

Each hint mentions vacuum twice, because it's adding the vacuum message at the end, but not removing it from the beginning. The vacuum string should be on its own line, since we will have to modify that for back branches (skip indexes and truncation).

I'm also having second thoughts about "Ensure that autovacuum is progressing" in the hint. That might be better in the docs, if we decide to go ahead with adding a specific checklist there.

In vacuum.c:

 errhint("Close open transactions soon to avoid wraparound problems.\n"
- "You might also need to commit or roll back old prepared transactions, or drop stale replication slots.")));
+ "You might also need to commit or roll back old prepared transactions, drop stale replication slots, or kill long-running sessions. Ensure that autovacuum is progressing, or run a manual database-wide VACUUM.")));

This appears in vacuum_get_cutoffs(), which is called by vacuum and cluster, and the open transactions were already mentioned, so this is not the place for this change.

> This particular wording was chosen for consistency with multixact.c:
>
> ```
> errmsg("database is not accepting commands that generate new
> MultiXactIds to avoid wraparound data loss in database \"%s\"",
> ```

Okay, I didn't look into that -- seems like a good precedent.

v4-0002:

- errhint("Stop the postmaster and vacuum that database in single-user mode.\n"
+ errhint("VACUUM that database.\n"

Further in the spirit of consistency, the mulixact path already has "Execute a database-wide VACUUM in that database.\n", and that seems like better wording.

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

pgsql-hackers by date:

Previous
From: Masahiko Sawada
Date:
Subject: Re: Minimal logical decoding on standbys
Next
From: Masahiko Sawada
Date:
Subject: Re: Should vacuum process config file reload more often