Re: [HACKERS] [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple
Alvaro Herrera <alvherre@2ndquadrant.com>
From: Alvaro Herrera <alvherre@2ndQuadrant.com>
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-06T16:21:15Z
Lists: pgsql-hackers
Attachments
- tweaks.patch (text/plain) patch
I think you've done a stellar job of identifying what the actual problem was. I like the new (simpler) coding of that portion of HeapTupleSatisfiesVacuum. freeze-the-dead is not listed in isolation_schedule; an easy fix. I confirm that the test crashes with an assertion failure without the code fix, and that it doesn't with it. I think the comparison to OldestXmin should be reversed: if (!TransactionIdPrecedes(xmax, OldestXmin)) return HEAPTUPLE_RECENTLY_DEAD; return HEAPTUPLE_DEAD; This way, an xmax that has exactly the OldestXmin value will return RECENTLY_DEAD rather DEAD, which seems reasonable to me (since OldestXmin value itself is supposed to be still possibly visible to somebody). Also, this way it is consistent with the other comparison to OldestXmin at the bottom of the function. There is no reason for the "else" or the extra braces. Put together, I propose the attached delta for 0001. Your commit message does a poor job of acknowledging prior work on diagnosing the problem starting from Dan's initial test case and patch. I haven't looked at your 0002 yet. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Commits
-
Backport addition of rs_old_rel to rewriteheap's state.
- 152a56905658 9.3.21 landed
-
Perform a lot more sanity checks when freezing tuples.
- ed8e1aff6ace 9.4.16 landed
- d3044f8b0732 10.2 landed
- 986a9153b970 9.6.7 landed
- 94d1c88103ff 9.5.11 landed
- 4800f16a7ad0 9.3.21 landed
- 699bf7d05c68 11.0 landed
-
Revert bogus fixes of HOT-freezing bug
- c6764eb3aea6 11.0 landed
- f05ae2fa94b4 9.3.20 landed
- ef0339ee5dcf 9.4.15 landed
- b3888b60d3f0 9.5.10 landed
- 7a95966bc03c 10.1 landed
- 08ba67d596a1 9.6.6 landed
-
Fix traversal of half-frozen update chains
- a5736bf754c8 11.0 landed
- fc0df3bdafd6 9.5.10 landed
- d441cff14249 9.6.6 landed
- b052d524ca71 9.3.20 landed
- 8b6d85f2dc1e 9.4.15 landed
- 22576734b805 10.1 landed
-
Fix freezing of a dead HOT-updated tuple
- 46c35116ae1a 10.0 cited
- 20b655224249 11.0 cited
-
During index build, check and elog (not just Assert) for broken HOT chain.
- d70cf811f7dd 9.4.0 cited
-
Fix WAL replay of locking an updated tuple
- 6bfa88acd3df 9.4.0 cited
-
Change the way we mark tuples as frozen.
- 37484ad2aace 9.4.0 cited