On Thu, Jul 23, 2020 at 2:10 PM Andres Freund <and...@anarazel.de> wrote: > In the case the HOT logic triggers, we'll call > heap_prepare_freeze_tuple() even when the tuple is dead.
I think this is very bad. I've always been confused about these comments, but I couldn't quite put my finger on the problem. Now I think I can: the comments here imagine that we have the option either to set tupgone, causing the line pointer to be marked unused by an eventual call to lazy_vacuum_page(), or that we can decline to set tupgone, which will leave the tuple around to be handled by the next vacuum. However, we don't really have a choice at all. A choice implies that either option is correct, and therefore we can elect the one we prefer. But here, it's not just that one option is incorrect, but that both options are incorrect. Setting tupgone controls whether or not the tuple is considered for freezing. If we DON'T consider freezing it, then we might manage to advance relfrozenxid while an older XID still exists in the table. If we DO consider freezing it, we will correctly conclude that it needs to be frozen, but then the freezing code is in an impossible situation, because it has no provision for getting rid of tuples, only for keeping them. Its logic assumes that whenever we are freezing xmin or xmax we do that in a way that causes the tuple to be visible to everyone, but this tuple should be visible to no one. So with your changes it now errors out instead of corrupting data, but that's just replacing one bad thing (data corruption) with another (VACUUM failures). I think the actual correct behavior here is to mark the line pointer as dead. The easiest way to accomplish that is probably to have lazy_scan_heap() just emit an extra XLOG_HEAP2_CLEAN record beyond whatever HOT-pruning already did, if it's necessary. A better solution would probably be to merge HOT-pruning with setting things all-visible and have a single function that does both, but that seems a lot more invasive, and definitely unsuitable for back-patching. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
