On Tue, Apr 16, 2019 at 12:44 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > So after thinking about this a bit more ... > > ISTM that what we have here is a race condition (ie, tuple changed state > since heap_page_prune), and that ideally we want the code to resolve it > as if no race had happened. That is, either of these behaviors would > be acceptable: > > 1. Delete the tuple, just as heap_page_prune would've done if it had seen > it DEAD. (Possibly we could implement that by jumping back and doing > heap_page_prune again, but that seems pretty messy and bug-prone. > In any case, if we're not doing index cleanup then this must reduce to > "replace tuple by a dead line pointer", not remove it altogether.) > > 2. Act as if the tuple were still live, just as would've happened if the > state didn't change till just after we looked instead of just before. > > Option #2 is a lot simpler and safer, and can be implemented as I > suggested earlier, assuming we're all good with the assumption that > heap_prepare_freeze_tuple isn't going to do anything bad.
After studying this more carefully, I agree. You and Andres and Alvaro are all correct, and I'm plain wrong. Thanks for explaining. I have committed a patch that changes the logic as per your suggestion, and also removes nleft_dead_tuples and nleft_dead_itemids. I'll reply separately to your other point about tups_vacuumed reporting. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
