mirror of
https://git.postgresql.org/git/postgresql.git
synced 2024-08-31 17:37:24 +02:00
Fix CREATE INDEX CONCURRENTLY for simultaneous prepared transactions.
In a cluster having used CREATE INDEX CONCURRENTLY while having enabled prepared transactions, queries that use the resulting index can silently fail to find rows. Fix this for future CREATE INDEX CONCURRENTLY by making it wait for prepared transactions like it waits for ordinary transactions. This expands the VirtualTransactionId structure domain to admit prepared transactions. It may be necessary to reindex to recover from past occurrences. Back-patch to 9.5 (all supported versions). Andrey Borodin, reviewed (in earlier versions) by Tom Lane and Michael Paquier. Discussion: https://postgr.es/m/2E712143-97F7-4890-B470-4A35142ABC82@yandex-team.ru
This commit is contained in:
parent
289ef386df
commit
d1ab4bf6ed
@ -846,8 +846,7 @@ WaitForLockersMultiple(List *locktags, LOCKMODE lockmode)
|
|||||||
|
|
||||||
/*
|
/*
|
||||||
* Note: GetLockConflicts() never reports our own xid, hence we need not
|
* Note: GetLockConflicts() never reports our own xid, hence we need not
|
||||||
* check for that. Also, prepared xacts are not reported, which is fine
|
* check for that. Also, prepared xacts are reported and awaited.
|
||||||
* since they certainly aren't going to do anything anymore.
|
|
||||||
*/
|
*/
|
||||||
|
|
||||||
/* Finally wait for each such transaction to complete */
|
/* Finally wait for each such transaction to complete */
|
||||||
|
@ -2788,9 +2788,7 @@ FastPathGetRelationLockEntry(LOCALLOCK *locallock)
|
|||||||
* so use of this function has to be thought about carefully.
|
* so use of this function has to be thought about carefully.
|
||||||
*
|
*
|
||||||
* Note we never include the current xact's vxid in the result array,
|
* Note we never include the current xact's vxid in the result array,
|
||||||
* since an xact never blocks itself. Also, prepared transactions are
|
* since an xact never blocks itself.
|
||||||
* ignored, which is a bit more debatable but is appropriate for current
|
|
||||||
* uses of the result.
|
|
||||||
*/
|
*/
|
||||||
VirtualTransactionId *
|
VirtualTransactionId *
|
||||||
GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode)
|
GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode)
|
||||||
@ -2815,19 +2813,21 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode)
|
|||||||
|
|
||||||
/*
|
/*
|
||||||
* Allocate memory to store results, and fill with InvalidVXID. We only
|
* Allocate memory to store results, and fill with InvalidVXID. We only
|
||||||
* need enough space for MaxBackends + a terminator, since prepared xacts
|
* need enough space for MaxBackends + max_prepared_xacts + a terminator.
|
||||||
* don't count. InHotStandby allocate once in TopMemoryContext.
|
* InHotStandby allocate once in TopMemoryContext.
|
||||||
*/
|
*/
|
||||||
if (InHotStandby)
|
if (InHotStandby)
|
||||||
{
|
{
|
||||||
if (vxids == NULL)
|
if (vxids == NULL)
|
||||||
vxids = (VirtualTransactionId *)
|
vxids = (VirtualTransactionId *)
|
||||||
MemoryContextAlloc(TopMemoryContext,
|
MemoryContextAlloc(TopMemoryContext,
|
||||||
sizeof(VirtualTransactionId) * (MaxBackends + 1));
|
sizeof(VirtualTransactionId) *
|
||||||
|
(MaxBackends + max_prepared_xacts + 1));
|
||||||
}
|
}
|
||||||
else
|
else
|
||||||
vxids = (VirtualTransactionId *)
|
vxids = (VirtualTransactionId *)
|
||||||
palloc0(sizeof(VirtualTransactionId) * (MaxBackends + 1));
|
palloc0(sizeof(VirtualTransactionId) *
|
||||||
|
(MaxBackends + max_prepared_xacts + 1));
|
||||||
|
|
||||||
/* Compute hash code and partition lock, and look up conflicting modes. */
|
/* Compute hash code and partition lock, and look up conflicting modes. */
|
||||||
hashcode = LockTagHashCode(locktag);
|
hashcode = LockTagHashCode(locktag);
|
||||||
@ -2902,13 +2902,9 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode)
|
|||||||
/* Conflict! */
|
/* Conflict! */
|
||||||
GET_VXID_FROM_PGPROC(vxid, *proc);
|
GET_VXID_FROM_PGPROC(vxid, *proc);
|
||||||
|
|
||||||
/*
|
|
||||||
* If we see an invalid VXID, then either the xact has already
|
|
||||||
* committed (or aborted), or it's a prepared xact. In either
|
|
||||||
* case we may ignore it.
|
|
||||||
*/
|
|
||||||
if (VirtualTransactionIdIsValid(vxid))
|
if (VirtualTransactionIdIsValid(vxid))
|
||||||
vxids[count++] = vxid;
|
vxids[count++] = vxid;
|
||||||
|
/* else, xact already committed or aborted */
|
||||||
|
|
||||||
/* No need to examine remaining slots. */
|
/* No need to examine remaining slots. */
|
||||||
break;
|
break;
|
||||||
@ -2965,11 +2961,6 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode)
|
|||||||
|
|
||||||
GET_VXID_FROM_PGPROC(vxid, *proc);
|
GET_VXID_FROM_PGPROC(vxid, *proc);
|
||||||
|
|
||||||
/*
|
|
||||||
* If we see an invalid VXID, then either the xact has already
|
|
||||||
* committed (or aborted), or it's a prepared xact. In either
|
|
||||||
* case we may ignore it.
|
|
||||||
*/
|
|
||||||
if (VirtualTransactionIdIsValid(vxid))
|
if (VirtualTransactionIdIsValid(vxid))
|
||||||
{
|
{
|
||||||
int i;
|
int i;
|
||||||
@ -2981,6 +2972,7 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode)
|
|||||||
if (i >= fast_count)
|
if (i >= fast_count)
|
||||||
vxids[count++] = vxid;
|
vxids[count++] = vxid;
|
||||||
}
|
}
|
||||||
|
/* else, xact already committed or aborted */
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -2990,7 +2982,7 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode)
|
|||||||
|
|
||||||
LWLockRelease(partitionLock);
|
LWLockRelease(partitionLock);
|
||||||
|
|
||||||
if (count > MaxBackends) /* should never happen */
|
if (count > MaxBackends + max_prepared_xacts) /* should never happen */
|
||||||
elog(PANIC, "too many conflicting locks found");
|
elog(PANIC, "too many conflicting locks found");
|
||||||
|
|
||||||
vxids[count].backendId = InvalidBackendId;
|
vxids[count].backendId = InvalidBackendId;
|
||||||
@ -4346,6 +4338,21 @@ VirtualXactLock(VirtualTransactionId vxid, bool wait)
|
|||||||
|
|
||||||
Assert(VirtualTransactionIdIsValid(vxid));
|
Assert(VirtualTransactionIdIsValid(vxid));
|
||||||
|
|
||||||
|
if (VirtualTransactionIdIsPreparedXact(vxid))
|
||||||
|
{
|
||||||
|
LockAcquireResult lar;
|
||||||
|
|
||||||
|
/*
|
||||||
|
* Prepared transactions don't hold vxid locks. The
|
||||||
|
* LocalTransactionId is always a normal, locked XID.
|
||||||
|
*/
|
||||||
|
SET_LOCKTAG_TRANSACTION(tag, vxid.localTransactionId);
|
||||||
|
lar = LockAcquire(&tag, ShareLock, false, !wait);
|
||||||
|
if (lar != LOCKACQUIRE_NOT_AVAIL)
|
||||||
|
LockRelease(&tag, ShareLock, false);
|
||||||
|
return lar != LOCKACQUIRE_NOT_AVAIL;
|
||||||
|
}
|
||||||
|
|
||||||
SET_LOCKTAG_VIRTUALTRANSACTION(tag, vxid);
|
SET_LOCKTAG_VIRTUALTRANSACTION(tag, vxid);
|
||||||
|
|
||||||
/*
|
/*
|
||||||
|
@ -47,10 +47,10 @@ extern bool Debug_deadlocks;
|
|||||||
|
|
||||||
/*
|
/*
|
||||||
* Top-level transactions are identified by VirtualTransactionIDs comprising
|
* Top-level transactions are identified by VirtualTransactionIDs comprising
|
||||||
* the BackendId of the backend running the xact, plus a locally-assigned
|
* PGPROC fields backendId and lxid. For prepared transactions, the
|
||||||
* LocalTransactionId. These are guaranteed unique over the short term,
|
* LocalTransactionId is an ordinary XID. These are guaranteed unique over
|
||||||
* but will be reused after a database restart; hence they should never
|
* the short term, but will be reused after a database restart or XID
|
||||||
* be stored on disk.
|
* wraparound; hence they should never be stored on disk.
|
||||||
*
|
*
|
||||||
* Note that struct VirtualTransactionId can not be assumed to be atomically
|
* Note that struct VirtualTransactionId can not be assumed to be atomically
|
||||||
* assignable as a whole. However, type LocalTransactionId is assumed to
|
* assignable as a whole. However, type LocalTransactionId is assumed to
|
||||||
@ -62,15 +62,16 @@ extern bool Debug_deadlocks;
|
|||||||
*/
|
*/
|
||||||
typedef struct
|
typedef struct
|
||||||
{
|
{
|
||||||
BackendId backendId; /* determined at backend startup */
|
BackendId backendId; /* backendId from PGPROC */
|
||||||
LocalTransactionId localTransactionId; /* backend-local transaction id */
|
LocalTransactionId localTransactionId; /* lxid from PGPROC */
|
||||||
} VirtualTransactionId;
|
} VirtualTransactionId;
|
||||||
|
|
||||||
#define InvalidLocalTransactionId 0
|
#define InvalidLocalTransactionId 0
|
||||||
#define LocalTransactionIdIsValid(lxid) ((lxid) != InvalidLocalTransactionId)
|
#define LocalTransactionIdIsValid(lxid) ((lxid) != InvalidLocalTransactionId)
|
||||||
#define VirtualTransactionIdIsValid(vxid) \
|
#define VirtualTransactionIdIsValid(vxid) \
|
||||||
(((vxid).backendId != InvalidBackendId) && \
|
(LocalTransactionIdIsValid((vxid).localTransactionId))
|
||||||
LocalTransactionIdIsValid((vxid).localTransactionId))
|
#define VirtualTransactionIdIsPreparedXact(vxid) \
|
||||||
|
((vxid).backendId == InvalidBackendId)
|
||||||
#define VirtualTransactionIdEquals(vxid1, vxid2) \
|
#define VirtualTransactionIdEquals(vxid1, vxid2) \
|
||||||
((vxid1).backendId == (vxid2).backendId && \
|
((vxid1).backendId == (vxid2).backendId && \
|
||||||
(vxid1).localTransactionId == (vxid2).localTransactionId)
|
(vxid1).localTransactionId == (vxid2).localTransactionId)
|
||||||
|
@ -55,12 +55,11 @@ installcheck: all
|
|||||||
check: all
|
check: all
|
||||||
$(pg_isolation_regress_check) --schedule=$(srcdir)/isolation_schedule
|
$(pg_isolation_regress_check) --schedule=$(srcdir)/isolation_schedule
|
||||||
|
|
||||||
# Versions of the check tests that include the prepared_transactions test
|
# Non-default tests. It only makes sense to run these if set up to use
|
||||||
# It only makes sense to run these if set up to use prepared transactions,
|
# prepared transactions, via TEMP_CONFIG for the check case, or via the
|
||||||
# via TEMP_CONFIG for the check case, or via the postgresql.conf for the
|
# postgresql.conf for the installcheck case.
|
||||||
# installcheck case.
|
|
||||||
installcheck-prepared-txns: all temp-install
|
installcheck-prepared-txns: all temp-install
|
||||||
$(pg_isolation_regress_installcheck) --schedule=$(srcdir)/isolation_schedule prepared-transactions
|
$(pg_isolation_regress_installcheck) --schedule=$(srcdir)/isolation_schedule prepared-transactions prepared-transactions-cic
|
||||||
|
|
||||||
check-prepared-txns: all temp-install
|
check-prepared-txns: all temp-install
|
||||||
$(pg_isolation_regress_check) --schedule=$(srcdir)/isolation_schedule prepared-transactions
|
$(pg_isolation_regress_check) --schedule=$(srcdir)/isolation_schedule prepared-transactions prepared-transactions-cic
|
||||||
|
@ -23,9 +23,9 @@ you can do something like
|
|||||||
./pg_isolation_regress fk-contention fk-deadlock
|
./pg_isolation_regress fk-contention fk-deadlock
|
||||||
(look into the specs/ subdirectory to see the available tests).
|
(look into the specs/ subdirectory to see the available tests).
|
||||||
|
|
||||||
The prepared-transactions test requires the server's
|
Certain tests require the server's max_prepared_transactions parameter to be
|
||||||
max_prepared_transactions parameter to be set to at least 3; therefore it
|
set to at least 3; therefore they are not run by default. To include them in
|
||||||
is not run by default. To include it in the test run, use
|
the test run, use
|
||||||
make check-prepared-txns
|
make check-prepared-txns
|
||||||
or
|
or
|
||||||
make installcheck-prepared-txns
|
make installcheck-prepared-txns
|
||||||
|
18
src/test/isolation/expected/prepared-transactions-cic.out
Normal file
18
src/test/isolation/expected/prepared-transactions-cic.out
Normal file
@ -0,0 +1,18 @@
|
|||||||
|
Parsed test spec with 2 sessions
|
||||||
|
|
||||||
|
starting permutation: w1 p1 cic2 c1 r2
|
||||||
|
step w1: BEGIN; INSERT INTO cic_test VALUES (1);
|
||||||
|
step p1: PREPARE TRANSACTION 's1';
|
||||||
|
step cic2:
|
||||||
|
CREATE INDEX CONCURRENTLY on cic_test(a);
|
||||||
|
|
||||||
|
ERROR: canceling statement due to lock timeout
|
||||||
|
step c1: COMMIT PREPARED 's1';
|
||||||
|
step r2:
|
||||||
|
SET enable_seqscan to off;
|
||||||
|
SET enable_bitmapscan to off;
|
||||||
|
SELECT * FROM cic_test WHERE a = 1;
|
||||||
|
|
||||||
|
a
|
||||||
|
|
||||||
|
1
|
37
src/test/isolation/specs/prepared-transactions-cic.spec
Normal file
37
src/test/isolation/specs/prepared-transactions-cic.spec
Normal file
@ -0,0 +1,37 @@
|
|||||||
|
# This test verifies that CREATE INDEX CONCURRENTLY interacts with prepared
|
||||||
|
# transactions correctly.
|
||||||
|
setup
|
||||||
|
{
|
||||||
|
CREATE TABLE cic_test (a int);
|
||||||
|
}
|
||||||
|
|
||||||
|
teardown
|
||||||
|
{
|
||||||
|
DROP TABLE cic_test;
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
# Sessions for CREATE INDEX CONCURRENTLY test
|
||||||
|
session "s1"
|
||||||
|
step "w1" { BEGIN; INSERT INTO cic_test VALUES (1); }
|
||||||
|
step "p1" { PREPARE TRANSACTION 's1'; }
|
||||||
|
step "c1" { COMMIT PREPARED 's1'; }
|
||||||
|
|
||||||
|
session "s2"
|
||||||
|
# The isolation tester never recognizes that a lock of s1 blocks s2, because a
|
||||||
|
# prepared transaction's locks have no pid associated. While there's a slight
|
||||||
|
# chance of timeout while waiting for an autovacuum-held lock, that wouldn't
|
||||||
|
# change the output. Hence, no timeout is too short.
|
||||||
|
setup { SET lock_timeout = 10; }
|
||||||
|
step "cic2"
|
||||||
|
{
|
||||||
|
CREATE INDEX CONCURRENTLY on cic_test(a);
|
||||||
|
}
|
||||||
|
step "r2"
|
||||||
|
{
|
||||||
|
SET enable_seqscan to off;
|
||||||
|
SET enable_bitmapscan to off;
|
||||||
|
SELECT * FROM cic_test WHERE a = 1;
|
||||||
|
}
|
||||||
|
|
||||||
|
permutation "w1" "p1" "cic2" "c1" "r2"
|
Loading…
Reference in New Issue
Block a user