From: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
---|---|
To: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
Cc: | "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, Alexander Lakhin <exclusion(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Perform streaming logical transactions by background workers and parallel apply |
Date: | 2023-05-08 03:07:51 |
Message-ID: | CAD21AoB0kM3SEy+HNWxFBt_HSemCq7XKE3woiAHoEb+kUnbh7Q@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, May 2, 2023 at 12:22 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Fri, Apr 28, 2023 at 11:48 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >
> > While investigating this issue, I've reviewed the code around
> > callbacks and worker termination etc and I found a problem.
> >
> > A parallel apply worker calls the before_shmem_exit callbacks in the
> > following order:
> >
> > 1. ShutdownPostgres()
> > 2. logicalrep_worker_onexit()
> > 3. pa_shutdown()
> >
> > Since the worker is detached during logicalrep_worker_onexit(),
> > MyLogicalReplication->leader_pid is an invalid when we call
> > pa_shutdown():
> >
> > static void
> > pa_shutdown(int code, Datum arg)
> > {
> > Assert(MyLogicalRepWorker->leader_pid != InvalidPid);
> > SendProcSignal(MyLogicalRepWorker->leader_pid,
> > PROCSIG_PARALLEL_APPLY_MESSAGE,
> > InvalidBackendId);
> >
> > Also, if the parallel apply worker fails shm_toc_lookup() during the
> > initialization, it raises an error (because of noError = false) but
> > ends up a SEGV as MyLogicalRepWorker is still NULL.
> >
> > I think that we should not use MyLogicalRepWorker->leader_pid in
> > pa_shutdown() but instead store the leader's pid to a static variable
> > before registering pa_shutdown() callback.
> >
>
> Why not simply move the registration of pa_shutdown() to someplace
> after logicalrep_worker_attach()?
If we do that, the worker won't call dsm_detach() if it raises an
ERROR in logicalrep_worker_attach(), is that okay? It seems that it's
no practically problem since we call dsm_backend_shutdown() in
shmem_exit(), but if so why do we need to call it in pa_shutdown()?
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2023-05-08 03:09:09 | Re: PGDOCS - Replica Identity quotes |
Previous Message | Amit Kapila | 2023-05-08 03:07:35 | Re: [DOC] Update ALTER SUBSCRIPTION documentation |