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

Andres Freund <andres@anarazel.de>

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-22T17:59:14Z
Lists: pgsql-bugs

Attachments

Hi,

On 2021-11-16 16:22:06 -0800, Peter Geoghegan wrote:
> 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.

I'm not quite sure what you referring to with "convincing me"? Whether to go
for something comprehensive or more minimal? I'm kind of agnostic on what the
right approach is.


> Why don't you produce a minimal fix for backpatch? I'll review that,
> just as you reviewed my patch.

Here's a draft of that change.


I think it's worth to clean up the regression test I wrote and use it
regardless of which version of the fix we end up choosing. But I'm a bit bit
on the fence - it's quite complicated.

OTOH, I think with the framework in place it'd not be too hard to write a few
more tests for odd pruning scenarios...

If we were to, it'd probably best to to comment out the mon_* stuff, as that
is what requires the amcheck / pageinspect extensions, which we'd likely not
want to depend on. But it's pretty crucial for development.

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,