Skip to content

Commit a286b6f

Browse files
simonat2ndQuadrantsimonat2ndQuadrant
committed
Resolve timing issue with logging locks for Hot Standby.
We log AccessExclusiveLocks for replay onto standby nodes, but because of timing issues on ProcArray it is possible to log a lock that is still held by a just committed transaction that is very soon to be removed. To avoid any timing issue we avoid applying locks made by transactions with InvalidXid. Simon Riggs, bug report Tom Lane, diagnosis Pavan Deolasee
1 parent 8bff640 commit a286b6f

File tree

4 files changed

+88
-44
lines changed

4 files changed

+88
-44
lines changed

‎src/backend/storage/ipc/procarray.c

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,7 @@ ProcArrayApplyRecoveryInfo(RunningTransactions running)
466466
* Remove stale transactions, if any.
467467
*/
468468
ExpireOldKnownAssignedTransactionIds(running->oldestRunningXid);
469-
StandbyReleaseOldLocks(running->oldestRunningXid);
469+
StandbyReleaseOldLocks(running->xcnt, running->xids);
470470

471471
/*
472472
* If our snapshot is already valid, nothing else to do...
@@ -520,12 +520,6 @@ ProcArrayApplyRecoveryInfo(RunningTransactions running)
520520
* OK, we need to initialise from the RunningTransactionsData record
521521
*/
522522

523-
/*
524-
* Release any locks belonging to old transactions that are not
525-
* running according to the running-xacts record.
526-
*/
527-
StandbyReleaseOldLocks(running->nextXid);
528-
529523
/*
530524
* Nobody else is running yet, but take locks anyhow
531525
*/

‎src/backend/storage/ipc/standby.c

Lines changed: 75 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -528,7 +528,9 @@ StandbyAcquireAccessExclusiveLock(TransactionId xid, Oid dbOid, Oid relOid)
528528
LOCKTAG locktag;
529529

530530
/* Already processed? */
531-
if (TransactionIdDidCommit(xid) || TransactionIdDidAbort(xid))
531+
if (!TransactionIdIsValid(xid) ||
532+
TransactionIdDidCommit(xid) ||
533+
TransactionIdDidAbort(xid))
532534
return;
533535

534536
elog(trace_recovery(DEBUG4),
@@ -610,34 +612,86 @@ StandbyReleaseLockTree(TransactionId xid, int nsubxids, TransactionId *subxids)
610612
}
611613

