On Tue, Apr 16, 2019 at 12:44 PM Tom Lane <tgl@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