Re: [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple

Michael Paquier <michael.paquier@gmail.com>

From: Michael Paquier <michael.paquier@gmail.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Peter Geoghegan <pg@bowt.ie>, "Wong, Yi Wen" <yiwong@amazon.com>, "Wood, Dan" <hexpert@amazon.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2017-10-06T13:28:03Z
Lists: pgsql-hackers
On Fri, Oct 6, 2017 at 7:57 PM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> Michael Paquier wrote:
>> On Fri, Oct 6, 2017 at 1:24 AM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>> +/*
>> + * Given a tuple, verify whether the given Xmax matches the tuple's Xmin,
>> + * taking into account that the Xmin might have been frozen.
>> + */
>> [...]
>> +   /*
>> +    * We actually don't know if there's a match, but if the previous tuple
>> +    * was frozen, we cannot really rely on a perfect match.
>> +    */
>
> I don't know what you had in mind here,

Impossible to know if I don't actually send the contents :)

> but I tweaked the 9.3 version so that it now looks like this:

I wanted to mention that the comments could be reworked. And forgot to
suggest some.

> /*
>  * HeapTupleUpdateXmaxMatchesXmin - verify update chain xmax/xmin lineage
>  *
>  * Given the new version of a tuple after some update, verify whether the
>  * given Xmax (corresponding to the previous version) matches the tuple's
>  * Xmin, taking into account that the Xmin might have been frozen after the
>  * update.
>  */
> bool
> HeapTupleUpdateXmaxMatchesXmin(TransactionId xmax, HeapTupleHeader htup)
> {
>         TransactionId   xmin = HeapTupleHeaderGetXmin(htup);
>
>         /*
>          * If the xmax of the old tuple is identical to the xmin of the new one,
>          * it's a match.
>          */
>         if (TransactionIdEquals(xmax, xmin))
>                 return true;
>
>         /*
>          * When a tuple is frozen, the original Xmin is lost, but we know it's a
>          * committed transaction.  So unless the Xmax is InvalidXid, we don't
>          * know for certain that there is a match, but there may be one; and we
>          * must return true so that a HOT chain that is half-frozen can be walked
>          * correctly.
>          */
>         if (TransactionIdEquals(xmin, FrozenTransactionId) &&
>                 TransactionIdIsValid(xmax))
>                 return true;
>
>         return false;
> }

Those are clearly improvements.
-- 
Michael


Commits

  1. Backport addition of rs_old_rel to rewriteheap's state.

  2. Perform a lot more sanity checks when freezing tuples.

  3. Revert bogus fixes of HOT-freezing bug

  4. Fix traversal of half-frozen update chains

  5. Fix freezing of a dead HOT-updated tuple

  6. During index build, check and elog (not just Assert) for broken HOT chain.

  7. Fix WAL replay of locking an updated tuple

  8. Change the way we mark tuples as frozen.