On 2013-11-29 16:30:08 -0300, Alvaro Herrera wrote:
> As a second bug, heap_freeze_tuple() didn't properly handle multixacts
> that need to be frozen according to cutoff_multi, but whose updater xid
> is still alive. Instead of preserving the update Xid, it just set Xmax
> invalid, which leads to both old and new tuple versions becoming
> visible. This is pretty rare in practice, but a real threat
> nonetheless. Existing corrupted rows, unfortunately, cannot be repaired
> in an automated fashion.
I think this bug is worth mentioning here explicitly. As released, if
you have a table where some rows are updated using an xmax as multi, and
you freeze it you're pretty likely to experience corruption where you
see both the old and the new version of a tuple as live. I haven't seen
this one in the wild but just noticed it while looking at the other
freezing bug, but it's really quite easy to reproduce. As demonstrated
in the attached isolationtester spec which doubles the row count via an
UPDATE in 9.3/HEAD.
> + * Note the update Xid cannot possibly be older than
> + * cutoff_xid; if it were, we wouldn't be here: if committed,
> + * the tuple would have been pruned, and if aborted, the Xmax
> + * would have been marked Invalid by HeapTupleSatisfiesVacuum.
> + * (Not in-progress either, because then cutoff_xid would be
> + * newer.)
s/newer/older/?
> @@ -5644,24 +5729,54 @@ heap_tuple_needs_freeze(HeapTupleHeader tuple, TransactionId cutoff_xid,
> TransactionIdPrecedes(xid, cutoff_xid))
> return true;
Maybe add a comment referring to heap_freeze_tuple() for justification
of individual parts and that it needs to be kept in sync?
> + nmembers = GetMultiXactIdMembers(xid, &members, true);
> + for (i = 0; i < nmembers; i++)
> + {
> + TransactionId member = members[i].xid;
> +
> + Assert(TransactionIdIsNormal(member));
> +
> + /* we don't care about lockers */
> + if (!ISUPDATE_from_mxstatus(members[i].status))
> + continue;
> +
> + if (TransactionIdPrecedes(member, cutoff_xid))
> + {
> + pfree(members);
> + return true;
> + }
> + }
This now can use GetUpdateXid() as well.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services