Re: [PATCH] Disable bgworkers during servers start in pg_upgrade - Mailing list pgsql-hackers

From Bruce Momjian
Subject Re: [PATCH] Disable bgworkers during servers start in pg_upgrade
Date
Msg-id 20210826130945.GB22637@momjian.us
Whole thread Raw
In response to Re: [PATCH] Disable bgworkers during servers start in pg_upgrade  (Julien Rouhaud <rjuju123@gmail.com>)
Responses Re: [PATCH] Disable bgworkers during servers start in pg_upgrade  (Daniel Gustafsson <daniel@yesql.se>)
List pgsql-hackers
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.laxalde@dalibo.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@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

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




pgsql-hackers by date:

Previous
From: Masahiko Sawada
Date:
Subject: Re: Skipping logical replication transactions on subscriber side
Next
From: Daniel Gustafsson
Date:
Subject: Re: [PATCH] Disable bgworkers during servers start in pg_upgrade