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

On Thu, Aug 27, 2026 at 09:41:49AM -0700, Bharath Rupireddy wrote:

REPACK (CONCURRENTLY) starts a decoding bgworker and then waits in
start_repack_decoding_worker() for the worker to set a shared-memory
flag. The wait has no liveness check on the worker itself. If the
worker never reaches that point (e.g., fork failure under memory
pressure, or when BecomeLockGroupMember() returns false, or early exit
before the shm_mq error redirect is set up), the backend waits
indefinitely with no way out other than cancellation. I reproduced
this with an induced fork failure, so I think we need to tighten this
for both PG19 and HEAD branches.

Oops, I just concurrently reported this [0]/messages/by-id/apBpOVZOyqrakEr_@nathan. Note that teardown can
deadlock, too.

[0]: /messages/by-id/apBpOVZOyqrakEr_@nathan

--
nathan

Hi,

On Thu, Aug 27, 2026 at 9:47 AM Nathan Bossart <nathandbossart@gmail.com> wrote:

On Thu, Aug 27, 2026 at 09:41:49AM -0700, Bharath Rupireddy wrote:

REPACK (CONCURRENTLY) starts a decoding bgworker and then waits in
start_repack_decoding_worker() for the worker to set a shared-memory
flag. The wait has no liveness check on the worker itself. If the
worker never reaches that point (e.g., fork failure under memory
pressure, or when BecomeLockGroupMember() returns false, or early exit
before the shm_mq error redirect is set up), the backend waits
indefinitely with no way out other than cancellation. I reproduced
this with an induced fork failure, so I think we need to tighten this
for both PG19 and HEAD branches.

Oops, I just concurrently reported this [0]. Note that teardown can
deadlock, too.

[0] /messages/by-id/apBpOVZOyqrakEr_@nathan

Thanks. Here's my first attempt at fixing both the fork failure hang
and the teardown deadlock. I tried to use the parallel query approach
as much as possible.

The fork failure hang can occur because the backend running concurrent
repack sleeps on a CV and ignores the SIGUSR1 sent via bgw_notify_pid
by the postmaster upon fork failure.

The teardown deadlock can occur because the backend waits for the
worker to exit before detaching the error queue, while the worker is
blocked writing to a full queue, so both end up waiting on each other.

Although these issues seem rare to hit, I think it's good to tighten
the repack code because users can see them via SQL. Therefore, I think
we need to backpatch these to PG19. Please have a look at the attached
patch.

While here, I noticed that the same wait event is used for both the
worker startup wait and the file export wait. Ideally these would have
separate wait events, but given that the startup wait is typically
very short, reusing the same one seems fine.

--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com

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

On Thu, Aug 27, 2026 at 9:47 AM Nathan Bossart <nathandbossart@gmail.com> wrote:

On Thu, Aug 27, 2026 at 09:41:49AM -0700, Bharath Rupireddy wrote:

REPACK (CONCURRENTLY) starts a decoding bgworker and then waits in
start_repack_decoding_worker() for the worker to set a shared-memory
flag. The wait has no liveness check on the worker itself. If the
worker never reaches that point (e.g., fork failure under memory
pressure, or when BecomeLockGroupMember() returns false, or early exit
before the shm_mq error redirect is set up), the backend waits
indefinitely with no way out other than cancellation. I reproduced
this with an induced fork failure, so I think we need to tighten this
for both PG19 and HEAD branches.

Oops, I just concurrently reported this [0]. Note that teardown can
deadlock, too.

[0] /messages/by-id/apBpOVZOyqrakEr_@nathan

Thanks. Here's my first attempt at fixing both the fork failure hang
and the teardown deadlock. I tried to use the parallel query approach
as much as possible.

The fork failure hang can occur because the backend running concurrent
repack sleeps on a CV and ignores the SIGUSR1 sent via bgw_notify_pid
by the postmaster upon fork failure.

