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

Hi John,

> Thanks for picking up this badly-needed topic again!

Many thanks for the review!

> 0001:
>
> +    In this condition the system can still execute read-only transactions.
> +    The active transactions will continue to execute and will be able to
> +    commit.
>
> This is ambiguous. I'd first say that any transactions already started can continue, and then say that only new read-only transactions can be started.

Fixed.

> 0004:
>
> -HINT:  Stop the postmaster and vacuum that database in single-user mode.
> +HINT:  VACUUM or VACUUM FREEZE that database.
>
> VACUUM FREEZE is worse and should not be mentioned, since it does unnecessary work. Emergency vacuum is not school -- you don't get extra credit for doing unnecessary work.

Fixed.

> Also, we may consider adding a boxed NOTE warning specifically against single-user mode, especially if this recommendation will change in at least some minor releases so people may not hear about it. See also [1].

Done.

> - * If we're past xidStopLimit, refuse to execute transactions, unless
> - * we are running in single-user mode (which gives an escape hatch
> - * to the DBA who somehow got past the earlier defenses).
> + * If we're past xidStopLimit, refuse to allocate new XIDs.
>
> This patch doesn't completely get rid of the need for single-user mode, so it should keep all information about it. If a DBA wanted to e.g. drop or truncate a table to save vacuum time, it is still possible to do it in single-user mode, so the escape hatch is still useful.

Fixed.

> In swapping this topic back in my head, I also saw [2] where Robert suggested
>
> "that old prepared transactions and stale replication
> slots should be emphasized more prominently.  Maybe something like:
>
> HINT:  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."

It looks like the hint regarding replication slots was added at some
point. Currently we have:

```
errhint( [...]
    "You might also need to commit or roll back old prepared
transactions, or drop stale replication slots.")));
```

So I choose to keep it as is for now. Please let me know if you think
we should also add a suggestion to kill long-running sessions, etc.

-- 
Best regards,
Aleksander Alekseev