Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit1ce150b

Browse files
committed
Don't TransactionIdDidAbort in HeapTupleGetUpdateXid
It is dangerous to do so, because some code expects to be able to see what'sthe true Xmax even if it is aborted (particularly while traversing HOTchains). So don't do it, and instead rely on the callers to verify forabortedness, if necessary.Several race conditions and bugs fixed in the process. One isolation testchanges the expected output due to these.This also reverts commitc235a6a, which is no longer necessary.Backpatch to 9.3, where this function was introduced.Andres Freund
1 parent1df0122 commit1ce150b

File tree

4 files changed

+75
-74
lines changed

4 files changed

+75
-74
lines changed

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

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3181,7 +3181,11 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
31813181
if (!HEAP_XMAX_IS_LOCKED_ONLY(oldtup.t_data->t_infomask))
31823182
update_xact=HeapTupleGetUpdateXid(oldtup.t_data);
31833183

3184-
/* there was no UPDATE in the MultiXact; or it aborted. */
3184+
/*
3185+
* There was no UPDATE in the MultiXact; or it aborted. No
3186+
* TransactionIdIsInProgress() call needed here, since we called
3187+
* MultiXactIdWait() above.
3188+
*/
31853189
if (!TransactionIdIsValid(update_xact)||
31863190
TransactionIdDidAbort(update_xact))
31873191
can_continue= true;
@@ -5441,6 +5445,9 @@ GetMultiXactIdHintBits(MultiXactId multi, uint16 *new_infomask,
54415445
* Given a multixact Xmax and corresponding infomask, which does not have the
54425446
* HEAP_XMAX_LOCK_ONLY bit set, obtain and return the Xid of the updating
54435447
* transaction.
5448+
*
5449+
* Caller is expected to check the status of the updating transaction, if
5450+
* necessary.
54445451
*/
54455452
staticTransactionId
54465453
MultiXactIdGetUpdateXid(TransactionIdxmax,uint16t_infomask)
@@ -5465,19 +5472,11 @@ MultiXactIdGetUpdateXid(TransactionId xmax, uint16 t_infomask)
54655472
for (i=0;i<nmembers;i++)
54665473
{
54675474
/* Ignore lockers */
5468-
if (members[i].status==MultiXactStatusForKeyShare||
5469-
members[i].status==MultiXactStatusForShare||
5470-
members[i].status==MultiXactStatusForNoKeyUpdate||
5471-
members[i].status==MultiXactStatusForUpdate)
5475+
if (!ISUPDATE_from_mxstatus(members[i].status))
54725476
continue;
54735477

5474-
/* ignore aborted transactions */
5475-
if (TransactionIdDidAbort(members[i].xid))
5476-
continue;
5477-
/* there should be at most one non-aborted updater */
5478+
/* there can be at most one updater */
54785479
Assert(update_xact==InvalidTransactionId);
5479-
Assert(members[i].status==MultiXactStatusNoKeyUpdate||
5480-
members[i].status==MultiXactStatusUpdate);
54815480
update_xact=members[i].xid;
54825481
#ifndefUSE_ASSERT_CHECKING
54835482

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

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -479,22 +479,12 @@ heap_prune_chain(Relation relation, Buffer buffer, OffsetNumber rootoffnum,
479479
break;
480480

481481
caseHEAPTUPLE_DELETE_IN_PROGRESS:
482-
{
483-
TransactionIdxmax;
484-
485-
/*
486-
* This tuple may soon become DEAD. Update the hint field
487-
* so that the page is reconsidered for pruning in future.
488-
* If there was a MultiXactId updater, and it aborted after
489-
* HTSV checked, then we will get an invalid Xid here.
490-
* There is no need for future pruning of the page in that
491-
* case, so skip it.
492-
*/
493-
xmax=HeapTupleHeaderGetUpdateXid(htup);
494-
if (TransactionIdIsValid(xmax))
495-
heap_prune_record_prunable(prstate,xmax);
496-
}
497-
482+
/*
483+
* This tuple may soon become DEAD. Update the hint field
484+
* so that the page is reconsidered for pruning in future.
485+
*/
486+
heap_prune_record_prunable(prstate,
487+
HeapTupleHeaderGetUpdateXid(htup));
498488
break;
499489

500490
caseHEAPTUPLE_LIVE:

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

Lines changed: 51 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -223,8 +223,9 @@ HeapTupleSatisfiesSelf(HeapTuple htup, Snapshot snapshot, Buffer buffer)
223223
TransactionIdxmax;
224224

225225
xmax=HeapTupleGetUpdateXid(tuple);
226-
if (!TransactionIdIsValid(xmax))
227-
return true;
226+
227+
/* not LOCKED_ONLY, so it has to have an xmax */
228+
Assert(TransactionIdIsValid(xmax));
228229

229230
/* updating subtransaction must have aborted */
230231
if (!TransactionIdIsCurrentTransactionId(xmax))
@@ -277,14 +278,17 @@ HeapTupleSatisfiesSelf(HeapTuple htup, Snapshot snapshot, Buffer buffer)
277278
return true;
278279

279280
xmax=HeapTupleGetUpdateXid(tuple);
280-
if (!TransactionIdIsValid(xmax))
281-
return true;
281+
282+
/* not LOCKED_ONLY, so it has to have an xmax */
283+
Assert(TransactionIdIsValid(xmax));
284+
282285
if (TransactionIdIsCurrentTransactionId(xmax))
283286
return false;
284287
if (TransactionIdIsInProgress(xmax))
285288
return true;
286289
if (TransactionIdDidCommit(xmax))
287290
return false;
291+
/* it must have aborted or crashed */
288292
return true;
289293
}
290294

@@ -497,8 +501,9 @@ HeapTupleSatisfiesUpdate(HeapTuple htup, CommandId curcid,
497501
TransactionIdxmax;
498502

499503
xmax=HeapTupleGetUpdateXid(tuple);
500-
if (!TransactionIdIsValid(xmax))
501-
returnHeapTupleMayBeUpdated;
504+
505+
/* not LOCKED_ONLY, so it has to have an xmax */
506+
Assert(TransactionIdIsValid(xmax));
502507

503508
/* updating subtransaction must have aborted */
504509
if (!TransactionIdIsCurrentTransactionId(xmax))
@@ -573,14 +578,9 @@ HeapTupleSatisfiesUpdate(HeapTuple htup, CommandId curcid,
573578
}
574579

575580
xmax=HeapTupleGetUpdateXid(tuple);
576-
if (!TransactionIdIsValid(xmax))
577-
{
578-
if (MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple)))
579-
returnHeapTupleBeingUpdated;
580581

581-
SetHintBits(tuple,buffer,HEAP_XMAX_INVALID,InvalidTransactionId);
582-
returnHeapTupleMayBeUpdated;
583-
}
582+
/* not LOCKED_ONLY, so it has to have an xmax */
583+
Assert(TransactionIdIsValid(xmax));
584584

585585
if (TransactionIdIsCurrentTransactionId(xmax))
586586
{
@@ -590,13 +590,18 @@ HeapTupleSatisfiesUpdate(HeapTuple htup, CommandId curcid,
590590
returnHeapTupleInvisible;/* updated before scan started */
591591
}
592592

593-
if (MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple)))
593+
if (TransactionIdIsInProgress(xmax))
594594
returnHeapTupleBeingUpdated;
595595

