From: Andres Freund <andres@anarazel.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: REINDEX INDEX results in a crash for an index of pg_class since 9.6
Date: Tue, 30 Apr 2019 11:27:41 -0700
Message-ID: <20190430182741.s6o7mo5ebmgxxi5x@alap3.anarazel.de> (raw)
In-Reply-To: <29546.1556647550@sss.pgh.pa.us>
References: <12189.1556575627@sss.pgh.pa.us>
<20190429220924.lfptluzkujb26y2n@alap3.anarazel.de>
<20190430030342.jmbgnih4q3cdoufz@alap3.anarazel.de>
<26051.1556599053@sss.pgh.pa.us>
<BC406D10-146C-4495-AC5E-492CADBFA144@anarazel.de>
<26528.1556599820@sss.pgh.pa.us>
<20190430070552.jzqgcy4ihalx7nur@alap3.anarazel.de>
<30579.1556639470@sss.pgh.pa.us>
<20190430173457.pg6i6urvevse5jer@alap3.anarazel.de>
<29546.1556647550@sss.pgh.pa.us>
Hi,
On 2019-04-30 14:05:50 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > It's the lock-upgrade problem I theorized about
> > upthread. ReindexIndex(), via RangeVarCallbackForReindexIndex(), takes a
> > ShareLock on pg_class, and then goes on to upgrade to RowExclusiveLock
> > in RelationSetNewRelfilenode(). But at that time another session
> > obviously can already have the ShareLock and would also want to upgrade.
>
> Hmm. Note that this is totally independent of the deadlock mechanism
> I reported in my last message on this thread.
Yea :(
> > I'm not sure it's worth fixing this.
>
> I am not sure it's even *possible* to fix all these cases.
I think it's worth fixing the most common ones though. It sure sucks
that a plain REINDEX TABLE pg_class; isn't safe to run.
> Even if we could, it's out of scope for v12 let alone the back branches.
Unfortunately agreed. It's possible we could come up with a fix to
backpatch after maturing some, but certainly not before the release.
> I think the only practical solution is to remove those reindex tests.
> Even if we ran them in a script with no concurrent scripts, there'd
> be risk of failures against autovacuum, I'm afraid. Not often, but
> often enough to be annoying.
> Possibly we could run them in a TAP test that configures a cluster
> with autovac disabled?
Hm. Would it be sufficient to instead move them to a non-concurrent
test group, and stick a BEGIN; LOCK pg_class, ....; COMMIT; around it? I
think that ought to make it safe against autovacuum, and theoretically
there shouldn't be any overlapping pg_class/index updates that we'd need
to wait for?
This is a pretty finnicky area of the code, with obviously not enough
test coverage. I'm inclined to remove them from the back branches, and
try to get them working in master?
Greetings,
Andres Freund
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: andres@anarazel.de, tgl@sss.pgh.pa.us, michael@paquier.xyz, pgsql-hackers@lists.postgresql.org
Subject: Re: REINDEX INDEX results in a crash for an index of pg_class since 9.6
In-Reply-To: <20190430182741.s6o7mo5ebmgxxi5x@alap3.anarazel.de>
* 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