The teardown deadlock can occur because the backend waits for the
worker to exit before detaching the error queue, while the worker is
blocked writing to a full queue, so both end up waiting on each other.

Although these issues seem rare to hit, I think it's good to tighten
the repack code because users can see them via SQL. Therefore, I think
we need to backpatch these to PG19. Please have a look at the attached
patch.

Thanks for the fix(es). One thing I'm not sure I understand is:

@@ -3851,9 +3900,11 @@ ProcessRepackMessages(void)

 	/*
 	 * Nothing to do if we haven't launched the worker yet or have already
-	 * terminated it.
+	 * terminated it. stop_repack_decoding_worker() detaches the error queue
+	 * before clearing decoding_worker, so also bail out once error_mqh is
+	 * gone.
 	 */
-	if (decoding_worker == NULL)
+	if (decoding_worker == NULL || decoding_worker->error_mqh == NULL)
 		return;

/*

I don't think that stop_repack_decoding_worker() can clear ->error_mqh w/o
also clearing decoding_worker.

Other than that, I'm not sure you need to mention the condition variable in
the comments. And maybe even the mentions of parallel workers are not
necessary.

While here, I noticed that the same wait event is used for both the
worker startup wait and the file export wait. Ideally these would have
separate wait events, but given that the startup wait is typically
very short, reusing the same one seems fine.

I think that initially I also considered this situation not worth a new wait
event, but I probably had missed an existing one:
WAIT_EVENT_BGWORKER_STARTUP. It's already used for multiple workers, so we
could perhaps use it here.

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

Hi,

On Mon, Aug 31, 2026 at 3:31 AM Antonin Houska <ah@cybertec.at> wrote:

Thanks for the fix(es). One thing I'm not sure I understand is:

Thanks for reviewing.

@@ -3851,9 +3900,11 @@ ProcessRepackMessages(void)

/*
* Nothing to do if we haven't launched the worker yet or have already
-        * terminated it.
+        * terminated it. stop_repack_decoding_worker() detaches the error queue
+        * before clearing decoding_worker, so also bail out once error_mqh is
+        * gone.
*/
-       if (decoding_worker == NULL)
+       if (decoding_worker == NULL || decoding_worker->error_mqh == NULL)
return;

/*

I don't think that stop_repack_decoding_worker() can clear ->error_mqh w/o
also clearing decoding_worker.

With the patch, there's a window where stop_repack_decoding_worker()
sets the error queue pointer to NULL before setting the worker's
shared memory pointer to NULL. I would like to keep this check.|

Other than that, I'm not sure you need to mention the condition variable in
the comments. And maybe even the mentions of parallel workers are not
necessary.

Reworded the comments.

While here, I noticed that the same wait event is used for both the
worker startup wait and the file export wait. Ideally these would have
separate wait events, but given that the startup wait is typically
very short, reusing the same one seems fine.

I think that initially I also considered this situation not worth a new wait
event, but I probably had missed an existing one:
WAIT_EVENT_BGWORKER_STARTUP. It's already used for multiple workers, so we
could perhaps use it here.

I think using WAIT_EVENT_BGWORKER_STARTUP is fine, because if a worker
is ever stuck here, one can look at pg_stat_activity to tell whether
this is the REPACK decoding worker. I've done that in the 0002 patch.

Please find the attached v2 patches.

--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com

On Mon, Aug 31, 2026 at 11:14 AM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:

Hi,

On Mon, Aug 31, 2026 at 3:31 AM Antonin Houska <ah@cybertec.at> wrote:

Thanks for the fix(es). One thing I'm not sure I understand is:

Thanks for reviewing.

@@ -3851,9 +3900,11 @@ ProcessRepackMessages(void)

/*
* Nothing to do if we haven't launched the worker yet or have already
-        * terminated it.
+        * terminated it. stop_repack_decoding_worker() detaches the error queue
+        * before clearing decoding_worker, so also bail out once error_mqh is
+        * gone.
*/
-       if (decoding_worker == NULL)
+       if (decoding_worker == NULL || decoding_worker->error_mqh == NULL)
return;

