Re: [PATCH] Clarify the behavior of the system when approaching XID wraparound

On Mon, Apr 3, 2023 at 7:33 PM Aleksander Alekseev <aleksander(at)timescale(dot)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