| From: | Antonin Houska <ah(at)cybertec(dot)at> |
|---|---|
| To: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
| Cc: | 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-08 07:17:31 |
| Message-ID: | 7564.1788851851@localhost |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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.
> 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 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;
+ }
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tatsuo Ishii | 2026-09-08 07:18:26 | Re: Row pattern recognition |
| Previous Message | Michael Paquier | 2026-09-08 07:14:20 | Re: Improve error handling in test modules: test_extensible, test_bitmapset |