Skip to content

Commit 5184932

Browse files
nmischnmisch
committed
Fix CREATE INDEX CONCURRENTLY for the newest prepared transactions.
The purpose of commit 8a54e12 was to fix this, and it sufficed when the PREPARE TRANSACTION completed before the CIC looked for lock conflicts. Otherwise, things still broke. As before, in a cluster having used CIC while having enabled prepared transactions, queries that use the resulting index can silently fail to find rows. It may be necessary to reindex to recover from past occurrences; REINDEX CONCURRENTLY suffices. Fix this for future index builds by making CIC wait for arbitrarily-recent prepared transactions and for ordinary transactions that may yet PREPARE TRANSACTION. As part of that, have PREPARE TRANSACTION transfer locks to its dummy PGPROC before it calls ProcArrayClearTransaction(). Back-patch to 9.6 (all supported versions). Andrey Borodin, reviewed (in earlier versions) by Andres Freund. Discussion: https://postgr.es/m/01824242-AA92-4FE9-9BA7-AEBAFFEA3D0C@yandex-team.ru
1 parent e428699 commit 5184932

File tree

9 files changed

+466
-36
lines changed

9 files changed

+466
-36
lines changed

‎src/backend/access/transam/twophase.c

Lines changed: 57 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -410,14 +410,24 @@ MarkAsPreparing(TransactionId xid, const char *gid,
410410
proc->pgprocno = gxact->pgprocno;
411411
SHMQueueElemInit(&(proc->links));
412412
proc->waitStatus = STATUS_OK;
413-
/* We set up the gxact's VXID as InvalidBackendId/XID */
414-
proc->lxid = (LocalTransactionId) xid;
413+
if (LocalTransactionIdIsValid(MyProc->lxid))
414+
{
415+
/* clone VXID, for TwoPhaseGetXidByVirtualXID() to find */
416+
proc->lxid = MyProc->lxid;
417+
proc->backendId = MyBackendId;
418+
}
419+
else
420+
{
421+
Assert(AmStartupProcess() || !IsPostmasterEnvironment);
422+
/* GetLockConflicts() uses this to specify a wait on the XID */
423+
proc->lxid = xid;
424+
proc->backendId = InvalidBackendId;
425+
}
415426
pgxact->xid = xid;
416427
pgxact->xmin = InvalidTransactionId;
417428
pgxact->delayChkpt = false;
418429
pgxact->vacuumFlags = 0;
419430
proc->pid = 0;
420-
proc->backendId = InvalidBackendId;
421431
proc->databaseId = databaseid;
422432
proc->roleId = owner;
423433
proc->isBackgroundWorker = false;
@@ -801,6 +811,50 @@ TwoPhaseGetGXact(TransactionId xid)
801811
return result;
802812
}
803813

814+
/*
815+
* TwoPhaseGetXidByVirtualXID
816+
* Lookup VXID among xacts prepared since last startup.
817+
*
818+
* (This won't find recovered xacts.) If more than one matches, return any
819+
* and set "have_more" to true. To witness multiple matches, a single
820+
* BackendId must consume 2^32 LXIDs, with no intervening database restart.
821+
*/
822+
TransactionId
823+
TwoPhaseGetXidByVirtualXID(VirtualTransactionId vxid,
824+
bool *have_more)
825+
{
826+
int i;
827+
TransactionId result = InvalidTransactionId;
828+
829+
Assert(VirtualTransactionIdIsValid(vxid));
830+
LWLockAcquire(TwoPhaseStateLock, LW_SHARED);
831+
832+
for (i = 0; i < TwoPhaseState->numPrepXacts; i++)
833+
{
834+
GlobalTransaction gxact = TwoPhaseState->prepXacts[i];
835+
PGPROC *proc;
836+
VirtualTransactionId proc_vxid;
837+
838+
if (!gxact->valid)
839+
continue;
840+
proc = &ProcGlobal->allProcs[gxact->pgprocno];
841+
GET_VXID_FROM_PGPROC(proc_vxid, *proc);
842+
if (VirtualTransactionIdEquals(vxid, proc_vxid))
843+
{
844+
if (result != InvalidTransactionId)
845+
{
846+
*have_more = true;
847+
break;
848+
}
849+
result = ProcGlobal->allPgXact[gxact->pgprocno].xid;
850+
}
851+
}
852+
853+
LWLockRelease(TwoPhaseStateLock);
854+
855+
return result;
856+
}
857+
804858
/*
805859
* TwoPhaseGetDummyProc
806860
* Get the dummy backend ID for prepared transaction specified by XID

‎src/backend/access/transam/xact.c

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2368,6 +2368,13 @@ PrepareTransaction(void)
23682368
/* Reset XactLastRecEnd until the next transaction writes something */
23692369
XactLastRecEnd = 0;
23702370

2371+
/*
2372+
* Transfer our locks to a dummy PGPROC. This has to be done before
2373+
* ProcArrayClearTransaction(). Otherwise, a GetLockConflicts() would
2374+
* conclude "xact already committed or aborted" for our locks.
2375+
*/
2376+
PostPrepare_Locks(xid);
2377+
23712378
/*
23722379
* Let others know about no transaction in progress by me. This has to be
23732380
* done *after* the prepared transaction has been marked valid, else
@@ -2407,7 +2414,6 @@ PrepareTransaction(void)
24072414

24082415
PostPrepare_MultiXact(xid);
24092416

2410-
PostPrepare_Locks(xid);
24112417
PostPrepare_PredicateLocks(xid);
24122418

24132419
ResourceOwnerRelease(TopTransactionResourceOwner,

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -822,9 +822,10 @@ XactLockTableWaitErrorCb(void *arg)
822822
* To do this, obtain the current list of lockers, and wait on their VXIDs
823823
* until they are finished.
824824
*
825-
* Note we don't try to acquire the locks on the given locktags, only the VXIDs
826-
* of its lock holders; if somebody grabs a conflicting lock on the objects
827-
* after we obtained our initial list of lockers, we will not wait for them.
825+
* Note we don't try to acquire the locks on the given locktags, only the
826+
* VXIDs and XIDs of their lock holders; if somebody grabs a conflicting lock
827+
* on the objects after we obtained our initial list of lockers, we will not
828+
* wait for them.
828829
*/
829830
void
830831
WaitForLockersMultiple(List *locktags, LOCKMODE lockmode)

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

Lines changed: 81 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2787,8 +2787,12 @@ FastPathGetRelationLockEntry(LOCALLOCK *locallock)
27872787
*
27882788
* The result array is palloc'd and is terminated with an invalid VXID.
27892789
*
2790-
* Of course, the result could be out of date by the time it's returned,
2791-
* so use of this function has to be thought about carefully.
2790+
* Of course, the result could be out of date by the time it's returned, so
2791+
* use of this function has to be thought about carefully. Similarly, a
2792+
* PGPROC with no "lxid" will be considered non-conflicting regardless of any
2793+
* lock it holds. Existing callers don't care about a locker after that
2794+
* locker's pg_xact updates complete. CommitTransaction() clears "lxid" after
2795+
* pg_xact updates and before releasing locks.
27922796
*
27932797
* Note we never include the current xact's vxid in the result array,
27942798
* since an xact never blocks itself.
@@ -4405,37 +4409,80 @@ VirtualXactLockTableCleanup(void)
44054409
}
44064410
}
44074411

