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