Re: Synchronizing slots from primary to standby

On Tue, Feb 20, 2024 at 6:56 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> Miscellaneous comments:

Thanks for the comments.

> ========================
> 1.
> +void
> +ShutDownSlotSync(void)
> +{
> + SpinLockAcquire(&SlotSyncCtx->mutex);
> +
> + SlotSyncCtx->stopSignaled = true;
>
> This flag is never reset back. I think we should reset this once the
> promotion is complete. Though offhand, I don't see any problem with
> this but it doesn't look clean and can be a source of bugs in the
> future.

Did reset of flag in MaybeStartSlotSyncWorker() when we attempt to
start the worker after promotion completion and find that stopSignaled
is true while pmState is PM_RUN. From that point onwards, we can rely
on pmState to prevent the launch of the slot sync worker and thus can
reset stopSignaled.

> 2.
> +char *
> +CheckDbnameInConninfo(void)
>  {
>   char    *dbname;
>
> Let's name this function as CheckAndGetDbnameFromConninfo().

Modified.

> Apart from the above, I have made cosmetic changes in the attached.

Included these changes. Thanks.

Here are the v93 patches. It also addresses Swada-san's comment of
converting LOG to ERROR on receiving wal_level < logical.

I have also incorporated one more change wherein we check that
'Shutdown <= SmartShutdown' before launching the slot sync worker.
Since we do not need slot sync process to help in the rest of the
shutdown process, so better not to start it when shutdown (immediate
or fast) is going on. I have done this based on the details in [1]. It
is similar to WalReceiver behaviour.  Thoughts?

[1]: https://www.postgresql.org/message-id/flat/CAJpy0uCeQm2aFJLkx-D0BeAEvSdViTZf4wD7zT9coDHfLv1NaA%40mail.gmail.com

thanks
Shveta