| From: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
|---|---|
| To: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
| Cc: | Antonin Houska <ah(at)cybertec(dot)at>, Nathan Bossart <nathandbossart(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Álvaro Herrera <alvherre(at)kurilemu(dot)de> |
| Subject: | Re: REPACK (CONCURRENTLY) backend waits indefinitely when decoding worker fails to start |
| Date: | 2026-09-07 19:15:00 |
| Message-ID: | CALj2ACVxb8rWTie8xBdXUHhaQosomHjQHRUqr2VX+Fx0qFXTiw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
On Fri, Sep 4, 2026 at 5:11 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
wrote:
>
> I've reviewed the patch and I have two questions:
Thanks for reviewing it.
> + /*
> + * Associate the worker's handle with the error queue, just as if it
had
> + * been passed to shm_mq_attach(); we passed NULL there because the
worke
> + * did not exist yet. This lets ProcessRepackMessages() notice the
worker
> + * is gone instead of blocking on the queue.
> + */
> + shm_mq_set_handle(decoding_worker->error_mqh,
decoding_worker->handle);
> +
>
> The second sentence starting with "This lets ..." is unclear to me.
> ProcessRepackMessage() receives the message with nowait, no?
Yes, the read is nowait, and I get that the comment is misleading. All we
do here is connect the backend running concurrent repack (the receiver)
with the decoding worker (the sender) by saving the worker's handle to the
error message queue. We could not pass that handle to shm_mq_attach() above
because the sender did not exist at that point. The handle only matters
until the sender attaches to the queue. Until then, the receiver uses the
handle to check whether the sender is still alive, and reports detached if
the sender is already gone, instead of blocking on the queue. parallel.c
does the same for its workers.
> ---
> + if (status == BGWH_STOPPED)
> + ereport(ERROR,
> + errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> + errmsg("REPACK decoding worker failed to start"),
> + errhint("More details may be available in the
> server log."));
> + if (status == BGWH_POSTMASTER_DIED)
>
> parallel.c handles BGWH_STOPPED differently; it checks that the worker
> stopped without attaching to the error queue. IIUC if the worker
> stopped after attaching to the error queue, an error message should
> arrive at the leader and the leader could handle it in the next CFI.
> Is there any reason why start_repack_decoding_worker() handles it
> differently?
Right. There are two cases. The worker started and exited after attaching
to the error message queue, or it started and exited before attaching to
it. In the second case, throwing the generic error is correct, and in the
first the next CFI captures the worker's error. The attached v3 handles
both. Please have a look.
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.
Please find the attached v3 patch.
--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com
| Attachment | Content-Type | Size |
|---|---|---|
| v3-0001-Fix-hang-and-deadlock-in-concurrent-REPACK-worker.patch | application/x-patch | 10.4 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Peter Eisentraut | 2026-09-07 19:34:28 | Re: PGQ catalog representation and pg_dump support |
| Previous Message | Antonin Houska | 2026-09-07 18:46:20 | Re: REPACK (CONCURRENTLY) backend waits indefinitely when decoding worker fails to start |