Skip to content

Commit 0531549

Browse files
alvherrealvherre
committed
Avoid uselessly looking up old LOCK_ONLY multixacts
Commit 0ac5ad5 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 multixacts 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
1 parent c258179 commit 0531549

File tree

5 files changed

+62
-25
lines changed

5 files changed

+62
-25
lines changed

‎contrib/pgrowlocks/pgrowlocks.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,8 @@ pgrowlocks(PG_FUNCTION_ARGS)
165165

166166
allow_old = !(infomask & HEAP_LOCK_MASK) &&
167167
(infomask & HEAP_XMAX_LOCK_ONLY);
168-
nmembers = GetMultiXactIdMembers(xmax, &members, allow_old);
168+
nmembers = GetMultiXactIdMembers(xmax, &members, allow_old,
169+
false);
169170
if (nmembers == -1)
170171
{
171172
values[Atnum_xids] = "{0}";

‎src/backend/access/heap/heapam.c

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4183,7 +4183,9 @@ heap_lock_tuple(Relation relation, HeapTuple tuple,
41834183
* the case, HeapTupleSatisfiesUpdate would have returned
41844184
* MayBeUpdated and we wouldn't be here.
41854185
*/
4186-
nmembers = GetMultiXactIdMembers(xwait, &members, false);
4186+
nmembers =
4187+
GetMultiXactIdMembers(xwait, &members, false,
4188+
HEAP_XMAX_IS_LOCKED_ONLY(infomask));
41874189

41884190
for (i = 0; i < nmembers; i++)
41894191
{
@@ -4204,7 +4206,8 @@ heap_lock_tuple(Relation relation, HeapTuple tuple,
42044206
}
42054207
}
42064208

4207-
pfree(members);
4209+
if (members)
4210+
pfree(members);
42084211
}
42094212

42104213
/*
@@ -4353,7 +4356,9 @@ heap_lock_tuple(Relation relation, HeapTuple tuple,
43534356
* been the case, HeapTupleSatisfiesUpdate would have returned
43544357
* MayBeUpdated and we wouldn't be here.
43554358
*/
4356-
nmembers = GetMultiXactIdMembers(xwait, &members, false);
4359+
nmembers =
4360+
GetMultiXactIdMembers(xwait, &members, false,
4361+
HEAP_XMAX_IS_LOCKED_ONLY(infomask));
43574362

43584363
if (nmembers <= 0)
43594364
{
@@ -4834,7 +4839,7 @@ compute_new_xmax_infomask(TransactionId xmax, uint16 old_infomask,
48344839
* MultiXactIdExpand if we weren't to do this, so this check is not
48354840
* incurring extra work anyhow.
48364841
*/
4837-
if (!MultiXactIdIsRunning(xmax))
4842+
if (!MultiXactIdIsRunning(xmax, HEAP_XMAX_IS_LOCKED_ONLY(old_infomask)))
48384843
{
48394844
if (HEAP_XMAX_IS_LOCKED_ONLY(old_infomask) ||
48404845
TransactionIdDidAbort(MultiXactIdGetUpdateXid(xmax,
@@ -5175,7 +5180,8 @@ heap_lock_updated_tuple_rec(Relation rel, ItemPointer tid, TransactionId xid,
51755180
int i;
51765181
MultiXactMember *members;
51775182

5178-
nmembers = GetMultiXactIdMembers(rawxmax, &members, false);
5183+
nmembers = GetMultiXactIdMembers(rawxmax, &members, false,
5184+
HEAP_XMAX_IS_LOCKED_ONLY(old_infomask));
51795185
for (i = 0; i < nmembers; i++)
51805186
{
51815187
HTSU_Result res;
@@ -5533,7 +5539,8 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
55335539
*/
55345540
Assert((!(t_infomask & HEAP_LOCK_MASK) &&
55355541
HEAP_XMAX_IS_LOCKED_ONLY(t_infomask)) ||
5536-
!MultiXactIdIsRunning(multi));
5542+
!MultiXactIdIsRunning(multi,
5543+
HEAP_XMAX_IS_LOCKED_ONLY(t_infomask)));
55375544
if (HEAP_XMAX_IS_LOCKED_ONLY(t_infomask))
55385545
{
55395546
*flags |= FRM_INVALIDATE_XMAX;
@@ -5576,7 +5583,9 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
55765583

55775584
allow_old = !(t_infomask & HEAP_LOCK_MASK) &&
55785585
HEAP_XMAX_IS_LOCKED_ONLY(t_infomask);
5579-
nmembers = GetMultiXactIdMembers(multi, &members, allow_old);
5586+
nmembers =
5587+
GetMultiXactIdMembers(multi, &members, allow_old,
5588+
HEAP_XMAX_IS_LOCKED_ONLY(t_infomask));
55805589
if (nmembers <= 0)
55815590
{
55825591
/* Nothing worth keeping */
@@ -5983,7 +5992,7 @@ GetMultiXactIdHintBits(MultiXactId multi, uint16 *new_infomask,
59835992
* We only use this in multis we just created, so they cannot be values
59845993
* pre-pg_upgrade.
59855994
*/
5986-
nmembers = GetMultiXactIdMembers(multi, &members, false);
5995+
nmembers = GetMultiXactIdMembers(multi, &members, false, false);
59875996

59885997
for (i = 0; i < nmembers; i++)
59895998
{
@@ -6062,7 +6071,7 @@ MultiXactIdGetUpdateXid(TransactionId xmax, uint16 t_infomask)
60626071
* Since we know the LOCK_ONLY bit is not set, this cannot be a multi from
60636072
* pre-pg_upgrade.
60646073
*/
6065-
nmembers = GetMultiXactIdMembers(xmax, &members, false);
6074+
nmembers = GetMultiXactIdMembers(xmax, &members, false, false);
60666075

60676076
if (nmembers > 0)
60686077
{
@@ -6148,7 +6157,8 @@ Do_MultiXactIdWait(MultiXactId multi, MultiXactStatus status,
61486157
int remain = 0;
61496158

61506159
allow_old = !(infomask & HEAP_LOCK_MASK) && HEAP_XMAX_IS_LOCKED_ONLY(infomask);
6151-
nmembers = GetMultiXactIdMembers(multi, &members, allow_old);
6160+
nmembers = GetMultiXactIdMembers(multi, &members, allow_old,
6161+
HEAP_XMAX_IS_LOCKED_ONLY(infomask));
61526162

61536163
if (nmembers >= 0)
61546164
{
@@ -6294,7 +6304,8 @@ heap_tuple_needs_freeze(HeapTupleHeader tuple, TransactionId cutoff_xid,
62946304

62956305
allow_old = !(tuple->t_infomask & HEAP_LOCK_MASK) &&
62966306
HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask);
6297-
nmembers = GetMultiXactIdMembers(multi, &members, allow_old);
6307+
nmembers = GetMultiXactIdMembers(multi, &members, allow_old,
6308+
HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask));
62986309

62996310
for (i = 0; i < nmembers; i++)
63006311
{

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

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -428,7 +428,7 @@ MultiXactIdExpand(MultiXactId multi, TransactionId xid, MultiXactStatus status)
428428
* caller of this function does a check that the multixact is no longer
429429
* running.
430430
*/
431-
nmembers = GetMultiXactIdMembers(multi, &members, false);
431+
nmembers = GetMultiXactIdMembers(multi, &members, false, false);
432432

433433
if (nmembers < 0)
434434
{
@@ -517,7 +517,7 @@ MultiXactIdExpand(MultiXactId multi, TransactionId xid, MultiXactStatus status)
517517
* a pg_upgraded share-locked tuple.
518518
*/
519519
bool
520-
MultiXactIdIsRunning(MultiXactId multi)
520+
MultiXactIdIsRunning(MultiXactId multi, bool isLockOnly)
521521
{
522522
MultiXactMember *members;
523523
int nmembers;
@@ -529,7 +529,7 @@ MultiXactIdIsRunning(MultiXactId multi)
529529
* "false" here means we assume our callers have checked that the given
530530
* multi cannot possibly come from a pg_upgraded database.
531531
*/
532-
nmembers = GetMultiXactIdMembers(multi, &members, false);
532+
nmembers = GetMultiXactIdMembers(multi, &members, false, isLockOnly);
533533

534534
if (nmembers < 0)
535535
{
@@ -1095,10 +1095,15 @@ GetNewMultiXactId(int nmembers, MultiXactOffset *offset)
10951095
* the value currently known as the next to assign, raise an error. Previously
10961096
* these also returned -1, but since this can lead to the wrong visibility
10971097
* results, it is dangerous to do that.
1098+
*
1099+
* onlyLock must be set to true if caller is certain that the given multi
1100+
* is used only to lock tuples; can be false without loss of correctness,
1101+
* but passing a true means we can return quickly without checking for
1102+
* old updates.
10981103
*/
10991104
int
11001105
GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **members,
1101-
bool allow_old)
1106+
bool allow_old, bool onlyLock)
11021107
{
11031108
int pageno;
11041109
int prev_pageno;
@@ -1132,6 +1137,19 @@ GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **members,
11321137
/* Set our OldestVisibleMXactId[] entry if we didn't already */
11331138
MultiXactIdSetOldestVisible();
11341139

1140+
/*
1141+
* If we know the multi is used only for locking and not for updates,
1142+
* then we can skip checking if the value is older than our oldest
1143+
* visible multi. It cannot possibly still be running.
1144+
*/
1145+
if (onlyLock &&
1146+
MultiXactIdPrecedes(multi, OldestVisibleMXactId[MyBackendId]))
1147+
{
1148+
debug_elog2(DEBUG2, "GetMembers: a locker-only multi is too old");
1149+
*members = NULL;
1150+
return -1;
1151+
}
1152+
11351153
/*
11361154
* We check known limits on MultiXact before resorting to the SLRU area.
11371155
*
@@ -1335,7 +1353,7 @@ MultiXactHasRunningRemoteMembers(MultiXactId multi)
13351353
int nmembers;
13361354
int i;
13371355

1338-
nmembers = GetMultiXactIdMembers(multi, &members, true);
1356+
nmembers = GetMultiXactIdMembers(multi, &members, true, false);
13391357
if (nmembers <= 0)
13401358
return false;
13411359

@@ -2807,7 +2825,8 @@ pg_get_multixact_members(PG_FUNCTION_ARGS)
28072825

28082826
multi = palloc(sizeof(mxact));
28092827
/* no need to allow for old values here */
2810-
multi->nmembers = GetMultiXactIdMembers(mxid, &multi->members, false);
2828+
multi->nmembers = GetMultiXactIdMembers(mxid, &multi->members, false,
2829+
false);
28112830
multi->iter = 0;
28122831

28132832
tupdesc = CreateTemplateTupleDesc(2, false);

‎src/backend/utils/time/tqual.c

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -607,14 +607,19 @@ HeapTupleSatisfiesUpdate(HeapTuple htup, CommandId curcid,
607607
*/
608608
if ((tuple->t_infomask & (HEAP_XMAX_EXCL_LOCK |
609609
HEAP_XMAX_KEYSHR_LOCK)) &&
610-
MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple)))
610+
MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple), true))
611611
return HeapTupleBeingUpdated;
612612

613613
SetHintBits(tuple, buffer, HEAP_XMAX_INVALID, InvalidTransactionId);
614614
return HeapTupleMayBeUpdated;
615615
}
616616

617617
xmax = HeapTupleGetUpdateXid(tuple);
618+
if (!TransactionIdIsValid(xmax))
619+
{
620+
if (MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple), false))
621+
return HeapTupleBeingUpdated;
622+
}
618623

619624
/* not LOCKED_ONLY, so it has to have an xmax */
620625
Assert(TransactionIdIsValid(xmax));
@@ -627,7 +632,7 @@ HeapTupleSatisfiesUpdate(HeapTuple htup, CommandId curcid,
627632
return HeapTupleInvisible; /* updated before scan started */
628633
}
629634

630-
if (TransactionIdIsInProgress(xmax))
635+
if (MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple), false))
631636
return HeapTupleBeingUpdated;
632637

633638
if (TransactionIdDidCommit(xmax))
@@ -638,7 +643,7 @@ HeapTupleSatisfiesUpdate(HeapTuple htup, CommandId curcid,
638643
* what about the other members?
639644
*/
640645

641-
if (!MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple)))
646+
if (!MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple), false))
642647
{
643648
/*
644649
* There's no member, even just a locker, alive anymore, so we can
@@ -1240,7 +1245,8 @@ HeapTupleSatisfiesVacuum(HeapTuple htup, TransactionId OldestXmin,
12401245
*/
12411246
if ((tuple->t_infomask & (HEAP_XMAX_EXCL_LOCK |
12421247
HEAP_XMAX_KEYSHR_LOCK)) &&
1243-
MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple)))
1248+
MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple),
1249+
true))
12441250
return HEAPTUPLE_LIVE;
12451251
SetHintBits(tuple, buffer, HEAP_XMAX_INVALID, InvalidTransactionId);
12461252

@@ -1267,7 +1273,7 @@ HeapTupleSatisfiesVacuum(HeapTuple htup, TransactionId OldestXmin,
12671273
{
12681274
TransactionId xmax;
12691275

1270-
if (MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple)))
1276+
if (MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple), false))
12711277
{
12721278
/* already checked above */
12731279
Assert(!HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask));

‎src/include/access/multixact.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,10 +91,10 @@ extern MultiXactId MultiXactIdCreateFromMembers(int nmembers,
9191
MultiXactMember *members);
9292

9393
extern MultiXactId ReadNextMultiXactId(void);
94-
extern bool MultiXactIdIsRunning(MultiXactId multi);
94+
extern bool MultiXactIdIsRunning(MultiXactId multi, bool isLockOnly);
9595
extern void MultiXactIdSetOldestMember(void);
9696
extern int GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **xids,
97-
bool allow_old);
97+
bool allow_old, bool isLockOnly);
9898
extern bool MultiXactHasRunningRemoteMembers(MultiXactId multi);
9999
extern bool MultiXactIdPrecedes(MultiXactId multi1, MultiXactId multi2);
100100
extern bool MultiXactIdPrecedesOrEquals(MultiXactId multi1,

0 commit comments

Comments
 (0)