Hi, On Tue, Sep 8, 2026 at 12:17 AM Antonin Houska <[email protected]> wrote: > > Bharath Rupireddy <[email protected]> wrote: > > > On Fri, Sep 4, 2026 at 5:11 PM Masahiko Sawada <[email protected]> > > 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
v4-0001-Fix-hang-and-deadlock-in-concurrent-REPACK-worker.patch
Description: Binary data
