Re: WaitForOlderSnapshots in DETACH PARTITION causes deadlocks

From: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: "Imseih (AWS), Sami" <simseih(at)amazon(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: WaitForOlderSnapshots in DETACH PARTITION causes deadlocks
Date: 2023-07-26 13:57:03
Message-ID: 20230726135703.f2hxz3asiwxhi4af@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2023-Jul-25, Michael Paquier wrote:

> On Mon, Jul 24, 2023 at 07:40:04PM +0000, Imseih (AWS), Sami wrote:
> > WaitForOlderSnapshots is used here to ensure that snapshots older than
> > the start of the ALTER TABLE DETACH CONCURRENTLY are completely removed
> > to guarantee consistency, however it does seem to cause deadlocks for at
> > least RR/SERIALIZABLE transactions.
> >
> > There are cases [2] in which certain operations are accepted as not being
> > MVCC-safe, and now I am wondering if this is another case? Deadlocks are
> > not a good scenario, and WaitForOlderSnapshot does not appear to do
> > anything for READ COMMITTED transactions.
> >
> > So do we actually need WaitForOlderSnapshot in the FINALIZE code? and
> > Could be acceptable that this operation is marked as not MVCC-safe like
> > the other aforementioned operations?
>
> I guess that there is an argument with lifting that a bit. Based on
> the test case your are providing, a deadlock occuring between the
> FINALIZE and a scan of the top-level partitioned table in a
> transaction that began before the DETACH CONCURRENTLY does not seem
> like the best answer to have from the user perspective. I got to
> wonder whether there is room to make the wait for older snapshots in
> the finalize phase more robust, though, and actually make it wait
> until the first transaction commits rather than fail because of a
> deadlock like that.

It took a lot of work to get SERIALIZABLE/RR transactions to work with
DETACHED CONCURRENTLY, so I'm certainly not in a hurry to give up and
just "document that it doesn't work". I don't think that would be an
acceptable feature regression.

I spent some time yesterday trying to turn the reproducer into an
isolationtester spec file. I have not succeeded yet, but I'll continue
with that this afternoon.

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"Every machine is a smoke machine if you operate it wrong enough."
https://twitter.com/libseybieda/status/1541673325781196801

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Aleksander Alekseev 2023-07-26 13:59:31 Re: [PATCH] Check more invariants during syscache initialization
Previous Message Aleksander Alekseev 2023-07-26 13:52:14 Re: Remove unused fields in ReorderBufferTupleBuf