Horton

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

Alvaro Herrera <alvherre@alvh.no-ip.org>

View thread View on postgr.es
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Andres Freund <andres@anarazel.de>
Cc: "Wood, Dan" <hexpert@amazon.com>, Peter Geoghegan <pg@bowt.ie>, 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>, pgsql-hackers@postgresql.org
Date: 2017-12-06T20:23:55Z
Lists: pgsql-hackers

Attachments

Andres Freund wrote:

> I've played around quite some with the attached patch. So far, after
> applying the second patch, neither VACUUM nor VACUUM FULL / CLUSTER make
> the situation worse for already existing corruption. HOT pruning can
> change the exact appearance of existing corruption a bit, but I don't
> think it can make the corruption meaningfully worse.  It's a bit
> annoying and scary to add so many checks to backbranches but it kinda
> seems required.  The error message texts aren't perfect, but these are
> "should never be hit" type elog()s so I'm not too worried about that.

Looking at 0002: I agree with the stuff being done here.  I think a
couple of these checks could be moved one block outerwards in term of
scope; I don't see any reason why the check should not apply in that
case.  I didn't catch any place missing additional checks.

Despite these being "shouldn't happen" conditions, I think we should
turn these up all the way to ereports with an errcode and all, and also
report the XIDs being complained about.  No translation required,
though.  Other than those changes and minor copy editing a commit
(attached), 0002 looks good to me.

I started thinking it'd be good to report block number whenever anything
happened while scanning the relation.  The best way to go about this
seems to be to add an errcontext callback to lazy_scan_heap, so I attach
a WIP untested patch to add that.  (I'm not proposing this for
back-patch for now, mostly because I don't have the time/energy to push
for it right now.)

It appears that you got all the places that seem to reasonably need
additional checks.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

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.