Re: [PATCH] Disable bgworkers during servers start in pg_upgrade

Hi,

On Fri, Jan 28, 2022 at 11:02:46AM +0100, Denis Laxalde wrote:
>
> I tried that simple change first:
>
> diff --git a/src/backend/access/transam/xlog.c
> b/src/backend/access/transam/xlog.c
> index dfe2a0bcce..8feab0cb96 100644
> --- a/src/backend/access/transam/xlog.c
> +++ b/src/backend/access/transam/xlog.c
> @@ -8498,6 +8498,9 @@ HotStandbyActiveInReplay(void)
> bool
> XLogInsertAllowed(void)
> {
> + if (IsBinaryUpgrade)
> + return false;
> +
>
>
> But then, pg_upgrade's tests (make -C src/bin/pg_upgrade/ check) fail at
> vaccumdb but not during pg_dumpall:
>
> [...]
>
> command: "/home/denis/src/pgsql/build/tmp_install/home/denis/.local/pgsql/bin/vacuumdb"
> --host /home/denis/src/pgsql/build/src/bin/pg_upgrade --port 51696
> --username denis --all --analyze >> "pg_upgrade_utility.log" 2>&1
> vacuumdb: vacuuming database "postgres"
> vacuumdb: error: processing of database "postgres" failed: PANIC: cannot
> make new WAL entries during recovery
>
> In contrast with pg_dump/pg_dumpall, vacuumdb has no --binary-upgrade flag,
> so it does not seem possible to use a special GUC setting to allow WAL
> writes in that vacuumdb session at the moment.
> Should we add --binary-upgrade to vacuumdb as well? Or am I going in the
> wrong direction?

I think having a new option for vacuumdb is the right move.

It seems unlikely that any cron or similar on the host will try to run some
concurrent vacuumdb, but we still have to enforce that only the one executed by
pg_upgrade can succeed.

I guess it could be an undocumented option, similar to postgres' -b, which
would only be allowed iff --all and --freeze is also passed to be extra safe.

While at it:

> vacuumdb: error: processing of database "postgres" failed: PANIC: cannot
> make new WAL entries during recovery

Should we tweak that message when IsBinaryUpgrade is true?