On Mon, Nov 15, 2021 at 6:39 PM Andres Freund <andres@anarazel.de> wrote:
> > What prevents the scenario that some other backend e.g. has a snapshot with
> > xmin=xmax=RECENTLY_DEAD-row. If the RECENTLY_DEAD row has an xid that is later
> > than the DEAD row, this afaict would make it perfectly legal to prune the DEAD
> > row, but *not* the RECENTLY_DEAD one.
>
> I think the code is actually correct if awfully commented. I.e. the above
> scenario cannot happen in a harmful situation. The relevant piece is that the
> set of "valid" snapshots is limited (for a specific cluster history).
> IOW, there *can* be a snapshot with xmin=xmax=RECENTLY_DEAD-row, but it'd not
> see the RECENTLY_DEAD row anyway.
I'm not surprised that this turned out to be correct. It hasn't made
me change my mind; I still believe that the best way forward is to
backpatch a more comprehensive fix, like my patch. Again, I just think
that that approach has the best chance of avoiding any further
problems.
The alternative proposal that you lean towards (minimal fix on 14,
commit the comprehensive fix to HEAD only) seems fine overall, though.
In any case I don't think that there is much point in further debating
it. If I was going to convince you, it would have happened by now. It
is your bug (kind of), and so I defer to you on this.
Why don't you produce a minimal fix for backpatch? I'll review that,
just as you reviewed my patch.
--
Peter Geoghegan