diff --git a/src/backend/replication/syncrep.c b/src/backend/replication/syncrep.c index df1e341c764..3fa5fa42ee4 100644 --- a/src/backend/replication/syncrep.c +++ b/src/backend/replication/syncrep.c @@ -177,16 +177,52 @@ SyncRepWaitForLSN(XLogRecPtr lsn, bool commit) Assert(MyProc->syncRepState == SYNC_REP_NOT_WAITING); /* - * We don't wait for sync rep if WalSndCtl->sync_standbys_defined is not - * set. See SyncRepUpdateSyncStandbysDefined. + * We don't wait for sync rep if SYNC_STANDBY_DEFINED is not set. See + * SyncRepUpdateSyncStandbysDefined(). * * Also check that the standby hasn't already replied. Unlikely race * condition but we'll be fetching that cache line anyway so it's likely * to be a low cost check. + * + * If the sync standby data has not been initialized yet + * (SYNC_STANDBY_INIT is not set), fall back to a check based on the LSN, + * then do a direct GUC check. */ - if (!WalSndCtl->sync_standbys_defined || - lsn <= WalSndCtl->lsn[mode]) + 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 (lsn <= WalSndCtl->lsn[mode]) + { + /* + * The LSN is older than what we need to wait for. The sync standby + * data has not been initialized yet, but we are OK to not wait + * because we know that there is no point in doing so based on the + * LSN. + */ + LWLockRelease(SyncRepLock); + return; + } + else if (!SyncStandbysDefined()) + { + /* + * If we are here, the sync standby data has not been initialized yet, + * and the LSN is newer than what need to wait for, so we have fallen + * back to the best thing we could do in this case: a check on + * SyncStandbysDefined() to see if the GUC is set or not. + * + * When 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. Here, 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; } @@ -927,7 +963,7 @@ SyncRepWakeQueue(bool all, int mode) /* * The checkpointer calls this as needed to update the shared - * sync_standbys_defined flag, so that backends don't remain permanently wedged + * sync_standbys_status flag, so that backends don't remain permanently wedged * if synchronous_standby_names is unset. It's safe to check the current value * without the lock, because it's only ever updated by one process. But we * must take the lock to change it. @@ -937,7 +973,8 @@ SyncRepUpdateSyncStandbysDefined(void) { bool sync_standbys_defined = SyncStandbysDefined(); - if (sync_standbys_defined != WalSndCtl->sync_standbys_defined) + if (sync_standbys_defined != + ((WalSndCtl->sync_standbys_status & SYNC_STANDBY_DEFINED) != 0)) { LWLockAcquire(SyncRepLock, LW_EXCLUSIVE); @@ -961,7 +998,30 @@ SyncRepUpdateSyncStandbysDefined(void) * backend that hasn't yet reloaded its config might go to sleep on * the queue (and never wake up). This prevents that. */ - WalSndCtl->sync_standbys_defined = sync_standbys_defined; + WalSndCtl->sync_standbys_status = SYNC_STANDBY_INIT | + (sync_standbys_defined ? SYNC_STANDBY_DEFINED : 0); + + LWLockRelease(SyncRepLock); + } + else if ((WalSndCtl->sync_standbys_status & SYNC_STANDBY_INIT) == 0) + { + LWLockAcquire(SyncRepLock, LW_EXCLUSIVE); + + /* + * Note that there is no need to wake up the queues here. We would + * reach this path only if SyncStandbysDefined() returns false, or it + * would mean that some backends are waiting with the GUC set. See + * SyncRepWaitForLSN(). + */ + Assert(!SyncStandbysDefined()); + + /* + * Even if there is no sync standby defined, let the readers of this + * information know that the sync standby data has been initialized. + * This can just be done once, hence the previous check on + * SYNC_STANDBY_INIT to avoid useless work. + */ + WalSndCtl->sync_standbys_status |= SYNC_STANDBY_INIT; LWLockRelease(SyncRepLock); } diff --git a/src/include/replication/walsender_private.h b/src/include/replication/walsender_private.h index b7332655070..3b7167b9d30 100644 --- a/src/include/replication/walsender_private.h +++ b/src/include/replication/walsender_private.h @@ -98,15 +98,30 @@ typedef struct XLogRecPtr lsn[NUM_SYNC_REP_WAIT_MODE]; /* - * Are any sync standbys defined? Waiting backends can't reload the - * config file safely, so checkpointer updates this value as needed. - * Protected by SyncRepLock. + * Status of data related to the synchronous standbys. Waiting backends + * can't reload the config file safely, so checkpointer updates this value + * as needed. Protected by SyncRepLock. */ - bool sync_standbys_defined; + bits8 sync_standbys_status; WalSnd walsnds[FLEXIBLE_ARRAY_MEMBER]; } WalSndCtlData; +/* Flags for WalSndCtlData->sync_standbys_status */ + +/* + * Is the synchronous standby data initialized from the GUC? This is set the + * first time synchronous_standby_names is processed by the checkpointer. + */ +#define SYNC_STANDBY_INIT (1 << 0) + +/* + * Is the synchronous standby data defined? This is set when + * synchronous_standby_names has some data, after being processed by the + * checkpointer. + */ +#define SYNC_STANDBY_DEFINED (1 << 1) + extern WalSndCtlData *WalSndCtl;