Horton

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

Peter Geoghegan <pg@bowt.ie>

View thread View on postgr.es
From: Peter Geoghegan <pg@bowt.ie>
To: Andres Freund <andres@anarazel.de>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, "Wood, Dan" <hexpert@amazon.com>, pgsql-committers <pgsql-committers@postgresql.org>, "Wong, Yi Wen" <yiwong@amazon.com>, Michael Paquier <michael.paquier@gmail.com>, Robert Haas <robertmhaas@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2017-11-10T00:02:17Z
Lists: pgsql-hackers 
On Thu, Nov 9, 2017 at 2:24 PM, Andres Freund <andres@anarazel.de> wrote:
> Attached is a version of the already existing regression test that both
> reproduces the broken hot chain (and thus failing index lookups) and
> then also the tuple reviving.  I don't see any need for letting this run
> with arbitrary permutations.

I thought that the use of every possible permutation was excessive,
myself. It left us with an isolation test that didn't precisely
describe the behavior that is tested. What you came up with seems far,
far better, especially because of the comments you included. The mail
message-id references seem to add a lot, too.

> What I'm currently wondering about is how much we need to harden
> postgres against such existing corruption. If e.g. the hot chains are
> broken somebody might have reindexed thinking the problem is fixed - but
> if they then later vacuum everything goes to shit again, with dead rows
> reappearing.

I don't follow you here. Why would REINDEXing make the rows that
should be dead disappear again, even for a short period of time? It
might do so for index scans, I suppose, but not for sequential scans.
Are you concerned about a risk of somebody not noticing that
sequential scans are still broken?

Actually, on second thought, I take that back -- I don't think that
REINDEXing will even finish once a HOT chain is broken by the bug.
IndexBuildHeapScan() actually does quite a good job of making sure
that HOT chains are sane, which is how the enhanced amcheck notices
the bug here in practice. (Before this bug was discovered, I would
have expected amcheck to catch problems like it slightly later, during
the Bloom filter probe for that HOT chain...but, in fact, it never
gets there with corruption from this bug in practice, AFAIK.)

-- 
Peter Geoghegan

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.