Re: Prepared transaction releasing locks before deregistering its GID

On 19.02.2019 7:44, Michael Paquier wrote:
> On Tue, Feb 19, 2019 at 01:07:06AM -0300, Alvaro Herrera wrote:
>> On 2019-Feb-19, Michael Paquier wrote:
>>> extern GlobalTransaction MarkAsPreparing(TransactionId xid, const char *gid,
>> Hmm, ABI break ...
> Well, sure. I always post patches for HEAD first. And I was actually
> wondering if that's worth back-patching per the odds of facing the
> error and seeing how old it is.
> --
> Michael

May be I missed something, but why it is not possible just to move
removing 2PC GXact before releasing transaction locks:

diff --git a/src/backend/access/transam/twophase.c
b/src/backend/access/transam/twophase.c
index 9a8a6bb..574d28b 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1560,17 +1560,6 @@ FinishPreparedTransaction(const char *gid, bool
isCommit)
        if (hdr->initfileinval)
                RelationCacheInitFilePostInvalidate();

-       /* And now do the callbacks */
-       if (isCommit)
-               ProcessRecords(bufptr, xid, twophase_postcommit_callbacks);
-       else
-               ProcessRecords(bufptr, xid, twophase_postabort_callbacks);
-
-       PredicateLockTwoPhaseFinish(xid, isCommit);
-
-       /* Count the prepared xact as committed or aborted */
-       AtEOXact_PgStat(isCommit);
-
        /*
         * And now we can clean up any files we may have left.
         */
@@ -1582,6 +1571,17 @@ FinishPreparedTransaction(const char *gid, bool
isCommit)
        LWLockRelease(TwoPhaseStateLock);
        MyLockedGxact = NULL;

+       /* And now do the callbacks */
+       if (isCommit)
+               ProcessRecords(bufptr, xid, twophase_postcommit_callbacks);
+       else
+               ProcessRecords(bufptr, xid, twophase_postabort_callbacks);
+
+       PredicateLockTwoPhaseFinish(xid, isCommit);
+
+       /* Count the prepared xact as committed or aborted */
+       AtEOXact_PgStat(isCommit);
+
        RESUME_INTERRUPTS();

        pfree(buf);

--
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company