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

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