Remove AELs from subxids correctly on standby

Issues relate only to subtransactions that hold AccessExclusiveLocks
when replayed on standby.

Prior to PG10, aborting subtransactions that held an
AccessExclusiveLock failed to release the lock until top level commit or
abort. 49bff5300d527 fixed that.

However, 49bff5300d527 also introduced a similar bug where subtransaction
commit would fail to release an AccessExclusiveLock, leaving the lock to
be removed sometimes early and sometimes late. This commit fixes
that bug also. Backpatch to PG10 needed.

Tested by observation. Note need for multi-node isolationtester to improve
test coverage for this and other HS cases.

Reported-by: Simon Riggs
Author: Simon Riggs

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index b51eeb087e8f404853a19b502e0ad06ff182172f..8e6aef332cb09afb51c70f871c00ee5e7baee4dd 100644 (file)
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -5602,12 +5602,10 @@ xact_redo_commit(xl_xact_parsed_commit *parsed,
                /*
                 * Release locks, if any. We do this for both two phase and normal one
                 * phase transactions. In effect we are ignoring the prepare phase and
-                * just going straight to lock release. At commit we release all locks
-                * via their top-level xid only, so no need to provide subxact list,
-                * which will save time when replaying commits.
+                * just going straight to lock release.
                 */
                if (parsed->xinfo & XACT_XINFO_HAS_AE_LOCKS)
-                       StandbyReleaseLockTree(xid, 0, NULL);
+                       StandbyReleaseLockTree(xid, parsed->nsubxacts, parsed->subxacts);
        }
 
        if (parsed->xinfo & XACT_XINFO_HAS_ORIGIN)

diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index 518c6d74b292d55e11cadf1a860380b9cd24b178..7f293d989b5917ad2bce93fde2c904a4507f3aa0 100644 (file)
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -684,11 +684,8 @@ ProcArrayApplyRecoveryInfo(RunningTransactions running)
 
        /*
         * Remove stale locks, if any.
-        *
-        * Locks are always assigned to the toplevel xid so we don't need to care
-        * about subxcnt/subxids (and by extension not about ->suboverflowed).
         */
-       StandbyReleaseOldLocks(running->xcnt, running->xids);
+       StandbyReleaseOldLocks(running->oldestRunningXid);
 
        /*
         * If our snapshot is already valid, nothing else to do...

diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c
index 38d6db1e12619354b355ff50b22b8ca2c19b7d05..1eacc5ec14375655601521e23d339f8f1c4d1d2d 100644 (file)
--- a/src/backend/storage/ipc/standby.c
+++ b/src/backend/storage/ipc/standby.c
@@ -722,7 +722,7 @@ StandbyReleaseAllLocks(void)
  *             as long as they're not prepared transactions.
  */
 void
-StandbyReleaseOldLocks(int nxids, TransactionId *xids)
+StandbyReleaseOldLocks(TransactionId oldxid)
 {
        ListCell   *cell,
                           *prev,
@@ -741,26 +741,8 @@ StandbyReleaseOldLocks(int nxids, TransactionId *xids)
 
                if (StandbyTransactionIdIsPrepared(lock->xid))
                        remove = false;
-               else
-               {
-                       int                     i;
-                       bool            found = false;
-
-                       for (i = 0; i < nxids; i++)
-                       {
-                               if (lock->xid == xids[i])
-                               {
-                                       found = true;
-                                       break;
-                               }
-                       }
-
-                       /*
-                        * If its not a running transaction, remove it.
-                        */
-                       if (!found)
-                               remove = true;
-               }
+               else if (TransactionIdPrecedes(lock->xid, oldxid))
+                       remove = true;
 
                if (remove)
                {

diff --git a/src/include/storage/standby.h b/src/include/storage/standby.h
index 28bf8f2f3987e24b4155c127082a3c6111737632..1fcd8cf1b59a2b02188fc733407327bfdf59f6d0 100644 (file)
--- a/src/include/storage/standby.h
+++ b/src/include/storage/standby.h
@@ -50,7 +50,7 @@ extern void StandbyAcquireAccessExclusiveLock(TransactionId xid, Oid dbOid, Oid
 extern void StandbyReleaseLockTree(TransactionId xid,
                                           int nsubxids, TransactionId *subxids);
 extern void StandbyReleaseAllLocks(void);
-extern void StandbyReleaseOldLocks(int nxids, TransactionId *xids);
+extern void StandbyReleaseOldLocks(TransactionId oldxid);
 
 #define MinSizeOfXactRunningXacts offsetof(xl_running_xacts, xids)