Re: REPACK (CONCURRENTLY) backend waits indefinitely when decoding worker fails to start

From: Nikolay Samokhvalov <nik(at)postgres(dot)ai>
To: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
Cc: Antonin Houska <ah(at)cybertec(dot)at>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Nathan Bossart <nathandbossart(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, alvherre(at)kurilemu(dot)de
Subject: Re: REPACK (CONCURRENTLY) backend waits indefinitely when decoding worker fails to start
Date: 2026-09-13 18:37:49
Message-ID: CAM527d8cF-6azsRWOPaOUCmsdgMG=ene7-st91_NM7v0q3uLLg@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Sep 7, 2026 at 12:15 PM Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> After thinking more about this, I simplified the handling. There are two
> things the backend needs to wait for. First, for the worker to come up and
> attach to the error message queue. Second, for the worker to set up the
> logical decoding machinery before it waits for snapshot export. The first
> wait catches fork failures and worker startup issues. I used similar logic
> to what parallel.c uses for this. The second wait catches failures that
> happen after the worker starts up and attaches to the error queue but
> before it finishes setting up the logical decoding machinery. I kept the
> shared memory initialized flag with the CV wait as-is for this.

Álvaro pointed me here from my thread:
https://postgr.es/m/CAM527d9A7FBXJn7A3w2VAPq0Gojm7tnUDFMh1uwJitGfi2kNeQ@mail.gmail.com

My AI harness tested v4. Silent worker exits still hang three
post-attachment waits. Mihail raised snapshot/replay liveness in
December 2025.

Snapshot repro on a test build configured with --enable-injection-points,
with v4 plus only the attached patch's test support:

create extension injection_points;
create table t (i int primary key);
insert into t select generate_series(1, 100);
set statement_timeout = '2s';
select injection_points_attach(
'repack-worker-before-snapshot-export',
'injection_points', 'injection_exit', null);
repack (concurrently) t;

This times out after worker exit. The attached patch applies atop v4.

As far as I can see, the current uncommitted patches cover overall 5 cases:

- startup before queue attach: Bharath v4
- full error queue teardown: Bharath v4
- silent exit after attach, before initialization: attached patch
- silent exit before snapshot export: attached patch
- silent exit before replay export: attached patch

Nik

On Sun, Sep 13, 2026 at 11:09 AM Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Tue, Sep 8, 2026 at 12:17 AM Antonin Houska <ah(at)cybertec(dot)at> wrote:
> >
> > Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> >
> > > On Fri, Sep 4, 2026 at 5:11 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> > >
> > > After thinking more about this, I simplified the handling. There are two things the backend needs to wait for. First, for the worker to come up
> > > and attach to the error message queue. Second, for the worker to set up the logical decoding machinery before it waits for snapshot export.
> > > The first wait catches fork failures and worker startup issues. I used similar logic to what parallel.c uses for this. The second wait catches
> > > failures that happen after the worker starts up and attaches to the error queue but before it finishes setting up the logical decoding
> > > machinery. I kept the shared memory initialized flag with the CV wait as-is for this.
> > >
> > > Although the initialized flag wait may seem redundant with the snapshot export wait in get_initial_snapshot(), I would still keep it because it
> > > ensures the worker has fully set up the decoding before the backend proceeds.
> > >
> > > Dividing this into two separate waits (waiting for the worker to come up and attach to the error message queue, and then waiting for it to
> > > finish setup) makes the logic simpler, lets us reuse most of parallel.c's code, is easier to reason about, and fixes the hang issue without letting
> > > the backend reach the snapshot export wait with the worker not fully ready.
> >
> > Another reason for two separate waits is that two separate event types make
> > sense: WAIT_EVENT_BGWORKER_STARTUP and WAIT_EVENT_REPACK_WORKER_EXPORT.
>
> That's correct.
>
> > > Please find the attached v3 patch.
> >
> > Just two comments:
>
> Thanks for reviewing it.
>
> > * wait_for_repack_worker_to_attach() - as the decoding_worker variable is
> > declared static, this function does not necessarily need the argument.
>
> Removed the function parameter.
>
> > * Regarding comment: when the following is reached, the error message queue
> > has already been detached, so no implicit detaching should happen. Also,
> > there are no "other shared memory queues".
> >
> > + /*
> > + * If we have allocated a shared memory segment, detach it. This will
> > + * implicitly detach the error message queue, and any other shared memory
> > + * queues, stored there.
> > + */
> > + if (decoding_worker->seg != NULL)
> > + {
> > + dsm_detach(decoding_worker->seg);
> > + decoding_worker->seg = NULL;
> > + }
>
> I borrowed it from parallel.c. I agree it can be simplified, and I
> have done that.
>
> Please find the attached v4 patch.
>
> --
> Bharath Rupireddy
> Amazon Web Services: https://aws.amazon.com

Attachment Content-Type Size
0001-repack-worker-post-attachment-exit-minimal.patch application/x-patch 16.3 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Nikolay Samokhvalov 2026-09-13 18:39:39 Re: [PATCH] Avoid a REPACK worker startup hang
Previous Message Andrew Dunstan 2026-09-13 18:28:12 Re: pg_get_*_ddl() needs a redesign