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

On Thu, Aug 26, 2021 at 03:24:33PM +0800, Julien Rouhaud wrote:
> On Thu, Aug 26, 2021 at 3:15 PM Denis Laxalde <denis(dot)laxalde(at)dalibo(dot)com> wrote:
> >
> > Michael Paquier a écrit :
> > >> @@ -5862,6 +5862,9 @@ do_start_bgworker(RegisteredBgWorker *rw)
> > >> static bool
> > >> bgworker_should_start_now(BgWorkerStartTime start_time)
> > >> {
> > >> + if (IsBinaryUpgrade)
> > >> + return false;
> > >> +
> > > Using -c max_worker_processes=0 would just have the same effect, no?
> > > So we could just patch pg_upgrade's server.c to get the same level of
> > > protection?
> >
> > Yes, same effect indeed. This would log "too many background workers"
> > messages in pg_upgrade logs, though.
> > See attached patch implementing this suggestion.
>
> I disagree. It can appear to have the same effect but it's not
> guaranteed. Any module in shared_preload_libraries could stick a
> "max_worker_processes +=X" if it thinks it should account for its own
> ressources. That may not be something encouraged, but it's definitely
> possible (and I think Andres recently mentioned that some extensions
> do things like that, although maybe for other GUCs) and could result
> in a corruption of a pg_upgrade'd cluster, so I still think that
> changing bgworker_should_start_now() is a better option.

I am not sure. We have a lot of pg_upgrade code that turns off things
like autovacuum and that has worked fine:

snprintf(cmd, sizeof(cmd),
"\"%s/pg_ctl\" -w -l \"%s\" -D \"%s\" -o \"-p %d%s%s %s%s\" start",
cluster->bindir, SERVER_LOG_FILE, cluster->pgconfig, cluster->port,
(cluster->controldata.cat_ver >=
BINARY_UPGRADE_SERVER_FLAG_CAT_VER) ? " -b" :
" -c autovacuum=off -c autovacuum_freeze_max_age=2000000000",
(cluster == &new_cluster) ?
" -c synchronous_commit=off -c fsync=off -c full_page_writes=off -c vacuum_defer_cleanup_age=0" : "",
cluster->pgopts ? cluster->pgopts : "", socket_string);

Basically, pg_upgrade has avoided any backend changes that could be
controlled by GUCs and I am not sure we want to start adding such
changes for just this.

--
Bruce Momjian <bruce(at)momjian(dot)us> https://momjian.us
EDB https://enterprisedb.com

If only the physical world exists, free will is an illusion.