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

Hi,

On Mon, Aug 31, 2026 at 3:31 AM Antonin Houska <ah(at)cybertec(dot)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