Horton

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

Andres Freund <andres@anarazel.de>

View thread View on postgr.es
From: Andres Freund <andres@anarazel.de>
To: Peter Geoghegan <pg@bowt.ie>
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-11T00:47:27Z
Lists: pgsql-bugs 
Hi,

On 2021-11-10 13:04:43 -0800, Peter Geoghegan wrote:
> On Wed, Nov 10, 2021 at 11:20 AM Andres Freund <andres@anarazel.de> wrote:
> > The way this definitely breaks - I have been able to reproduce this in
> > isolation - is when one tuple is processed twice by heap_prune_chain(), and
> > the result of HeapTupleSatisfiesVacuum() changes from
> > HEAPTUPLE_DELETE_IN_PROGRESS to DEAD.
> 
> I had no idea that that was now possible. I really think that this
> ought to be documented centrally.

Where would you suggest?


> The relevant code in pruneheap.c was always incredibly fragile -- no
> question. Even still, there is really no good reason to believe that
> that was actually a problem before commit dc7420c2. Even if we assume
> that there's a problem before 14, the surface area is vastly smaller
> than on 14 -- the relevant pruneheap.c code hasn't really ever changed
> since HOT went in. And so I think that the most sensible course of
> action here is this: commit a fix to Postgres 14 + HEAD only -- no
> backpatch to earlier versions.

Yea. The fact that I also saw *one* error in 13 worries me a bit, but perhaps
that was something else. Even if we eventually need to backpatch something
further, having it in 14/master first is good.

The fact that 13 didn't trigger the problem reliably doesn't necessarily much
- it's a pretty limited workload. There e.g. are no aborts.


I think we might be able to do something a bit more limited than what you
propose. But I'm not sure it's worth going for that.

Greetings,

Andres Freund

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,