596596
if (TransactionIdDidCommit(xmax))
597597
returnHeapTupleUpdated;
598+
599+
/* no member, even just a locker, alive anymore */
600+
if (!MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple)))
601+
SetHintBits(tuple,buffer,HEAP_XMAX_INVALID,
602+
InvalidTransactionId);
603+
598604
/* it must have aborted or crashed */
599-
SetHintBits(tuple,buffer,HEAP_XMAX_INVALID,InvalidTransactionId);
600605
returnHeapTupleMayBeUpdated;
601606
}
602607

@@ -722,8 +727,9 @@ HeapTupleSatisfiesDirty(HeapTuple htup, Snapshot snapshot,
722727
TransactionIdxmax;
723728

724729
xmax=HeapTupleGetUpdateXid(tuple);
725-
if (!TransactionIdIsValid(xmax))
726-
return true;
730+
731+
/* not LOCKED_ONLY, so it has to have an xmax */
732+
Assert(TransactionIdIsValid(xmax));
727733

728734
/* updating subtransaction must have aborted */
729735
if (!TransactionIdIsCurrentTransactionId(xmax))
@@ -780,8 +786,10 @@ HeapTupleSatisfiesDirty(HeapTuple htup, Snapshot snapshot,
780786
return true;
781787

782788
xmax=HeapTupleGetUpdateXid(tuple);
783-
if (!TransactionIdIsValid(xmax))
784-
return true;
789+
790+
/* not LOCKED_ONLY, so it has to have an xmax */
791+
Assert(TransactionIdIsValid(xmax));
792+
785793
if (TransactionIdIsCurrentTransactionId(xmax))
786794
return false;
787795
if (TransactionIdIsInProgress(xmax))
@@ -791,6 +799,7 @@ HeapTupleSatisfiesDirty(HeapTuple htup, Snapshot snapshot,
791799
}
792800
if (TransactionIdDidCommit(xmax))
793801
return false;
802+
/* it must have aborted or crashed */
794803
return true;
795804
}
796805

@@ -915,8 +924,9 @@ HeapTupleSatisfiesMVCC(HeapTuple htup, Snapshot snapshot,
915924
TransactionIdxmax;
916925

917926
xmax=HeapTupleGetUpdateXid(tuple);
918-
if (!TransactionIdIsValid(xmax))
919-
return true;
927+
928+
/* not LOCKED_ONLY, so it has to have an xmax */
929+
Assert(TransactionIdIsValid(xmax));
920930

921931
/* updating subtransaction must have aborted */
922932
if (!TransactionIdIsCurrentTransactionId(xmax))
@@ -975,8 +985,10 @@ HeapTupleSatisfiesMVCC(HeapTuple htup, Snapshot snapshot,
975985
Assert(!HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask));
976986

977987
xmax=HeapTupleGetUpdateXid(tuple);
978-
if (!TransactionIdIsValid(xmax))
979-
return true;
988+
989+
/* not LOCKED_ONLY, so it has to have an xmax */
990+
Assert(TransactionIdIsValid(xmax));
991+
980992
if (TransactionIdIsCurrentTransactionId(xmax))
981993
{
982994
if (HeapTupleHeaderGetCmax(tuple) >=snapshot->curcid)
@@ -993,6 +1005,7 @@ HeapTupleSatisfiesMVCC(HeapTuple htup, Snapshot snapshot,
9931005
return true;/* treat as still in progress */
9941006
return false;
9951007
}
1008+
/* it must have aborted or crashed */
9961009
return true;
9971010
}
9981011

@@ -1191,8 +1204,10 @@ HeapTupleSatisfiesVacuum(HeapTuple htup, TransactionId OldestXmin,
11911204
Assert(!HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask));
11921205

11931206
xmax=HeapTupleGetUpdateXid(tuple);
1194-
if (!TransactionIdIsValid(xmax))
1195-
returnHEAPTUPLE_LIVE;
1207+
1208+
/* not LOCKED_ONLY, so it has to have an xmax */
1209+
Assert(TransactionIdIsValid(xmax));
1210+
11961211
if (TransactionIdIsInProgress(xmax))
11971212
returnHEAPTUPLE_DELETE_IN_PROGRESS;
11981213
elseif (TransactionIdDidCommit(xmax))
@@ -1205,8 +1220,10 @@ HeapTupleSatisfiesVacuum(HeapTuple htup, TransactionId OldestXmin,
12051220
Assert(!(tuple->t_infomask&HEAP_XMAX_COMMITTED));
12061221

12071222
xmax=HeapTupleGetUpdateXid(tuple);
1208-
if (!TransactionIdIsValid(xmax))
1209-
returnHEAPTUPLE_LIVE;
1223+
1224+
/* not LOCKED_ONLY, so it has to have an xmax */
1225+
Assert(TransactionIdIsValid(xmax));
1226+
12101227
/* multi is not running -- updating xact cannot be */
12111228
Assert(!TransactionIdIsInProgress(xmax));
12121229
if (TransactionIdDidCommit(xmax))
@@ -1216,22 +1233,13 @@ HeapTupleSatisfiesVacuum(HeapTuple htup, TransactionId OldestXmin,
12161233
else
12171234
returnHEAPTUPLE_DEAD;
12181235
}
1219-
else
1220-
{
1221-
/*
1222-
* Not in Progress, Not Committed, so either Aborted or crashed.
1223-
*/
1224-
SetHintBits(tuple,buffer,HEAP_XMAX_INVALID,InvalidTransactionId);
1225-
returnHEAPTUPLE_LIVE;
1226-
}
12271236

12281237
/*
1229-
*Deleter committed, but perhaps it was recent enough that some open
1230-
*transactions could still seethetuple.
1238+
*Not in Progress, Not Committed, so either Aborted or crashed.
1239+
*RemovetheXmax.
12311240
*/
1232-
1233-
/* Otherwise, it's dead and removable */
1234-
returnHEAPTUPLE_DEAD;
1241+
SetHintBits(tuple,buffer,HEAP_XMAX_INVALID,InvalidTransactionId);
1242+
returnHEAPTUPLE_LIVE;
12351243
}
12361244

12371245
if (!(tuple->t_infomask&HEAP_XMAX_COMMITTED))
@@ -1474,8 +1482,9 @@ HeapTupleHeaderIsOnlyLocked(HeapTupleHeader tuple)
14741482

14751483
/* ... but if it's a multi, then perhaps the updating Xid aborted. */
14761484
xmax=HeapTupleGetUpdateXid(tuple);
1477-
if (!TransactionIdIsValid(xmax))/* shouldn't happen .. */
1478-
return true;
1485+
1486+
/* not LOCKED_ONLY, so it has to have an xmax */
1487+
Assert(TransactionIdIsValid(xmax));
14791488

14801489
if (TransactionIdIsCurrentTransactionId(xmax))
14811490
return false;

‎src/test/isolation/expected/delete-abort-savept.out‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,11 @@ key value
2323
step s1svp: SAVEPOINT f;
2424
step s1d: DELETE FROM foo;
2525
step s1r: ROLLBACK TO f;
26-
step s2l: SELECT * FROM foo FOR UPDATE; <waiting ...>
27-
step s1c: COMMIT;
28-
step s2l: <... completed>
26+
step s2l: SELECT * FROM foo FOR UPDATE;
2927
key value
3028

3129
1 1
30+
step s1c: COMMIT;
3231
step s2c: COMMIT;
3332

3433
starting permutation: s1l s1svp s1d s1r s2l s2c s1c
@@ -39,8 +38,12 @@ key value
3938
step s1svp: SAVEPOINT f;
4039
step s1d: DELETE FROM foo;
4140
step s1r: ROLLBACK TO f;
42-
step s2l: SELECT * FROM foo FOR UPDATE; <waiting ...>
43-
invalid permutation detected
41+
step s2l: SELECT * FROM foo FOR UPDATE;
42+
key value
43+
44+
1 1
45+
step s2c: COMMIT;
46+
step s1c: COMMIT;
4447

4548
starting permutation: s1l s1svp s1d s2l s1r s1c s2c
4649
step s1l: SELECT * FROM foo FOR KEY SHARE;

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp