MIME-Version: 1.0
References: 
 <CAHGQGwFzNYroAxSoyJhqTU-pH=t4Ej6RyvhVmBZ91Exj_TPMMQ@mail.gmail.com>
 <CAA4eK1+CrQNqiPDKv1wYfdkbX0FARJoi1=0ioaAqkLzbq2vG1w@mail.gmail.com>
 <CAHGQGwHABvuCoyM24HUiFZ5oJq_CoFomjt_cqD-0cJLMjFXJjQ@mail.gmail.com>
 <CAA4eK1LzZGfRANPAnv6NpKCH2ENuZO6HswgY14A=xsOXmucPhw@mail.gmail.com>
 <CABdArM6nepct0uxizCnZqy-kAjjTOndvu7bWtaNcmknuxx82Hg@mail.gmail.com>
 <CAJpy0uBhJB4HqouLXegD=miSkZfZp86tTC2K0K6nf=bHCcsPLQ@mail.gmail.com>
 <CABdArM6Mfhk2+9TVR_D3cgfPWPfHDuZEg7MOc5KqULwt0OcQUg@mail.gmail.com>
 <CAA4eK1+d2vN80-Yvy_Hr=ATF3XL5db+_W-sXF=2Vxm+OFBO82w@mail.gmail.com>
 <CAA4eK1KxU4b53GUor41A55x+Bx-DdOaQ9g1DqURyRY2Cg-hhPQ@mail.gmail.com>
 <CABdArM6pmn5yFqiU33KTYBXYM=Vny2ULnJY_gqFbsMEdt+1dPA@mail.gmail.com>
 <CAHGQGwEsEjXbCthXb02=HScBW7C=BHmygv9SK1VGCcc-9bmTsw@mail.gmail.com>
 <CABdArM5rrhSmFvVL4C5LL0iea-R0HRtB=ZvD=ereoTDa1Tm=NA@mail.gmail.com>
 <CAHGQGwFaLPMWTHaPtxTpRb3_=d4o+SLu6+89BNEzALdEnvSWmQ@mail.gmail.com>
 <CABdArM7fPqd9GSXLLyDZfX_bZkAaoJAVDGKSQULfvEvVZZHgsg@mail.gmail.com>
In-Reply-To: 
 <CABdArM7fPqd9GSXLLyDZfX_bZkAaoJAVDGKSQULfvEvVZZHgsg@mail.gmail.com>
From: shveta malik <shveta.malik@gmail.com>
Date: Tue, 31 Mar 2026 16:12:00 +0530
Message-ID: 
 <CAJpy0uDaR4LKy2T=vLWuCnY8nQ=m7Zde_sr44aoYr7T0jodV2Q@mail.gmail.com>
Subject: Re: Use SIGTERM instead of SIGUSR1 for slotsync worker to exit during
 promotion?
To: Nisha Moond <nisha.moond412@gmail.com>
Cc: Fujii Masao <masao.fujii@gmail.com>,
 Amit Kapila <amit.kapila16@gmail.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
 shveta malik <shveta.malik@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAJpy0uDaR4LKy2T%3DvLWuCnY8nQ%3Dm7Zde_sr44aoYr7T0jodV2Q%40mail.gmail.com>
Precedence: bulk

On Tue, Mar 31, 2026 at 11:35=E2=80=AFAM Nisha Moond <nisha.moond412@gmail.=
com> wrote:
>
> On Mon, Mar 30, 2026 at 4:39=E2=80=AFPM Fujii Masao <masao.fujii@gmail.co=
m> wrote:
> >
> > On Mon, Mar 30, 2026 at 1:18=E2=80=AFPM Nisha Moond <nisha.moond412@gma=
il.com> wrote:
> > > We were using the same log message in two places:
> > > check_and_set_sync_info() and HandleSlotSyncMessage().
> > > I think =E2=80=9Cwill not start=E2=80=9D fits better in the first cas=
e, while =E2=80=9Cwill
> > > stop=E2=80=9D makes sense to keep in the second.
> >
> > Thanks for updating the patch!
> >
> > With the patch, in my testing, standby promotion always produces
> > the following logs:
> >
> >     LOG:  replication slot synchronization worker will stop because
> > promotion is triggered
> >     LOG:  replication slot synchronization worker will not start
> > because promotion was triggered
> >
> > It looks like the postmaster immediately restarts the slotsync worker a=
fter
> > promotion terminates it, and that new worker then exits on seeing
> > SlotSyncCtx->stopSignaled.
> >
> > IMO, always emitting both messages is a bit confusing. It would be nice=
 to
> > suppress the second one if possible.
> >
> > One idea would be to prevent the restart altogether. For example,
> > ProcessSlotSyncMessage() could set SlotSyncCtx->last_start_time to
> > a special value (like -1), and SlotSyncWorkerCanRestart() could return
> > false (i.e., prevent postmater from starting up slotsync worker) when
> > it sees that. Alternatively, SlotSyncWorkerCanRestart() could simply
> > check SlotSyncCtx->stopSignaled.
> >
> > That said, as far as I remember correctly, postmaster is generally not
> > supposed to touch shared memory (per the comments in postmaster.c),
> > so I'm not sure this approach is acceptable. On the other hand,
> > postmaster and the slotsync worker already rely on SlotSyncCtx->last_st=
art_time,
> > so perhaps there's some precedent here.
> >
> IIUC, checking SlotSyncCtx->stopSignaled in SlotSyncWorkerCanRestart()
> may not be ideal, as it requires a spinlock to avoid races with the
> startup process and it is disallowed to take lock in postmaster main
> loop. Whereas, SlotSyncCtx->last_start_time doesn=E2=80=99t need a lock s=
ince
> the postmaster accesses it only when the worker is not alive.
>

I agree.

> Another option could be to log in check_and_set_sync_info() at DEBUG1
> instead of LOG level. This message appears only after stopSignaled is
> set, when promotion is already in progress and the first worker has
> logged =E2=80=9Cwill stop=E2=80=A6=E2=80=9D. The second worker doesn=E2=
=80=99t do any real work. Since
> there=E2=80=99s nothing actionable for users, using DEBUG1 would keep it
> useful for debugging (e.g., noticing immediate restarts) while
> avoiding extra log noise. Thoughts?
>

+1.

Do you think we can slightly tweak the comment in atop file to:

On promotion the startup process sets 'stopSignaled' and uses this
'pid' to signal synchronizing process with PROCSIG_SLOTSYNC_MESSAGE
and also to wake it up so that the process can immediately stop its
synchronizing work. Setting 'stopSignaled' on the other hand is  used
to handle the race condition....

Also shall we quick exit ProcessSlotSyncMessage() if syncing is
already finished by API?

thanks
Shveta