Re: Allow logical failover slots to wait on synchronous replication

Hi Shveta,

On Sun, Sep 8, 2024 at 11:16 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:

>
> I was trying to have a look at the patch again, it doesn't apply on
> the head, needs rebase.
>

Rebased with the latest changes.

> Regarding 'mode = SyncRepWaitMode', FWIW, SyncRepWaitForLSN() also
> does in a similar way. It gets mode in local var initially and uses it
> later. See [1]. So isn't there a chance too that 'SyncRepWaitMode' can
> change in between.
>
> [1]:
> mode = SyncRepWaitMode;
> .....
> ....
> if (!WalSndCtl->sync_standbys_defined ||
> lsn <= WalSndCtl->lsn[mode])
> {
> LWLockRelease(SyncRepLock);
> return;
> }

You are right, thanks for the correction. I tried reproducing with GDB
where SyncRepWaitMode
changes due to pg_ctl reload but was unable to do so. It seems like
SIGHUP only sets ConfigReloadPending = true,
which gets processed in the next loop in WalSndLoop() and that's
probably where I was getting confused.

In the latest patch, I've added:

Assert(SyncRepWaitMode >= 0);

which should be true since we call SyncRepConfigured() at the
beginning of StandbySlotsHaveCaughtup(),
and used SyncRepWaitMode directly.

Thank you

--
John Hsu - Amazon Web Services