On Tue, May 9, 2023 at 7:50 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Mon, May 8, 2023 at 8:09 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> >
> > I think it is only possible for the leader apply can worker to try to
> > receive the error message from an error queue after your 0002 patch.
> > Because another place already detached from the queue before stopping
> > the parallel apply workers. So, I combined both the patches and
> > changed a few comments and a commit message. Let me know what you
> > think of the attached.
>
> I have one comment on the detaching error queue part:
>
> + /*
> + * Detach from the error_mq_handle for the parallel apply worker before
> + * stopping it. This prevents the leader apply worker from trying to
> + * receive the message from the error queue that might already
> be detached
> + * by the parallel apply worker.
> + */
> + shm_mq_detach(winfo->error_mq_handle);
> + winfo->error_mq_handle = NULL;
>
> In pa_detach_all_error_mq(), we try to detach error queues of all
> workers in the pool. I think we should check if the queue is already
> detached (i.e. is NULL) there. Otherwise, we will end up a SEGV if an
> error happens after detaching the error queue and before removing the
> worker from the pool.
>
Agreed, I have made this change, added the same check at one other
place for the sake of consistency, and pushed the patch.
--
With Regards,
Amit Kapila.
