From ef026a8110f40941741585e07671398beea28f11 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Wed, 27 Nov 2013 17:19:41 -0300 Subject: [PATCH 1/5] Skip looking up very old LOCK_ONLY MultiXactIds Commit 0ac5ad5134f2 removed an optimization in multixact.c that skipped fetching members of MultiXactId that were older than our OldestVisibleMXactId value. The reason this was removed is that it is possible for multis that contain updates to be older than that value. However, if the caller is certain that the multi does not contain an update (because the infomask bits say so), it can pass this info down to GetMultiXactIdMembers, enabling it to use the old optimization. Pointed out by Andres Freund in 20131121200517.GM7240@alap2.anarazel.de --- contrib/pgrowlocks/pgrowlocks.c | 3 ++- src/backend/access/heap/heapam.c | 20 +++++++++++++------- src/backend/access/transam/multixact.c | 29 ++++++++++++++++++++++++----- src/backend/utils/time/tqual.c | 11 ++++++----- src/include/access/multixact.h | 4 ++-- 5 files changed, 47 insertions(+), 20 deletions(-) diff --git a/contrib/pgrowlocks/pgrowlocks.c b/contrib/pgrowlocks/pgrowlocks.c index 636ff05..8d4961c 100644 --- a/contrib/pgrowlocks/pgrowlocks.c +++ b/contrib/pgrowlocks/pgrowlocks.c @@ -168,7 +168,8 @@ pgrowlocks(PG_FUNCTION_ARGS) allow_old = !(infomask & HEAP_LOCK_MASK) && (infomask & HEAP_XMAX_LOCK_ONLY); - nmembers = GetMultiXactIdMembers(xmax, &members, allow_old); + nmembers = GetMultiXactIdMembers(xmax, &members, allow_old, + false); if (nmembers == -1) { values[Atnum_xids] = "{0}"; diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index a21f31b..32ef453 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -3987,7 +3987,9 @@ l3: * the case, HeapTupleSatisfiesUpdate would have returned * MayBeUpdated and we wouldn't be here. */ - nmembers = GetMultiXactIdMembers(xwait, &members, false); + nmembers = + GetMultiXactIdMembers(xwait, &members, false, + HEAP_XMAX_IS_LOCKED_ONLY(infomask)); for (i = 0; i < nmembers; i++) { @@ -4008,7 +4010,8 @@ l3: } } - pfree(members); + if (members) + pfree(members); } /* @@ -4157,7 +4160,9 @@ l3: * been the case, HeapTupleSatisfiesUpdate would have returned * MayBeUpdated and we wouldn't be here. */ - nmembers = GetMultiXactIdMembers(xwait, &members, false); + nmembers = + GetMultiXactIdMembers(xwait, &members, false, + HEAP_XMAX_IS_LOCKED_ONLY(infomask)); if (nmembers <= 0) { @@ -4626,7 +4631,7 @@ l5: * MultiXactIdExpand if we weren't to do this, so this check is not * incurring extra work anyhow. */ - if (!MultiXactIdIsRunning(xmax)) + if (!MultiXactIdIsRunning(xmax, HEAP_XMAX_IS_LOCKED_ONLY(old_infomask))) { if (HEAP_XMAX_IS_LOCKED_ONLY(old_infomask) || TransactionIdDidAbort(MultiXactIdGetUpdateXid(xmax, @@ -5217,7 +5222,7 @@ GetMultiXactIdHintBits(MultiXactId multi, uint16 *new_infomask, * We only use this in multis we just created, so they cannot be values * pre-pg_upgrade. */ - nmembers = GetMultiXactIdMembers(multi, &members, false); + nmembers = GetMultiXactIdMembers(multi, &members, false, false); for (i = 0; i < nmembers; i++) { @@ -5293,7 +5298,7 @@ MultiXactIdGetUpdateXid(TransactionId xmax, uint16 t_infomask) * Since we know the LOCK_ONLY bit is not set, this cannot be a multi from * pre-pg_upgrade. */ - nmembers = GetMultiXactIdMembers(xmax, &members, false); + nmembers = GetMultiXactIdMembers(xmax, &members, false, false); if (nmembers > 0) { @@ -5376,7 +5381,8 @@ Do_MultiXactIdWait(MultiXactId multi, MultiXactStatus status, int remain = 0; allow_old = !(infomask & HEAP_LOCK_MASK) && HEAP_XMAX_IS_LOCKED_ONLY(infomask); - nmembers = GetMultiXactIdMembers(multi, &members, allow_old); + nmembers = GetMultiXactIdMembers(multi, &members, allow_old, + HEAP_XMAX_IS_LOCKED_ONLY(infomask)); if (nmembers >= 0) { diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c index e3f5cbc..20b0ea2 100644 --- a/src/backend/access/transam/multixact.c +++ b/src/backend/access/transam/multixact.c @@ -391,7 +391,7 @@ MultiXactIdExpand(MultiXactId multi, TransactionId xid, MultiXactStatus status) * caller of this function does a check that the multixact is no longer * running. */ - nmembers = GetMultiXactIdMembers(multi, &members, false); + nmembers = GetMultiXactIdMembers(multi, &members, false, false); if (nmembers < 0) { @@ -477,7 +477,7 @@ MultiXactIdExpand(MultiXactId multi, TransactionId xid, MultiXactStatus status) * a pg_upgraded share-locked tuple. */ bool -MultiXactIdIsRunning(MultiXactId multi) +MultiXactIdIsRunning(MultiXactId multi, bool isLockOnly) { MultiXactMember *members; int nmembers; @@ -489,7 +489,7 @@ MultiXactIdIsRunning(MultiXactId multi) * "false" here means we assume our callers have checked that the given * multi cannot possibly come from a pg_upgraded database. */ - nmembers = GetMultiXactIdMembers(multi, &members, false); + nmembers = GetMultiXactIdMembers(multi, &members, false, isLockOnly); if (nmembers < 0) { @@ -1029,10 +1029,15 @@ GetNewMultiXactId(int nmembers, MultiXactOffset *offset) * the value currently known as the next to assign, raise an error. Previously * these also returned -1, but since this can lead to the wrong visibility * results, it is dangerous to do that. + * + * onlyLock must be set to true if caller is certain that the given multi + * is used only to lock tuples; can be false without loss of correctness, + * but passing a true means we can return quickly without checking for + * old updates. */ int GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **members, - bool allow_old) + bool allow_old, bool onlyLock) { int pageno; int prev_pageno; @@ -1066,6 +1071,19 @@ GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **members, MultiXactIdSetOldestVisible(); /* + * If we know the multi is used only for locking and not for updates, + * then we can skip checking if the value is older than our oldest + * visible multi. It cannot possibly still be running. + */ + if (onlyLock && + MultiXactIdPrecedes(multi, OldestVisibleMXactId[MyBackendId])) + { + debug_elog2(DEBUG2, "GetMembers: a locker-only multi is too old"); + *members = NULL; + return -1; + } + + /* * We check known limits on MultiXact before resorting to the SLRU area. * * An ID older than MultiXactState->oldestMultiXactId cannot possibly be @@ -2507,7 +2525,8 @@ pg_get_multixact_members(PG_FUNCTION_ARGS) multi = palloc(sizeof(mxact)); /* no need to allow for old values here */ - multi->nmembers = GetMultiXactIdMembers(mxid, &multi->members, false); + multi->nmembers = GetMultiXactIdMembers(mxid, &multi->members, false, + false); multi->iter = 0; tupdesc = CreateTemplateTupleDesc(2, false); diff --git a/src/backend/utils/time/tqual.c b/src/backend/utils/time/tqual.c index ed66c49..e0d627b 100644 --- a/src/backend/utils/time/tqual.c +++ b/src/backend/utils/time/tqual.c @@ -565,7 +565,7 @@ HeapTupleSatisfiesUpdate(HeapTuple htup, CommandId curcid, */ if ((tuple->t_infomask & (HEAP_XMAX_EXCL_LOCK | HEAP_XMAX_KEYSHR_LOCK)) && - MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple))) + MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple), true)) return HeapTupleBeingUpdated; SetHintBits(tuple, buffer, HEAP_XMAX_INVALID, InvalidTransactionId); @@ -575,7 +575,7 @@ HeapTupleSatisfiesUpdate(HeapTuple htup, CommandId curcid, xmax = HeapTupleGetUpdateXid(tuple); if (!TransactionIdIsValid(xmax)) { - if (MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple))) + if (MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple), false)) return HeapTupleBeingUpdated; SetHintBits(tuple, buffer, HEAP_XMAX_INVALID, InvalidTransactionId); @@ -590,7 +590,7 @@ HeapTupleSatisfiesUpdate(HeapTuple htup, CommandId curcid, return HeapTupleInvisible; /* updated before scan started */ } - if (MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple))) + if (MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple), false)) return HeapTupleBeingUpdated; if (TransactionIdDidCommit(xmax)) @@ -1158,7 +1158,8 @@ HeapTupleSatisfiesVacuum(HeapTuple htup, TransactionId OldestXmin, */ if ((tuple->t_infomask & (HEAP_XMAX_EXCL_LOCK | HEAP_XMAX_KEYSHR_LOCK)) && - MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple))) + MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple), + true)) return HEAPTUPLE_LIVE; SetHintBits(tuple, buffer, HEAP_XMAX_INVALID, InvalidTransactionId); @@ -1185,7 +1186,7 @@ HeapTupleSatisfiesVacuum(HeapTuple htup, TransactionId OldestXmin, { TransactionId xmax; - if (MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple))) + if (MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple), false)) { /* already checked above */ Assert(!HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask)); diff --git a/src/include/access/multixact.h b/src/include/access/multixact.h index 8a9edde..3572992 100644 --- a/src/include/access/multixact.h +++ b/src/include/access/multixact.h @@ -82,10 +82,10 @@ extern MultiXactId MultiXactIdCreate(TransactionId xid1, extern MultiXactId MultiXactIdExpand(MultiXactId multi, TransactionId xid, MultiXactStatus status); extern MultiXactId ReadNextMultiXactId(void); -extern bool MultiXactIdIsRunning(MultiXactId multi); +extern bool MultiXactIdIsRunning(MultiXactId multi, bool isLockOnly); extern void MultiXactIdSetOldestMember(void); extern int GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **xids, - bool allow_old); + bool allow_old, bool isLockOnly); extern bool MultiXactIdPrecedes(MultiXactId multi1, MultiXactId multi2); extern void AtEOXact_MultiXact(void); -- 1.7.10.4