From: Denis Laxalde <denis.laxalde@dalibo.com>
To: Andres Freund <andres@anarazel.de>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: [PATCH] Disable bgworkers during servers start in pg_upgrade
Date: Wed, 27 Jan 2021 11:25:11 +0100
Message-ID: <20210127102511.tvhu2wmrxoz3tle4@dalibo.com> (raw)
In-Reply-To: <20210124003611.lkyxhd2mpdmv5xzh@alap3.anarazel.de>
References: <20210121152357.s6eflhqyh4g5e6dv@dalibo.com>
<20210124003611.lkyxhd2mpdmv5xzh@alap3.anarazel.de>
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
Attachments:0001-Set-default-transactions-to-read-only-at-servers-sta.patch (text/x-diff, 1.3K) download
From d8b74f9220775736917b7fc08bbe397d7e2eedcd Mon Sep 17 00:00:00 2001
From: Denis Laxalde <denis.laxalde@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.laxalde@dalibo.com>
Co-authored-by: Jehan-Guillaume de Rorthais <jgdr@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.cindex 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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: denis.laxalde@dalibo.com, andres@anarazel.de, pgsql-hackers@lists.postgresql.org
Subject: Re: [PATCH] Disable bgworkers during servers start in pg_upgrade
In-Reply-To: <20210127102511.tvhu2wmrxoz3tle4@dalibo.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox