database/postgres
database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-05-09 18:21:05 +03:00
Code

Don't mark pages all-visible spuriously

Browse Source 
Dan Wood diagnosed a long-standing problem that pages containing tuples
that are locked by multixacts containing live lockers may spuriously end
up as candidates for getting their all-visible flag set.  This has the
long-term effect that multixacts remain unfrozen; this may previously
pass undetected, but since commit XYZ it would be reported as
  "ERROR: found multixact 134100944 from before relminmxid 192042633"
because when a later vacuum tries to freeze the page it detects that a
multixact that should have gotten frozen, wasn't.

Dan proposed a (correct) patch that simply sets a variable to its
correct value, after a bogus initialization.  But, per discussion, it
seems better coding to avoid the bogus initializations altogether, since
they could give rise to more bugs later.  Therefore this fix rewrites
the logic a little bit to avoid depending on the bogus initializations.

This bug was part of a family introduced in 9.6 by commit a892234f830e;
later, commit 38e9f90a227d fixed most of them, but this one was
unnoticed.

Authors: Dan Wood, Pavan Deolasee, Álvaro Herrera
Reviewed-by: Masahiko Sawada, Pavan Deolasee, Álvaro Herrera
Discussion: https://postgr.es/m/84EBAC55-F06D-4FBE-A3F3-8BDA093CE3E3@amazon.com
This commit is contained in:
Alvaro Herrera 2018-05-04 15:24:44 -03:00
parent a9fbf550b1
commit 3a11485a52
1 changed files with 25 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
src/backend/access/heap/heapam.c
View File

@ -6676,9 +6676,10 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple,
xl_heap_freeze_tuple *frz, bool *totally_frozen_p) xl_heap_freeze_tuple *frz, bool *totally_frozen_p)
{ {
bool changed = false; bool changed = false;
bool freeze_xmax = false; bool xmax_already_frozen = false;
bool xmin_frozen;
bool freeze_xmax;
TransactionId xid; TransactionId xid;
bool totally_frozen = true;
frz->frzflags = 0; frz->frzflags = 0;
frz->t_infomask2 = tuple->t_infomask2; frz->t_infomask2 = tuple->t_infomask2;
@ -6687,6 +6688,8 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple,
/* Process xmin */ /* Process xmin */
xid = HeapTupleHeaderGetXmin(tuple); xid = HeapTupleHeaderGetXmin(tuple);
xmin_frozen = ((xid == FrozenTransactionId) ||
HeapTupleHeaderXminFrozen(tuple));
if (TransactionIdIsNormal(xid)) if (TransactionIdIsNormal(xid))
{ {
if (TransactionIdPrecedes(xid, relfrozenxid)) if (TransactionIdPrecedes(xid, relfrozenxid))
@ -6705,9 +6708,8 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple,
frz->t_infomask |= HEAP_XMIN_FROZEN; frz->t_infomask |= HEAP_XMIN_FROZEN;
changed = true; changed = true;
xmin_frozen = true;
} }
else
totally_frozen = false;
} }
/* /*
@ -6730,9 +6732,9 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple,
relfrozenxid, relminmxid, relfrozenxid, relminmxid,
cutoff_xid, cutoff_multi, &flags); cutoff_xid, cutoff_multi, &flags);
if (flags & FRM_INVALIDATE_XMAX) freeze_xmax = (flags & FRM_INVALIDATE_XMAX);
freeze_xmax = true;
else if (flags & FRM_RETURN_IS_XID) if (flags & FRM_RETURN_IS_XID)
{ {
/* /*
* NB -- some of these transformations are only valid because we * NB -- some of these transformations are only valid because we
@ -6746,7 +6748,6 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple,
if (flags & FRM_MARK_COMMITTED) if (flags & FRM_MARK_COMMITTED)
frz->t_infomask |= HEAP_XMAX_COMMITTED; frz->t_infomask |= HEAP_XMAX_COMMITTED;
changed = true; changed = true;
totally_frozen = false;
} }
else if (flags & FRM_RETURN_IS_MULTI) else if (flags & FRM_RETURN_IS_MULTI)
{ {
@ -6768,11 +6769,6 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple,
frz->xmax = newxmax; frz->xmax = newxmax;
changed = true; changed = true;
totally_frozen = false;
}
else
{
Assert(flags & FRM_NOOP);
} }
} }
else if (TransactionIdIsNormal(xid)) else if (TransactionIdIsNormal(xid))
@ -6800,11 +6796,24 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple,
freeze_xmax = true; freeze_xmax = true;
} }
else else
totally_frozen = false; freeze_xmax = false;
} }
else if ((tuple->t_infomask & HEAP_XMAX_INVALID) ||
!TransactionIdIsValid(HeapTupleHeaderGetRawXmax(tuple)))
{
freeze_xmax = false;
xmax_already_frozen = true;
}
else
ereport(ERROR,
(errcode(ERRCODE_DATA_CORRUPTED),
errmsg_internal("found xmax %u (infomask 0x%04x) not frozen, not multi, not normal",
xid, tuple->t_infomask)));
if (freeze_xmax) if (freeze_xmax)
{ {
Assert(!xmax_already_frozen);
frz->xmax = InvalidTransactionId; frz->xmax = InvalidTransactionId;
/* /*
@ -6857,7 +6866,8 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple,
} }
} }
*totally_frozen_p = totally_frozen; *totally_frozen_p = (xmin_frozen &&
(freeze_xmax || xmax_already_frozen));
return changed; return changed;
} }