Re: Prepared transaction releasing locks before deregistering its GID

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Cc: Oleksii Kliukin <alexk(at)hintbits(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Prepared transaction releasing locks before deregistering its GID
Date: 2019-02-20 08:22:01
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Wed, Feb 20, 2019 at 03:14:07PM +0900, Masahiko Sawada wrote:
> @@ -811,6 +811,9 @@ TwoPhaseGetGXact(TransactionId xid)
> static TransactionId cached_xid = InvalidTransactionId;
> static GlobalTransaction cached_gxact = NULL;
> + Assert(!lock_held ||
> + LWLockHeldByMeInMode(TwoPhaseStateLock, LW_EXCLUSIVE));
> +
> It seems strange to me, why do we always require an exclusive lock
> here in spite of acquiring a shared lock?

Thanks for looking at the patch. LWLockHeldByMe() is fine here. It
seems that I had a brain fade.

> TwoPhaseGetDummyBackendId() is called by
> multixact_twophase_postcommit() which is called while holding
> TwoPhaseStateLock in exclusive mode in FinishPreparedTransaction().
> Since we cache the global transaction when we call
> lock_twophase_commit() I couldn't produce issue but it seems to have a
> potential of locking problem.

You are right: this could cause a deadlock problem, but it actually
cannot be triggered now because the repetitive calls of
TwoPhaseGetGXact() make the resulting XID to be cached, so the lock
finishes by never be taken, but that could become broken in the
future. From a consistency point of view, adding the same option to
TwoPhaseGetGXact() and TwoPhaseGetGXact() to control the lock
acquisition is much better as well. Please note that the recovery
tests stress multixact post-commit callbacks already, so you can see
the caching behavior with that one:
CREATE TABLE t_009_tbl2 (id int, msg text);
INSERT INTO t_009_tbl2 VALUES (27, 'issued to stuff');
COMMIT PREPARED 'xact_009_13';

The assertion really needs to be before the cached XID is checked

Attached is an updated patch. Thanks for the feedback.

Attachment Content-Type Size
2pc-commit-locks-v2.patch text/x-diff 7.1 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2019-02-20 08:34:15 Re: BUG #15572: Misleading message reported by "Drop function operation" on DB with functions having same name
Previous Message Amit Langote 2019-02-20 07:37:48 Re: Another way to fix inherited UPDATE/DELETE