Re: sync_standbys_defined read/write race on startup

On 10.04.2025 05:25, Michael Paquier wrote:
> Confirmed. One thing where it would be possible to make things work
> is to introduce some persistency of the injection points, say these
> are flushed at shutdown. We could do that without touching at the
> backend code and only in the module injection_points, but perhaps this
> thread is not enough to justify this extra complexity.

Agree, it should be discussed in another thread.

On 10.04.2025 05:25, Michael Paquier wrote:
> What do you think?

Yes, patch for me looks ok, only one suggestion, I am not sure

but I am afraid we are loosing this.

        if (WalSndCtl->sync_standbys_status & SYNC_STANDBY_INIT)
        {
                if ((WalSndCtl->sync_standbys_status &
SYNC_STANDBY_DEFINED) == 0 ||
                        lsn <= WalSndCtl->lsn[mode])
                {
                        LWLockRelease(SyncRepLock);
                        return;
                }
        }
        else if (!SyncStandbysDefined())
        {
                /*
                 * If we are here, the sync standby data has not been
initialized yet,
                 * so fall back to the best thing we can do: a check on
                 * SyncStandbysDefined() to see if the GUC is set or not.
                 *
                 * If the GUC has a value, we wait until the
checkpointer updates the
                 * status data because we cannot be sure yet if we
should wait or not.
                 * If the GUC has *no* value, we are sure that there is
no point to
                 * wait; this matters for example when initializing a
cluster, where
                 * we should never wait, and no sync standbys is the
default behavior.
                 */
                LWLockRelease(SyncRepLock);
                return;
        }
+       else if (lsn <= WalSndCtl->lsn[mode])
+       {
+               LWLockRelease(SyncRepLock);
+               return;
+       }

What do you think?

Best regards,

Maksim Melnikov