612614
/*
613-
* StandbyReleaseLocksMany
614-
* Release standby locks held by XIDs < removeXid
615-
*
616-
* If keepPreparedXacts is true, keep prepared transactions even if
617-
* they're older than removeXid
615+
* Called at end of recovery and when we see a shutdown checkpoint.
618616
*/
619-
static void
620-
StandbyReleaseLocksMany(TransactionId removeXid, bool keepPreparedXacts)
617+
void
618+
StandbyReleaseAllLocks(void)
619+
{
620+
ListCell *cell,
621+
*prev,
622+
*next;
623+
LOCKTAG locktag;
624+
625+
elog(trace_recovery(DEBUG2), "release all standby locks");
626+
627+
prev = NULL;
628+
for (cell = list_head(RecoveryLockList); cell; cell = next)
629+
{
630+
xl_standby_lock *lock = (xl_standby_lock *) lfirst(cell);
631+
632+
next = lnext(cell);
633+
634+
elog(trace_recovery(DEBUG4),
635+
"releasing recovery lock: xid %u db %u rel %u",
636+
lock->xid, lock->dbOid, lock->relOid);
637+
SET_LOCKTAG_RELATION(locktag, lock->dbOid, lock->relOid);
638+
if (!LockRelease(&locktag, AccessExclusiveLock, true))
639+
elog(LOG,
640+
"RecoveryLockList contains entry for lock no longer recorded by lock manager: xid %u database %u relation %u",
641+
lock->xid, lock->dbOid, lock->relOid);
642+
RecoveryLockList = list_delete_cell(RecoveryLockList, cell, prev);
643+
pfree(lock);
644+
}
645+
}
646+
647+
/*
648+
* StandbyReleaseOldLocks
649+
* Release standby locks held by XIDs that aren't running, as long
650+
* as they're not prepared transactions.
651+
*/
652+
void
653+
StandbyReleaseOldLocks(int nxids, TransactionId *xids)
621654
{
622655
ListCell *cell,
623656
*prev,
624657
*next;
625658
LOCKTAG locktag;
626659

627-
/*
628-
* Release all matching locks.
629-
*/
630660
prev = NULL;
631661
for (cell = list_head(RecoveryLockList); cell; cell = next)
632662
{
633663
xl_standby_lock *lock = (xl_standby_lock *) lfirst(cell);
664+
bool remove = false;
634665

635666
next = lnext(cell);
636667

637-
if (!TransactionIdIsValid(removeXid) || TransactionIdPrecedes(lock->xid, removeXid))
668+
Assert(TransactionIdIsValid(lock->xid));
669+
670+
if (StandbyTransactionIdIsPrepared(lock->xid))
671+
remove = false;
672+
else
673+
{
674+
int i;
675+
bool found = false;
676+
677+
for (i = 0; i < nxids; i++)
678+
{
679+
if (lock->xid == xids[i])
680+
{
681+
found = true;
682+
break;
683+
}
684+
}
685+
686+
/*
687+
* If its not a running transaction, remove it.
688+
*/
689+
if (!found)
690+
remove = true;
691+
}
692+
693+
if (remove)
638694
{
639-
if (keepPreparedXacts && StandbyTransactionIdIsPrepared(lock->xid))
640-
continue;
641695
elog(trace_recovery(DEBUG4),
642696
"releasing recovery lock: xid %u db %u rel %u",
643697
lock->xid, lock->dbOid, lock->relOid);
@@ -654,27 +708,6 @@ StandbyReleaseLocksMany(TransactionId removeXid, bool keepPreparedXacts)
654708
}
655709
}
656710

657-
/*
658-
* Called at end of recovery and when we see a shutdown checkpoint.
659-
*/
660-
void
661-
StandbyReleaseAllLocks(void)
662-
{
663-
elog(trace_recovery(DEBUG2), "release all standby locks");
664-
StandbyReleaseLocksMany(InvalidTransactionId, false);
665-
}
666-
667-
/*
668-
* StandbyReleaseOldLocks
669-
* Release standby locks held by XIDs < removeXid, as long
670-
* as they're not prepared transactions.
671-
*/
672-
void
673-
StandbyReleaseOldLocks(TransactionId removeXid)
674-
{
675-
StandbyReleaseLocksMany(removeXid, true);
676-
}
677-
678711
/*
679712
* --------------------------------------------------------------------
680713
* Recovery handling for Rmgr RM_STANDBY_ID
@@ -816,6 +849,13 @@ standby_desc(StringInfo buf, uint8 xl_info, char *rec)
816849
* Later, when we apply the running xact data we must be careful to ignore
817850
* transactions already committed, since those commits raced ahead when
818851
* making WAL entries.
852+
*
853+
* The loose timing also means that locks may be recorded that have a
854+
* zero xid, since xids are removed from procs before locks are removed.
855+
* So we must prune the lock list down to ensure we hold locks only for
856+
* currently running xids, performed by StandbyReleaseOldLocks().
857+
* Zero xids should no longer be possible, but we may be replaying WAL
858+
* from a time when they were possible.
819859
*/
820860
void
821861
LogStandbySnapshot(TransactionId *nextXid)

‎src/backend/storage/lmgr/lock.c

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2317,8 +2317,18 @@ GetRunningTransactionLocks(int *nlocks)
23172317
{
23182318
PGPROC *proc = proclock->tag.myProc;
23192319
LOCK *lock = proclock->tag.myLock;
2320+
TransactionId xid = proc->xid;
23202321

2321-
accessExclusiveLocks[index].xid = proc->xid;
2322+
/*
2323+
* Don't record locks for transactions if we know they have already
2324+
* issued their WAL record for commit but not yet released lock.
2325+
* It is still possible that we see locks held by already complete
2326+
* transactions, if they haven't yet zeroed their xids.
2327+
*/
2328+
if (!TransactionIdIsValid(xid))
2329+
continue;
2330+
2331+
accessExclusiveLocks[index].xid = xid;
23222332
accessExclusiveLocks[index].dbOid = lock->tag.locktag_field1;
23232333
accessExclusiveLocks[index].relOid = lock->tag.locktag_field2;
23242334

‎src/include/storage/standby.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ extern void StandbyAcquireAccessExclusiveLock(TransactionId xid, Oid dbOid, Oid
4848
extern void StandbyReleaseLockTree(TransactionId xid,
4949
int nsubxids, TransactionId *subxids);
5050
extern void StandbyReleaseAllLocks(void);
51-
extern void StandbyReleaseOldLocks(TransactionId removeXid);
51+
extern void StandbyReleaseOldLocks(int nxids, TransactionId *xids);
5252

5353
/*
5454
* XLOG message types

0 commit comments

Comments
 (0)