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

Commitf05ae2f

Browse files
committed
Revert bogus fixes of HOT-freezing bug
It turns out we misdiagnosed what the real problem was. Revert theprevious changes, because they may have worse consequences goingforward. A better fix is forthcoming.The simplistic test case is kept, though disabled.Discussion:https://postgr.es/m/20171102112019.33wb7g5wp4zpjelu@alap3.anarazel.de
1 parente89867c commitf05ae2f

File tree

6 files changed

+60
-105
lines changed

6 files changed

+60
-105
lines changed

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

Lines changed: 46 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -1718,7 +1718,8 @@ heap_hot_search_buffer(ItemPointer tid, Relation relation, Buffer buffer,
17181718
* broken.
17191719
*/
17201720
if (TransactionIdIsValid(prev_xmax)&&
1721-
!HeapTupleUpdateXmaxMatchesXmin(prev_xmax,heapTuple->t_data))
1721+
!TransactionIdEquals(prev_xmax,
1722+
HeapTupleHeaderGetXmin(heapTuple->t_data)))
17221723
break;
17231724

17241725
/*
@@ -1887,7 +1888,7 @@ heap_get_latest_tid(Relation relation,
18871888
* tuple. Check for XMIN match.
18881889
*/
18891890
if (TransactionIdIsValid(priorXmax)&&
1890-
!HeapTupleUpdateXmaxMatchesXmin(priorXmax,tp.t_data))
1891+
!TransactionIdEquals(priorXmax,HeapTupleHeaderGetXmin(tp.t_data)))
18911892
{
18921893
UnlockReleaseBuffer(buffer);
18931894
break;
@@ -1919,39 +1920,6 @@ heap_get_latest_tid(Relation relation,
19191920
}/* end of loop */
19201921
}
19211922

1922-
/*
1923-
* HeapTupleUpdateXmaxMatchesXmin - verify update chain xmax/xmin lineage
1924-
*
1925-
* Given the new version of a tuple after some update, verify whether the
1926-
* given Xmax (corresponding to the previous version) matches the tuple's
1927-
* Xmin, taking into account that the Xmin might have been frozen after the
1928-
* update.
1929-
*/
1930-
bool
1931-
HeapTupleUpdateXmaxMatchesXmin(TransactionIdxmax,HeapTupleHeaderhtup)
1932-
{
1933-
TransactionIdxmin=HeapTupleHeaderGetXmin(htup);
1934-
1935-
/*
1936-
* If the xmax of the old tuple is identical to the xmin of the new one,
1937-
* it's a match.
1938-
*/
1939-
if (TransactionIdEquals(xmax,xmin))
1940-
return true;
1941-
1942-
/*
1943-
* When a tuple is frozen, the original Xmin is lost, but we know it's a
1944-
* committed transaction. So unless the Xmax is InvalidXid, we don't know
1945-
* for certain that there is a match, but there may be one; and we must
1946-
* return true so that a HOT chain that is half-frozen can be walked
1947-
* correctly.
1948-
*/
1949-
if (TransactionIdEquals(xmin,FrozenTransactionId)&&
1950-
TransactionIdIsValid(xmax))
1951-
return true;
1952-
1953-
return false;
1954-
}
19551923

19561924
/*
19571925
* UpdateXmaxHintBits - update tuple hint bits after xmax transaction ends
@@ -5077,7 +5045,8 @@ heap_lock_updated_tuple_rec(Relation rel, ItemPointer tid, TransactionId xid,
50775045
* the end of the chain, we're done, so return success.
50785046
*/
50795047
if (TransactionIdIsValid(priorXmax)&&
5080-
!HeapTupleUpdateXmaxMatchesXmin(priorXmax,mytup.t_data))
5048+
!TransactionIdEquals(HeapTupleHeaderGetXmin(mytup.t_data),
5049+
priorXmax))
50815050
{
50825051
UnlockReleaseBuffer(buf);
50835052
returnHeapTupleMayBeUpdated;
@@ -5520,26 +5489,14 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
55205489
Assert(TransactionIdIsValid(xid));
55215490

55225491
/*
5523-
* The updating transaction cannot possibly be still running, but
5524-
* verify whether it has committed, and request to set the
5525-
* COMMITTED flag if so. (We normally don't see any tuples in
5526-
* this state, because they are removed by page pruning before we
5527-
* try to freeze the page; but this can happen if the updating
5528-
* transaction commits after the page is pruned but before
5529-
* HeapTupleSatisfiesVacuum).
5492+
* If the xid is older than the cutoff, it has to have aborted,
5493+
* otherwise the tuple would have gotten pruned away.
55305494
*/
55315495
if (TransactionIdPrecedes(xid,cutoff_xid))
55325496
{
5533-
if (TransactionIdDidCommit(xid))
5534-
{
5535-
xid=FrozenTransactionId;
5536-
*flags=FRM_MARK_COMMITTED |FRM_RETURN_IS_XID;
5537-
}
5538-
else
5539-
{
5540-
*flags |=FRM_INVALIDATE_XMAX;
5541-
xid=InvalidTransactionId;/* not strictly necessary */
5542-
}
5497+
Assert(!TransactionIdDidCommit(xid));
5498+
*flags |=FRM_INVALIDATE_XMAX;
5499+
xid=InvalidTransactionId;/* not strictly necessary */
55435500
}
55445501
else
55455502
{
@@ -5610,51 +5567,65 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
56105567

56115568
/*
56125569
* It's an update; should we keep it? If the transaction is known
5613-
* aborted or crashed then it's okay to ignore it, otherwise not.
5570+
* aborted then it's okay to ignore it, otherwise not. However,
5571+
* if the Xid is older than the cutoff_xid, we must remove it.
5572+
* Note that such an old updater cannot possibly be committed,
5573+
* because HeapTupleSatisfiesVacuum would have returned
5574+
* HEAPTUPLE_DEAD and we would not be trying to freeze the tuple.
5575+
*
5576+
* Note the TransactionIdDidAbort() test is just an optimization
5577+
* and not strictly necessary for correctness.
56145578
*
56155579
* As with all tuple visibility routines, it's critical to test
5616-
* TransactionIdIsInProgress beforeTransactionIdDidCommit,
5580+
* TransactionIdIsInProgress beforethe transam.c routines,
56175581
* because of race conditions explained in detail in tqual.c.
5618-
*
5619-
* We normally don't see committed updating transactions earlier
5620-
* than the cutoff xid, because they are removed by page pruning
5621-
* before we try to freeze the page; but it can happen if the
5622-
* updating transaction commits after the page is pruned but
5623-
* before HeapTupleSatisfiesVacuum.
56245582
*/
56255583
if (TransactionIdIsCurrentTransactionId(xid)||
56265584
TransactionIdIsInProgress(xid))
56275585
{
56285586
Assert(!TransactionIdIsValid(update_xid));
56295587
update_xid=xid;
56305588
}
5631-
elseif (TransactionIdDidCommit(xid))
5589+
elseif (!TransactionIdDidAbort(xid))
56325590
{
56335591
/*
5634-
*The transaction committed, so we cantell caller to set
5635-
*HEAP_XMAX_COMMITTED. (We can only do this because we know
5636-
* the transaction is not running.)
5592+
*Test whether totell caller to set HEAP_XMAX_COMMITTED
5593+
*while we have the Xid still in cache. Note this can only
5594+
*be done ifthe transaction isknownnot running.
56375595
*/
5596+
if (TransactionIdDidCommit(xid))
5597+
update_committed= true;
56385598
Assert(!TransactionIdIsValid(update_xid));
5639-
update_committed= true;
56405599
update_xid=xid;
56415600
}
56425601

5643-
/*
5644-
* Not in progress, not committed -- must be aborted or crashed;
5645-
* we can ignore it.
5646-
*/
5647-
56485602
/*
56495603
* If we determined that it's an Xid corresponding to an update
56505604
* that must be retained, additionally add it to the list of
5651-
* members of the newMulti, in case we end up using that. (We
5605+
* members of the newMultis, in case we end up using that. (We
56525606
* might still decide to use only an update Xid and not a multi,
56535607
* but it's easier to maintain the list as we walk the old members
56545608
* list.)
5609+
*
5610+
* It is possible to end up with a very old updater Xid that
5611+
* crashed and thus did not mark itself as aborted in pg_clog.
5612+
* That would manifest as a pre-cutoff Xid. Make sure to ignore
5613+
* it.
56555614
*/
56565615
if (TransactionIdIsValid(update_xid))
5657-
newmembers[nnewmembers++]=members[i];
5616+
{
5617+
if (!TransactionIdPrecedes(update_xid,cutoff_xid))
5618+
{
5619+
newmembers[nnewmembers++]=members[i];
5620+
}
5621+
else
5622+
{
5623+
/* cannot have committed: would be HEAPTUPLE_DEAD */
5624+
Assert(!TransactionIdDidCommit(update_xid));
5625+
update_xid=InvalidTransactionId;
5626+
update_committed= false;
5627+
}
5628+
}
56585629
}
56595630
else
56605631
{
@@ -5721,10 +5692,7 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
57215692
*
57225693
* It is assumed that the caller has checked the tuple with
57235694
* HeapTupleSatisfiesVacuum() and determined that it is not HEAPTUPLE_DEAD
5724-
* (else we should be removing the tuple, not freezing it). However, note
5725-
* that we don't remove HOT tuples even if they are dead, and it'd be incorrect
5726-
* to freeze them (because that would make them visible), so we mark them as
5727-
* update-committed, and needing further freezing later on.
5695+
* (else we should be removing the tuple, not freezing it).
57285696
*
57295697
* NB: cutoff_xid *must* be <= the current global xmin, to ensure that any
57305698
* XID older than it could neither be running nor seen as running by any
@@ -5835,18 +5803,7 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple, TransactionId cutoff_xid,
58355803
elseif (TransactionIdIsNormal(xid)&&
58365804
TransactionIdPrecedes(xid,cutoff_xid))
58375805
{
5838-
/*
5839-
* Must freeze regular XIDs older than the cutoff. We must not freeze
5840-
* a HOT-updated tuple, though; doing so would bring it back to life.
5841-
*/
5842-
if (HeapTupleHeaderIsHotUpdated(tuple))
5843-
{
5844-
frz->t_infomask |=HEAP_XMAX_COMMITTED;
5845-
changed= true;
5846-
/* must not freeze */
5847-
}
5848-
else
5849-
freeze_xmax= true;
5806+
freeze_xmax= true;
58505807
}
58515808

58525809
if (freeze_xmax)

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -435,7 +435,7 @@ heap_prune_chain(Relation relation, Buffer buffer, OffsetNumber rootoffnum,
435435
* Check the tuple XMIN against prior XMAX, if any
436436
*/
437437
if (TransactionIdIsValid(priorXmax)&&
438-
!HeapTupleUpdateXmaxMatchesXmin(priorXmax,htup))
438+
!TransactionIdEquals(HeapTupleHeaderGetXmin(htup),priorXmax))
439439
break;
440440

441441
/*
@@ -774,7 +774,7 @@ heap_get_root_tuples(Page page, OffsetNumber *root_offsets)
774774
htup= (HeapTupleHeader)PageGetItem(page,lp);
775775

776776
if (TransactionIdIsValid(priorXmax)&&
777-
!HeapTupleUpdateXmaxMatchesXmin(priorXmax,htup))
777+
!TransactionIdEquals(priorXmax,HeapTupleHeaderGetXmin(htup)))
778778
break;
779779

780780
/* Remember the root line pointer for this item */

‎src/backend/commands/vacuumlazy.c

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1678,15 +1678,15 @@ lazy_record_dead_tuple(LVRelStats *vacrelstats,
16781678
ItemPointeritemptr)
16791679
{
16801680
/*
1681-
* The arraymust neveroverflow, since we rely on all deletable tuples
1682-
*being removed; inability to remove atuple might cause an old XID to
1683-
*persist beyondthefreeze limit, which could be disastrous later on.
1681+
* The arrayshouldn'toverflow under normal behavior, but perhaps it
1682+
*could if we are given areally small maintenance_work_mem. In that
1683+
*case, just forgetthelast few tuples (we'll get 'em next time).
16841684
*/
1685-
if (vacrelstats->num_dead_tuples>=vacrelstats->max_dead_tuples)
1686-
elog(ERROR,"dead tuple array overflow");
1687-
1688-
vacrelstats->dead_tuples[vacrelstats->num_dead_tuples]=*itemptr;
1689-
vacrelstats->num_dead_tuples++;
1685+
if (vacrelstats->num_dead_tuples<vacrelstats->max_dead_tuples)
1686+
{
1687+
vacrelstats->dead_tuples[vacrelstats->num_dead_tuples]=*itemptr;
1688+
vacrelstats->num_dead_tuples++;
1689+
}
16901690
}
16911691

16921692
/*

‎src/backend/executor/execMain.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2019,7 +2019,8 @@ EvalPlanQualFetch(EState *estate, Relation relation, int lockmode,
20192019
* buffer's content lock, since xmin never changes in an existing
20202020
* tuple.)
20212021
*/
2022-
if (!HeapTupleUpdateXmaxMatchesXmin(priorXmax,tuple.t_data))
2022+
if (!TransactionIdEquals(HeapTupleHeaderGetXmin(tuple.t_data),
2023+
priorXmax))
20232024
{
20242025
ReleaseBuffer(buffer);
20252026
returnNULL;
@@ -2137,7 +2138,8 @@ EvalPlanQualFetch(EState *estate, Relation relation, int lockmode,
21372138
/*
21382139
* As above, if xmin isn't what we're expecting, do nothing.
21392140
*/
2140-
if (!HeapTupleUpdateXmaxMatchesXmin(priorXmax,tuple.t_data))
2141+
if (!TransactionIdEquals(HeapTupleHeaderGetXmin(tuple.t_data),
2142+
priorXmax))
21412143
{
21422144
ReleaseBuffer(buffer);
21432145
returnNULL;

‎src/include/access/heapam.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -127,9 +127,6 @@ extern void heap_get_latest_tid(Relation relation, Snapshot snapshot,
127127
ItemPointertid);
128128
externvoidsetLastTid(constItemPointertid);
129129

130-
externboolHeapTupleUpdateXmaxMatchesXmin(TransactionIdxmax,
131-
HeapTupleHeaderhtup);
132-
133130
externBulkInsertStateGetBulkInsertState(void);
134131
externvoidFreeBulkInsertState(BulkInsertState);
135132

‎src/test/isolation/isolation_schedule

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,5 @@ test: lock-committed-keyupdate
3030
test: update-locked-tuple
3131
test: propagate-lock-delete
3232
test: tuplelock-conflict
33-
test: freeze-the-dead
3433
test: drop-index-concurrently-1
3534
test: timeouts

0 commit comments

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