4412+
/*
4413+
* XactLockForVirtualXact
4414+
*
4415+
* If TransactionIdIsValid(xid), this is essentially XactLockTableWait(xid,
4416+
* NULL, NULL, XLTW_None) or ConditionalXactLockTableWait(xid). Unlike those
4417+
* functions, it assumes "xid" is never a subtransaction and that "xid" is
4418+
* prepared, committed, or aborted.
4419+
*
4420+
* If !TransactionIdIsValid(xid), this locks every prepared XID having been
4421+
* known as "vxid" before its PREPARE TRANSACTION.
4422+
*/
4423+
static bool
4424+
XactLockForVirtualXact(VirtualTransactionId vxid,
4425+
TransactionId xid, bool wait)
4426+
{
4427+
bool more = false;
4428+
4429+
/* There is no point to wait for 2PCs if you have no 2PCs. */
4430+
if (max_prepared_xacts == 0)
4431+
return true;
4432+
4433+
do
4434+
{
4435+
LockAcquireResult lar;
4436+
LOCKTAG tag;
4437+
4438+
/* Clear state from previous iterations. */
4439+
if (more)
4440+
{
4441+
xid = InvalidTransactionId;
4442+
more = false;
4443+
}
4444+
4445+
/* If we have no xid, try to find one. */
4446+
if (!TransactionIdIsValid(xid))
4447+
xid = TwoPhaseGetXidByVirtualXID(vxid, &more);
4448+
if (!TransactionIdIsValid(xid))
4449+
{
4450+
Assert(!more);
4451+
return true;
4452+
}
4453+
4454+
/* Check or wait for XID completion. */
4455+
SET_LOCKTAG_TRANSACTION(tag, xid);
4456+
lar = LockAcquire(&tag, ShareLock, false, !wait);
4457+
if (lar == LOCKACQUIRE_NOT_AVAIL)
4458+
return false;
4459+
LockRelease(&tag, ShareLock, false);
4460+
} while (more);
4461+
4462+
return true;
4463+
}
4464+
44084465
/*
44094466
* VirtualXactLock
44104467
*
4411-
* If wait = true, wait until the given VXID has been released, and then
4412-
* return true.
4468+
* If wait = true, wait as long as the given VXID or any XID acquired by the
4469+
* same transaction is still running. Then, return true.
44134470
*
4414-
* If wait = false, just check whether the VXID is still running, and return
4415-
* true or false.
4471+
* If wait = false, just check whether that VXID or one of those XIDs is still
4472+
* running, and return true or false.
44164473
*/
44174474
bool
44184475
VirtualXactLock(VirtualTransactionId vxid, bool wait)
44194476
{
44204477
LOCKTAG tag;
44214478
PGPROC *proc;
4479+
TransactionId xid = InvalidTransactionId;
44224480

44234481
Assert(VirtualTransactionIdIsValid(vxid));
44244482

4425-
if (VirtualTransactionIdIsPreparedXact(vxid))
4426-
{
4427-
LockAcquireResult lar;
4428-
4429-
/*
4430-
* Prepared transactions don't hold vxid locks. The
4431-
* LocalTransactionId is always a normal, locked XID.
4432-
*/
4433-
SET_LOCKTAG_TRANSACTION(tag, vxid.localTransactionId);
4434-
lar = LockAcquire(&tag, ShareLock, false, !wait);
4435-
if (lar != LOCKACQUIRE_NOT_AVAIL)
4436-
LockRelease(&tag, ShareLock, false);
4437-
return lar != LOCKACQUIRE_NOT_AVAIL;
4438-
}
4483+
if (VirtualTransactionIdIsRecoveredPreparedXact(vxid))
4484+
/* no vxid lock; localTransactionId is a normal, locked XID */
4485+
return XactLockForVirtualXact(vxid, vxid.localTransactionId, wait);
44394486

44404487
SET_LOCKTAG_VIRTUALTRANSACTION(tag, vxid);
44414488

@@ -4449,7 +4496,7 @@ VirtualXactLock(VirtualTransactionId vxid, bool wait)
44494496
*/
44504497
proc = BackendIdGetProc(vxid.backendId);
44514498
if (proc == NULL)
4452-
return true;
4499+
return XactLockForVirtualXact(vxid, InvalidTransactionId, wait);
44534500

44544501
/*
44554502
* We must acquire this lock before checking the backendId and lxid
@@ -4458,12 +4505,12 @@ VirtualXactLock(VirtualTransactionId vxid, bool wait)
44584505
*/
44594506
LWLockAcquire(&proc->backendLock, LW_EXCLUSIVE);
44604507

4461-
/* If the transaction has ended, our work here is done. */
44624508
if (proc->backendId != vxid.backendId
44634509
|| proc->fpLocalTransactionId != vxid.localTransactionId)
44644510
{
4511+
/* VXID ended */
44654512
LWLockRelease(&proc->backendLock);
4466-
return true;
4513+
return XactLockForVirtualXact(vxid, InvalidTransactionId, wait);
44674514
}
44684515

44694516
/*
@@ -4510,14 +4557,24 @@ VirtualXactLock(VirtualTransactionId vxid, bool wait)
45104557
proc->fpVXIDLock = false;
45114558
}
45124559

4560+
/*
4561+
* If the proc has an XID now, we'll avoid a TwoPhaseGetXidByVirtualXID()
4562+
* search. The proc might have assigned this XID but not yet locked it,
4563+
* in which case the proc will lock this XID before releasing the VXID.
4564+
* The backendLock critical section excludes VirtualXactLockTableCleanup(),
4565+
* so we won't save an XID of a different VXID. It doesn't matter whether
4566+
* we save this before or after setting up the primary lock table entry.
4567+
*/
4568+
xid = ProcGlobal->allPgXact[proc->pgprocno].xid;
4569+
45134570
/* Done with proc->fpLockBits */
45144571
LWLockRelease(&proc->backendLock);
45154572

45164573
/* Time to wait. */
45174574
(void) LockAcquire(&tag, ShareLock, false, false);
45184575

45194576
LockRelease(&tag, ShareLock, false);
4520-
return true;
4577+
return XactLockForVirtualXact(vxid, xid, wait);
45214578
}
45224579

45234580
/*

‎src/backend/utils/cache/inval.c

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,20 @@
6464
* (XXX is it worth testing likewise for duplicate catcache flush entries?
6565
* Probably not.)
6666
*
67+
* Many subsystems own higher-level caches that depend on relcache and/or
68+
* catcache, and they register callbacks here to invalidate their caches.
69+
* While building a higher-level cache entry, a backend may receive a
70+
* callback for the being-built entry or one of its dependencies. This
71+
* implies the new higher-level entry would be born stale, and it might
72+
* remain stale for the life of the backend. Many caches do not prevent
73+
* that. They rely on DDL for can't-miss catalog changes taking
74+
* AccessExclusiveLock on suitable objects. (For a change made with less
75+
* locking, backends might never read the change.) The relation cache,
76+
* however, needs to reflect changes from CREATE INDEX CONCURRENTLY no later
77+
* than the beginning of the next transaction. Hence, when a relevant
78+
* invalidation callback arrives during a build, relcache.c reattempts that
79+
* build. Caches with similar needs could do likewise.
80+
*
6781
* If a relcache flush is issued for a system relation that we preload
6882
* from the relcache init file, we must also delete the init file so that
6983
* it will be rebuilt during the next backend restart. The actual work of

0 commit comments

Comments
 (0)