Hi, Andres Freund a écrit : > On 2021-01-21 16:23:58 +0100, Denis Laxalde wrote: > > We found an issue in pg_upgrade on a cluster with a third-party > > background worker. The upgrade goes fine, but the new cluster is then in > > an inconsistent state. The background worker comes from the PoWA > > extension but the issue does not appear to related to this particular > > code. > > Well, it does imply that that backgrounder did something, as the pure > existence of a bgworker shouldn't affect > > anything. Presumably the issue is that the bgworker actually does > transactional writes, which causes problems because the xids / > multixacts from early during pg_upgrade won't actually be valid after we > do pg_resetxlog etc. > > > > As a solution, it seems that, for similar reasons that we restrict > > socket access to prevent accidental connections (from commit > > f763b77193), we should also prevent background workers to start at this > > step. > > I think that'd have quite the potential for negative impact - imagine > extensions that refuse to be loaded outside of shared_preload_libraries > (e.g. because they need to allocate shared memory) but that are required > during the course of pg_upgrade (e.g. because it's a tableam, a PL or > such). Those libraries will then tried to be loaded during the upgrade > (due to the _PG_init() hook being called when functions from the > extension are needed, e.g. the tableam or PL handler). > > Nor is it clear to me that the only way this would be problematic is > with shared_preload_libraries. A library in local_preload_libraries, or > a demand loaded library can trigger bgworkers (or database writes in > some other form) as well.
Thank you for those insights! > I wonder if we could > > a) set default_transaction_read_only to true, and explicitly change it > in the sessions that need that. > b) when in binary upgrade mode / -b, error out on all wal writes in > sessions that don't explicitly set a session-level GUC to allow > writes. Solution "a" appears to be enough to solve the problem described in my initial email. See attached patch implementing this. Cheers, Denis
>From d8b74f9220775736917b7fc08bbe397d7e2eedcd Mon Sep 17 00:00:00 2001 From: Denis Laxalde <denis.laxa...@dalibo.com> Date: Wed, 20 Jan 2021 17:25:58 +0100 Subject: [PATCH] Set default transactions to read-only at servers start in pg_upgrade In essence, this is for a similar reason that we use a restricted socket access from f763b77193b04eba03a1f4ce46df34dc0348419e because background workers may produce undesired activities during the upgrade. Author: Denis Laxalde <denis.laxa...@dalibo.com> Co-authored-by: Jehan-Guillaume de Rorthais <j...@dalibo.com> --- src/bin/pg_upgrade/server.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bin/pg_upgrade/server.c b/src/bin/pg_upgrade/server.c index 31b1425202..b17f348a5b 100644 --- a/src/bin/pg_upgrade/server.c +++ b/src/bin/pg_upgrade/server.c @@ -244,7 +244,7 @@ start_postmaster(ClusterInfo *cluster, bool report_and_exit_on_error) * vacuumdb --freeze actually freezes the tuples. */ snprintf(cmd, sizeof(cmd), - "\"%s/pg_ctl\" -w -l \"%s\" -D \"%s\" -o \"-p %d%s%s %s%s\" start", + "\"%s/pg_ctl\" -w -l \"%s\" -D \"%s\" -o \"-p %d%s%s %s%s -c default_transaction_read_only=on\" start", cluster->bindir, SERVER_LOG_FILE, cluster->pgconfig, cluster->port, (cluster->controldata.cat_ver >= BINARY_UPGRADE_SERVER_FLAG_CAT_VER) ? " -b" : -- 2.20.1
