Horton

Re: BUG #17255: Server crashes in index_delete_sort_cmp() due to race condition with vacuum

Peter Geoghegan <pg@bowt.ie>

View thread View on postgr.es
From: Peter Geoghegan <pg@bowt.ie>
To: Andres Freund <andres@anarazel.de>
Cc: Dmitry Dolgov <9erthalion6@gmail.com>, Alexander Lakhin <exclusion@gmail.com>, Matthias van de Meent <boekewurm+postgres@gmail.com>, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Date: 2021-11-17T00:22:06Z
Lists: pgsql-bugs 
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

Commits

  1. vacuumlazy.c: Standardize rel_pages terminology.

  2. vacuumlazy.c: document vistest and OldestXmin.

  3. heap pruning: Only call BufferGetBlockNumber() once.

  4. Fix possible HOT corruption when RECENTLY_DEAD changes to DEAD while pruning.

  5. Assert redirect pointers are sensible after heap_page_prune().

  6. Remove tupgone special case from vacuumlazy.c.

  7. Recycle nbtree pages deleted during same VACUUM.

  8. snapshot scalability: Don't compute global horizons while building snapshots.

  9. Refactor heap_page_prune so that instead of changing item states on-the-fly,