Re: pgsql: Avoid improbable PANIC during heap_update.

From: Jaime Casanova <jcasanov(at)systemguards(dot)com(dot)ec>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-committers(at)lists(dot)postgresql(dot)org
Subject: Re: pgsql: Avoid improbable PANIC during heap_update.
Date: 2022-09-30 20:44:22
Message-ID: YzdVJnQocQ6RHKxk@ahch-to
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

On Thu, Sep 29, 2022 at 02:55:40AM -0500, Jaime Casanova wrote:
> On Tue, Apr 13, 2021 at 04:17:39PM +0000, Tom Lane wrote:
> > Avoid improbable PANIC during heap_update.
> >
> > heap_update needs to clear any existing "all visible" flag on
> > the old tuple's page (and on the new page too, if different).
> > Per coding rules, to do this it must acquire pin on the appropriate
> > visibility-map page while not holding exclusive buffer lock;
> > which creates a race condition since someone else could set the
> > flag whenever we're not holding the buffer lock. The code is
> > supposed to handle that by re-checking the flag after acquiring
> > buffer lock and retrying if it became set. However, one code
> > path through heap_update itself, as well as one in its subroutine
> > RelationGetBufferForTuple, failed to do this. The end result,
> > in the unlikely event that a concurrent VACUUM did set the flag
> > while we're transiently not holding lock, is a non-recurring
> > "PANIC: wrong buffer passed to visibilitymap_clear" failure.
> >
>
> Hi,
>
> This doesn't look as improbable because I saw it at least 3 times with
> v15beta4.
>
> The first time I thought it was my fault, then I tried with a commit on
> september 25 (didn't remember which exactly but that doesn't seems too
> relevant).
> Finally I saw it again in a build with TRACE_VISIBILITYMAP defined (the
> same commit).
>
> But I haven't see it anymore on rc1. Anyway I'm attaching the backtrace
> (this is from the build with TRACE_VISIBILITYMAP), the query that was
> running at the time was (no changes were made to quad_poly_tbl table
> nor any indexes were added to this table):
>
> """
> update public.quad_poly_tbl set
> id = public.quad_poly_tbl.id
> returning
> public.quad_poly_tbl.id as c0
> """
>

Just to confirm I saw this on RC1

--
Jaime Casanova
Director de Servicios Profesionales
SystemGuards - Consultores de PostgreSQL

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Tom Lane 2022-09-30 20:51:20 Re: pgsql: Avoid improbable PANIC during heap_update.
Previous Message Andres Freund 2022-09-30 17:50:41 pgsql: mingw: Define PGDLLEXPORT as __declspec (dllexport) as done for

Browse pgsql-hackers by date

  From Date Subject
Next Message Jacob Champion 2022-09-30 20:45:29 Re: [PoC] Federated Authn/z with OAUTHBEARER
Previous Message Nathan Bossart 2022-09-30 20:41:47 Re: Suppressing useless wakeups in walreceiver