Skip to content

Commit df61581

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 5141e47 commit df61581

File tree

9 files changed

+469
-36
lines changed

9 files changed

+469
-36
lines changed

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

Lines changed: 60 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -461,14 +461,24 @@ MarkAsPreparingGuts(GlobalTransaction gxact, TransactionId xid, const char *gid,
461461
proc->pgprocno = gxact->pgprocno;
462462
SHMQueueElemInit(&(proc->links));
463463
proc->waitStatus = STATUS_OK;
464-
/* We set up the gxact's VXID as InvalidBackendId/XID */
465-
proc->lxid = (LocalTransactionId) xid;
464+
if (LocalTransactionIdIsValid(MyProc->lxid))
465+
{
466+
/* clone VXID, for TwoPhaseGetXidByVirtualXID() to find */
467+
proc->lxid = MyProc->lxid;
468+
proc->backendId = MyBackendId;
469+
}
470+
else
471+
{
472+
Assert(AmStartupProcess() || !IsPostmasterEnvironment);
473+
/* GetLockConflicts() uses this to specify a wait on the XID */
474+
proc->lxid = xid;
475+
proc->backendId = InvalidBackendId;
476+
}
466477
pgxact->xid = xid;
467478
pgxact->xmin = InvalidTransactionId;
468479
pgxact->delayChkpt = false;
469480
pgxact->vacuumFlags = 0;
470481
proc->pid = 0;
471-
proc->backendId = InvalidBackendId;
472482
proc->databaseId = databaseid;
473483
proc->roleId = owner;
474484
proc->tempNamespaceId = InvalidOid;
@@ -843,6 +853,53 @@ TwoPhaseGetGXact(TransactionId xid)
843853
return result;
844854
}
845855

856+
/*
857+
* TwoPhaseGetXidByVirtualXID
858+
* Lookup VXID among xacts prepared since last startup.
859+
*
860+
* (This won't find recovered xacts.) If more than one matches, return any
861+
* and set "have_more" to true. To witness multiple matches, a single
862+
* BackendId must consume 2^32 LXIDs, with no intervening database restart.
863+
*/
864+
TransactionId
865+
TwoPhaseGetXidByVirtualXID(VirtualTransactionId vxid,
866+
bool *have_more)
867+
{
868+
int i;
869+
TransactionId result = InvalidTransactionId;
870+
871+
Assert(VirtualTransactionIdIsValid(vxid));
872+
LWLockAcquire(TwoPhaseStateLock, LW_SHARED);
873+
874+
for (i = 0; i < TwoPhaseState->numPrepXacts; i++)
875+
{
876+
GlobalTransaction gxact = TwoPhaseState->prepXacts[i];
877+
PGPROC *proc;
878+
VirtualTransactionId proc_vxid;
879+
880+
if (!gxact->valid)
881+
continue;
882+
proc = &ProcGlobal->allProcs[gxact->pgprocno];
883+
GET_VXID_FROM_PGPROC(proc_vxid, *proc);
884+
if (VirtualTransactionIdEquals(vxid, proc_vxid))
885+
{
886+
/* Startup process sets proc->backendId to InvalidBackendId. */
887+
Assert(!gxact->inredo);
888+
889+
if (result != InvalidTransactionId)
890+
{
891+
*have_more = true;
892+
break;
893+
}
894+
result = gxact->xid;
895+
}
896+
}
897+
898+
LWLockRelease(TwoPhaseStateLock);
899+
900+
return result;
901+
}
902+
846903
/*
847904
* TwoPhaseGetDummyProc
848905
* 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
@@ -2393,6 +2393,13 @@ PrepareTransaction(void)
23932393
/* Reset XactLastRecEnd until the next transaction writes something */
23942394
XactLastRecEnd = 0;
23952395

2396+
/*
2397+
* Transfer our locks to a dummy PGPROC. This has to be done before
2398+
* ProcArrayClearTransaction(). Otherwise, a GetLockConflicts() would
2399+
* conclude "xact already committed or aborted" for our locks.
2400+
*/
2401+
PostPrepare_Locks(xid);
2402+
23962403
/*
23972404
* Let others know about no transaction in progress by me. This has to be
23982405
* done *after* the prepared transaction has been marked valid, else
@@ -2432,7 +2439,6 @@ PrepareTransaction(void)
24322439

24332440
PostPrepare_MultiXact(xid);
24342441

2435-
PostPrepare_Locks(xid);
24362442
PostPrepare_PredicateLocks(xid);
24372443

24382444
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
@@ -2784,8 +2784,12 @@ FastPathGetRelationLockEntry(LOCALLOCK *locallock)
27842784
*
27852785
* The result array is palloc'd and is terminated with an invalid VXID.
27862786
*
2787-
* Of course, the result could be out of date by the time it's returned,
2788-
* so use of this function has to be thought about carefully.
2787+
* Of course, the result could be out of date by the time it's returned, so
2788+
* use of this function has to be thought about carefully. Similarly, a
2789+
* PGPROC with no "lxid" will be considered non-conflicting regardless of any
2790+
* lock it holds. Existing callers don't care about a locker after that
2791+
* locker's pg_xact updates complete. CommitTransaction() clears "lxid" after
2792+
* pg_xact updates and before releasing locks.
27892793
*
27902794
* Note we never include the current xact's vxid in the result array,
27912795
* since an xact never blocks itself.
@@ -4402,37 +4406,80 @@ VirtualXactLockTableCleanup(void)
44024406
}
44034407
}
44044408

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

44204478
Assert(VirtualTransactionIdIsValid(vxid));
44214479

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

44374484
SET_LOCKTAG_VIRTUALTRANSACTION(tag, vxid);
44384485

@@ -4446,7 +4493,7 @@ VirtualXactLock(VirtualTransactionId vxid, bool wait)
44464493
*/
44474494
proc = BackendIdGetProc(vxid.backendId);
44484495
if (proc == NULL)
4449-
return true;
4496+
return XactLockForVirtualXact(vxid, InvalidTransactionId, wait);
44504497

44514498
/*
44524499
* We must acquire this lock before checking the backendId and lxid
@@ -4455,12 +4502,12 @@ VirtualXactLock(VirtualTransactionId vxid, bool wait)
44554502
*/
44564503
LWLockAcquire(&proc->backendLock, LW_EXCLUSIVE);
44574504

4458-
/* If the transaction has ended, our work here is done. */
44594505
if (proc->backendId != vxid.backendId
44604506
|| proc->fpLocalTransactionId != vxid.localTransactionId)
44614507
{
4508+
/* VXID ended */
44624509
LWLockRelease(&proc->backendLock);
4463-
return true;
4510+
return XactLockForVirtualXact(vxid, InvalidTransactionId, wait);
44644511
}
44654512

44664513
/*
@@ -4507,14 +4554,24 @@ VirtualXactLock(VirtualTransactionId vxid, bool wait)
45074554
proc->fpVXIDLock = false;
45084555
}
45094556

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

45134570
/* Time to wait. */
45144571
(void) LockAcquire(&tag, ShareLock, false, false);
45154572

45164573
LockRelease(&tag, ShareLock, false);
4517-
return true;
4574+
return XactLockForVirtualXact(vxid, xid, wait);
45184575
}
45194576

45204577
/*

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

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

0 commit comments

Comments
 (0)