/*

I don't think that stop_repack_decoding_worker() can clear ->error_mqh w/o
also clearing decoding_worker.

With the patch, there's a window where stop_repack_decoding_worker()
sets the error queue pointer to NULL before setting the worker's
shared memory pointer to NULL. I would like to keep this check.|

Other than that, I'm not sure you need to mention the condition variable in
the comments. And maybe even the mentions of parallel workers are not
necessary.

Reworded the comments.

While here, I noticed that the same wait event is used for both the
worker startup wait and the file export wait. Ideally these would have
separate wait events, but given that the startup wait is typically
very short, reusing the same one seems fine.

I think that initially I also considered this situation not worth a new wait
event, but I probably had missed an existing one:
WAIT_EVENT_BGWORKER_STARTUP. It's already used for multiple workers, so we
could perhaps use it here.

I think using WAIT_EVENT_BGWORKER_STARTUP is fine, because if a worker
is ever stuck here, one can look at pg_stat_activity to tell whether
this is the REPACK decoding worker. I've done that in the 0002 patch.

Please find the attached v2 patches.

I've reviewed the patch and I have two questions:

+   /*
+    * 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?

---
+       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?

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

Masahiko Sawada <sawada.mshk@gmail.com> wrote:

+   /*
+    * 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?

Although it probably does not break anything if we pass the handle to
shm_mq_set_handle() unnecessarily, I'm thinking if it's necessary for an error
queue.

AFAIU the purpose of the handle is to realize that it makes no sense to wait
for the background worker to attach to the queue. Once the worker has
attached, the worker's handle is no longer needed to detect that the worker
detached: the already-attached worker itself is supposed to call
shm_mq_detach().

If I use the parallel worker for reference, I think the leader should not be
asked (via the PROCSIG_PARALLEL_MESSAGE signal) to read from the error queue
until the worker has attached and redirected its messages to the queue - see
in ParallelWorkerMain():

mqh = shm_mq_attach(mq, seg, NULL);
pq_redirect_to_shm_mq(seg, mqh);

Therefore, the leader should not need the worker's handle.

I also thought about the other source of the PROCSIG_PARALLEL_MESSAGE signal,
a few lines above the call to shm_mq_attach():

before_shmem_exit(ParallelWorkerShutdown, PointerGetDatum(seg));

If the worker called proc_exit() after this, but before the call of
shm_mq_attach(), the worker handle might be useful for the leader to conclude
that the worker hasn't detached yet. However, the coding of
ProcessParallelMessages() is such that shm_mq_receive() can return
SHM_MQ_WOULD_BLOCK in such a case, so the ERROR "lost connection to parallel
worker" is not raised anyway.

While thinking about that ERROR, I realize that I might have misunderstood its
purpose, and therefore it's not present in
ProcessRepackMessage(). Nevertheless, I'm still not sure I understand when
exactly should that ERROR be raised. Per commit 2badb5afb8, the function
ParallelWorkerShutdown() is related. However, AFAICS, when the worker code
calls proc_exit(), the ParallelWorkerShutdown() callback can send the
PROCSIG_PARALLEL_MESSAGE signal while the worker is still attached to the
error queue. In that case, the ERROR is not raised.

I'm probably missing something. Thanks in advance for any hint.

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

Hi,

On Fri, Sep 4, 2026 at 5:11 PM Masahiko Sawada <sawada.mshk@gmail.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

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

On Fri, Sep 4, 2026 at 5:11 PM 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 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

Hi,

On Tue, Sep 8, 2026 at 12:17 AM Antonin Houska <ah@cybertec.at> wrote:

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

On Fri, Sep 4, 2026 at 5:11 PM 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 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

On Mon, Sep 7, 2026 at 12:15 PM Bharath Rupireddy
<bharath.rupireddyforpostgres@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 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.

Álvaro pointed me here from my thread:
/messages/by-id/CAM527d9A7FBXJn7A3w2VAPq0Gojm7tnUDFMh1uwJitGfi2kNeQ@mail.gmail.com

My AI harness tested v4. Silent worker exits still hang three
post-attachment waits. Mihail raised snapshot/replay liveness in
December 2025.

Snapshot repro on a test build configured with --enable-injection-points,
with v4 plus only the attached patch's test support:

create extension injection_points;
create table t (i int primary key);
insert into t select generate_series(1, 100);
set statement_timeout = '2s';
select injection_points_attach(
'repack-worker-before-snapshot-export',
'injection_points', 'injection_exit', null);
repack (concurrently) t;

This times out after worker exit. The attached patch applies atop v4.

As far as I can see, the current uncommitted patches cover overall 5 cases:

- startup before queue attach: Bharath v4
- full error queue teardown: Bharath v4
- silent exit after attach, before initialization: attached patch
- silent exit before snapshot export: attached patch
- silent exit before replay export: attached patch

Nik

On Sun, Sep 13, 2026 at 11:09 AM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:

Nikolay Samokhvalov <nik@postgres.ai> wrote:

On Mon, Sep 7, 2026 at 12:15 PM Bharath Rupireddy
<bharath.rupireddyforpostgres@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 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.

Álvaro pointed me here from my thread:
/messages/by-id/CAM527d9A7FBXJn7A3w2VAPq0Gojm7tnUDFMh1uwJitGfi2kNeQ@mail.gmail.com

My AI harness tested v4. Silent worker exits still hang three
post-attachment waits. Mihail raised snapshot/replay liveness in
December 2025.

Snapshot repro on a test build configured with --enable-injection-points,
with v4 plus only the attached patch's test support:

create extension injection_points;
create table t (i int primary key);
insert into t select generate_series(1, 100);
set statement_timeout = '2s';
select injection_points_attach(
'repack-worker-before-snapshot-export',
'injection_points', 'injection_exit', null);
repack (concurrently) t;

This times out after worker exit. The attached patch applies atop v4.

I'm not sure this should be considered a bug. AFAICS the problem happens due
to calling proc_exit() directly, i.e. exiting the worker w/o sending a message
via the error queue. (If an error was sent, ConditionVariableSleep() should
process it and raise ERROR in the backend running REPACK.) However, the REPACK
worker does not call proc_exit() directly.

(parallel.c tries to guard against worker calling proc_exit() and it seems to
have an issue, but it's a separate topic [1]/messages/by-id/64338.1788881902@localhost)

[1]: /messages/by-id/64338.1788881902@localhost

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

On Mon, Sep 14, 2026 at 2:00 AM Antonin Houska <ah@cybertec.at> wrote:

I'm not sure this should be considered a bug. AFAICS the problem happens
due to calling proc_exit() directly, i.e. exiting the worker w/o sending a
message via the error queue. (If an error was sent,
ConditionVariableSleep() should process it and raise ERROR in the backend
running REPACK.)

However, the REPACK worker does not call proc_exit() directly.

Yes, agreed: the direct proc_exit() test alone didn't establish a bug. I
checked whether error delivery can actually fail after attachment.

There is a path in mq_putmessage(): an interrupt while blocked on the full
error queue makes recursive FATAL output detach it via pq_mq_busy. The FATAL
reaches the server log, but not the REPACK backend.

I reproduced this on v4 plus only the attached repack_worker.c test hooks,
built with injection points and server-wide client_min_messages=debug1:

1. For queue_t with a primary key, attach the standard wait action:

create extension injection_points;
select injection_points_attach(
'repack-worker-before-snapshot-export', 'wait');

2. In another session run repack (concurrently) queue_t. When the worker waits
at InjectionPoint/repack-worker-before-snapshot-export, SIGSTOP the REPACK
backend and run:

select injection_points_wakeup(
'repack-worker-before-snapshot-export');

3. Repeat until the worker waits at IPC/MessageQueuePutMessage:

select pg_logical_emit_message(false, 'repack-queue-test',
repeat('x', 15 * 1024 * 1024));

4. Run select pg_terminate_backend(worker_pid), wait for the worker to exit,
then SIGCONT the REPACK backend.

With v4, REPACK remains active in RepackWorkerExport. With the patch, it
returns 55000, "REPACK worker exited before exporting requested data".

So the bug is real and the worker-status check is needed. The direct
proc_exit() test was only an incomplete reproducer. Thanks for making me
check it.

Nik

Hi,

On Sun, Sep 13, 2026 at 11:38 AM Nikolay Samokhvalov <nik@postgres.ai> wrote:

My AI harness tested v4. Silent worker exits still hang three
post-attachment waits. Mihail raised snapshot/replay liveness in
December 2025.

As far as I can see, the current uncommitted patches cover overall 5 cases:

- startup before queue attach: Bharath v4
- full error queue teardown: Bharath v4
- silent exit after attach, before initialization: attached patch
- silent exit before snapshot export: attached patch
- silent exit before replay export: attached patch

Thanks a lot for sharing these additional corner cases. The first two
and the later three are separate problems, but I agree we need to fix
them all. I spent some time reading how parallel query handles these
and ended up with the attached patches.

0001 fixes the first two cases: the backend now waits for the worker
to attach to the error message queue before waiting for decoding to be
initialized, and detaches from that queue before waiting for the
worker to exit.

0002 fixes the three silent exit cases reported above: the worker now
sends a Terminate message as its last act, so the queue going away
without it means the worker left with the work unfinished.

0003 adds tests using injection points. I reused the one shared
upthread and just added cases for the hang and the deadlock that 0001
fixes. I don't intend to get this in unless others think it's
necessary.

0004 is a cleanup: the backend left the flag saying there are worker
messages to process set even when there was no worker to read messages
from, so every later interrupt check went looking for messages for
nothing.

Thoughts?

--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com

Nikolay Samokhvalov <nik@postgres.ai> wrote:

On Mon, Sep 14, 2026 at 2:00 AM Antonin Houska <ah@cybertec.at> wrote:

I'm not sure this should be considered a bug. AFAICS the problem happens
due to calling proc_exit() directly, i.e. exiting the worker w/o sending a
message via the error queue. (If an error was sent,
ConditionVariableSleep() should process it and raise ERROR in the backend
running REPACK.)

However, the REPACK worker does not call proc_exit() directly.

Yes, agreed: the direct proc_exit() test alone didn't establish a bug. I
checked whether error delivery can actually fail after attachment.

There is a path in mq_putmessage(): an interrupt while blocked on the full
error queue makes recursive FATAL output detach it via pq_mq_busy. The FATAL
reaches the server log, but not the REPACK backend.

I reproduced this on v4 plus only the attached repack_worker.c test hooks,
built with injection points and server-wide client_min_messages=debug1:

1. For queue_t with a primary key, attach the standard wait action:

create extension injection_points;
select injection_points_attach(
'repack-worker-before-snapshot-export', 'wait');

2. In another session run repack (concurrently) queue_t. When the worker waits
at InjectionPoint/repack-worker-before-snapshot-export, SIGSTOP the REPACK
backend and run:

select injection_points_wakeup(
'repack-worker-before-snapshot-export');

3. Repeat until the worker waits at IPC/MessageQueuePutMessage:

select pg_logical_emit_message(false, 'repack-queue-test',
repeat('x', 15 * 1024 * 1024));

4. Run select pg_terminate_backend(worker_pid), wait for the worker to exit,
then SIGCONT the REPACK backend.

I wonder if you forgot to attach some files (I don't see the queue_t table in
the scripts you posted earlier), but I think I was able to reproduce the
problem using the instructions above.

IMO the problem is that the backend ignores the SHM_MQ_DETACHED state in
ProcessRepackMessages(). If all the messages in the queue have severity lower
than ERROR, the worker just processes them and continues waiting for the
worker to export the snapshot. However, as the worker is gone, there's no way
to wake the backend up.

Patch 0002 in [1]/messages/by-id/CALj2ACWJsOG0-2SQUxBXG9ULFs_GJpeVhhmp8AHf8c9iKDjMXA@mail.gmail.com should fix that.

[1]: /messages/by-id/CALj2ACWJsOG0-2SQUxBXG9ULFs_GJpeVhhmp8AHf8c9iKDjMXA@mail.gmail.com

Thanks for testing!

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

Hi Bharath,

I reviewed v5 patch by patch. v6 is attached

0001

It works. I made fork() really fail with a low RLIMIT_NPROC. On master,
REPACK (CONCURRENTLY) waits until statement_timeout. With 0001 it fails
at once with "REPACK decoding worker failed to start".

One problem. stop_repack_decoding_worker() now calls dsm_detach() before
it waits for the worker to exit. If the worker has mapped the segment
but not yet called SharedFileSetAttach(), the detach destroys the file
set, and the worker fails with

ERROR: could not attach to a SharedFileSet that is already destroyed

I hit this 12 times in 1200 cancelled REPACKs. Only shm_mq_detach() has
to come before the wait to fix the deadlock, so v6-0001 moves
dsm_detach() back after the wait, as on master. That gives 0 in 1200.

0002
Looks good. It follows parallel.c, and detaching before signalling is
needed, since with the old order the backend can read the queue too
early and never look again. With 0002 in, polling the worker status in
the wait loops, as Nikolay's patch does, is not needed.

No false alarms in 100 REPACK (CONCURRENTLY) runs with concurrent updates
and the owner's client_min_messages at debug1 and debug5, so the worker
often blocks on a full queue. pg_terminate_backend() on the worker gives
the backend the worker's own error at once.

0003
Please keep it. I checked that it catches each fix. Without the attach
wait, without the lost connection error, or with the queue detached
after the wait again, the test no longer passes.

One problem. The REPACK calls have no timeout, so a regression hangs
the test instead of failing it. Under meson it prints nothing until the
test is killed, and under make check nothing stops it. v6-0003 passes
timeout_default to those calls, so it now fails with "psql timed out"
at the right line. The rest of the test already waits with
timeout_default in poll_query_until(), pump_until() and
background_psql(), so this adds no new risk on slow machines.

0004
Looks fine. It matches what parallel.c does.

Thanks,
Shihao

Hello,

I have pushed these fixes, with some editorialization of my own.

I decided not to push the test case, because it has some nontrivial
additions to the injection point infrastructure that I'm not so sure
about. The part of the test that doesn't require those changes could
perhaps be considered for 19, and we can discuss adding the rest in 20.

Thanks!

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/

Hi,

On Thu, Sep 17, 2026 at 2:53 AM Alvaro Herrera <alvherre@kurilemu.de> wrote:

Hello,

I have pushed these fixes, with some editorialization of my own.

I decided not to push the test case, because it has some nontrivial
additions to the injection point infrastructure that I'm not so sure
about. The part of the test that doesn't require those changes could
perhaps be considered for 19, and we can discuss adding the rest in 20.

Thanks Alvaro for taking care of this. I think it is fine to not have
the tests in core and to have them here in the thread for reference.
My thinking is that the issues fixed here look rare in practice and
making them run reliably and deterministically would need more
developer cycles. Moreover, we haven't had such tests for parallel
query for a while, and testing resources are not free. Others may have
a different opinion.

And thanks Shihao for reviewing and identifying one of the problems
and proposing the fix for it.

--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com