From: Antonin Houska <ah@cybertec.at>
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
cc: Masahiko Sawada <sawada.mshk@gmail.com>,
    Nathan Bossart <nathandbossart@gmail.com>,
    PostgreSQL-development <pgsql-hackers@postgresql.org>,
    alvherre@kurilemu.de
Subject: Re: REPACK (CONCURRENTLY) backend waits indefinitely when decoding
 worker fails to start
In-reply-to: 
 <CALj2ACVxb8rWTie8xBdXUHhaQosomHjQHRUqr2VX+Fx0qFXTiw@mail.gmail.com>
References: 
 <CALj2ACVAxA9HxvFe8HSspTJ-UO4Aoz=kuQdZBeLrod0gqUxH3g@mail.gmail.com>
 <apBqNzV4N2CKa1ug@nathan>
 <CALj2ACUpxBdkAfvfT4=DsQ4k5Faz7W9Sqk27j+Y4HWexPSvUdA@mail.gmail.com>
 <27780.1788172295@localhost>
 <CALj2ACVoM4cd5bhOhqP+qqaetL9gWvChCFEoym6GZHHbz-faJw@mail.gmail.com>
 <CAD21AoDfyYqXH8wG2W=9j4T7yFrPvyM7tbGkvxNAMgUCP3w44w@mail.gmail.com>
 <CALj2ACVxb8rWTie8xBdXUHhaQosomHjQHRUqr2VX+Fx0qFXTiw@mail.gmail.com>
Comments: In-reply-to Bharath Rupireddy
 <bharath.rupireddyforpostgres@gmail.com>
   message dated "Mon, 07 Sep 2026 12:15:00 -0700."
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Date: Tue, 08 Sep 2026 09:17:31 +0200
Message-ID: <7564.1788851851@localhost>
Archived-At: 
 <https://www.postgresql.org/message-id/7564.1788851851%40localhost>
Precedence: bulk

Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote:

> On Fri, Sep 4, 2026 at 5:11=E2=80=AFPM Masahiko Sawada <sawada.mshk@gmail=
.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 t=
he logical decoding machinery before it waits for snapshot export.
> The first wait catches fork failures and worker startup issues. I used si=
milar 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.
>=20
> Although the initialized flag wait may seem redundant with the snapshot e=
xport wait in get_initial_snapshot(), I would still keep it because it
> ensures the worker has fully set up the decoding before the backend proce=
eds.
>=20
> 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 read=
y.

Another reason for two separate waits is that two separate event types make
sense: WAIT_EVENT_BGWORKER_STARTUP and WAIT_EVENT_REPACK_WORKER_EXPORT.

> Please find the attached v3 patch.

Just two comments:

* wait_for_repack_worker_to_attach() - as the decoding_worker variable is
  declared static, this function does not necessarily need the argument.

* 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 wi=
ll
+        * implicitly detach the error message queue, and any other shared =
memory
+        * queues, stored there.
+        */
+       if (decoding_worker->seg !=3D NULL)
+       {
+               dsm_detach(decoding_worker->seg);
+               decoding_worker->seg =3D NULL;
+       }

--=20
Antonin Houska
Web: https://www.cybertec-postgresql.com