public inbox for [email protected]help / color / mirror / Atom feed
Re: Allow async standbys wait for sync replication (was: Disallow quorum uncommitted (with synchronous standbys) txns in logical replication subscribers) 13+ messages / 4 participants [nested] [flat]
* Re: Allow async standbys wait for sync replication (was: Disallow quorum uncommitted (with synchronous standbys) txns in logical replication subscribers) @ 2022-02-26 16:07 Nathan Bossart <[email protected]> 0 siblings, 1 reply; 13+ messages in thread From: Nathan Bossart @ 2022-02-26 16:07 UTC (permalink / raw) To: Bharath Rupireddy <[email protected]>; +Cc: SATYANARAYANA NARLAPURAM <[email protected]>; PostgreSQL Hackers <[email protected]> On Sat, Feb 26, 2022 at 02:17:50PM +0530, Bharath Rupireddy wrote: > A global min LSN of SendRqstPtr of all the sync standbys can be > calculated and the async standbys can send WAL up to global min LSN. > This is unlike what the v1 patch does i.e. async standbys will wait > until the sync standbys report flush LSN back to the primary. Problem > with the global min LSN approach is that there can still be a small > window where async standbys can get ahead of sync standbys. Imagine > async standbys being closer to the primary than sync standbys and if > the failover has to happen while the WAL at SendRqstPtr isn't received > by the sync standbys, but the async standbys can receive them as they > are closer. We hit the same problem that we are trying to solve with > this patch. This is the reason, we are waiting till the sync flush LSN > as it guarantees more transactional protection. Do you mean that the application of WAL gets ahead on your async standbys or that the writing/flushing of WAL gets ahead? If synchronous_commit is set to 'remote_write' or 'on', I think either approach can lead to situations where the async standbys are ahead of the sync standbys with WAL application. For example, a conflict between WAL replay and a query on your sync standby could delay WAL replay, but the primary will not wait for this conflict to resolve before considering a transaction synchronously replicated and sending it to the async standbys. If writing/flushing WAL gets ahead on async standbys, I think something is wrong with the patch. If you aren't sending WAL to async standbys until it is synchronously replicated to the sync standbys, it should by definition be impossible for this to happen. If you wanted to make sure that WAL was not applied to async standbys before it was applied to sync standbys, I think you'd need to set synchronous_commit to 'remote_apply'. This would ensure that the WAL is replayed on sync standbys before the primary considers the transaction synchronously replicated and sends it to the async standbys. > Do you think allowing async standbys optionally wait for either remote > write or flush or apply or global min LSN of SendRqstPtr so that users > can choose what they want? I'm not sure I follow the difference between "global min LSN of SendRqstPtr" and remote write/flush/apply. IIUC you are saying that we could use the LSN of what is being sent to sync standbys instead of the LSN of what the primary considers synchronously replicated. I don't think we should do that because it provides no guarantee that the WAL has even been sent to the sync standbys before it is sent to the async standbys. For this feature, I think we always need to consider what the primary considers synchronously replicated. My suggested approach doesn't change that. I'm saying that instead of spinning in a loop waiting for the WAL to be synchronously replicated, we just immediately send WAL up to the LSN that is presently known to be synchronously replicated. You do bring up an interesting point, though. Is there a use-case for specifying synchronous_commit='on' but not sending WAL to async replicas until it is synchronously applied? Or alternatively, would anyone want to set synchronous_commit='remote_apply' but send WAL to async standbys as soon as it is written to the sync standbys? My initial reaction is that we should depend on the synchronous replication setup. As long as the primary considers an LSN synchronously replicated, it would be okay to send it to the async standbys. I personally don't think it is worth taking on the extra complexity for that level of configuration just yet. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Allow async standbys wait for sync replication (was: Disallow quorum uncommitted (with synchronous standbys) txns in logical replication subscribers) @ 2022-02-28 13:15 Bharath Rupireddy <[email protected]> parent: Nathan Bossart <[email protected]> 0 siblings, 1 reply; 13+ messages in thread From: Bharath Rupireddy @ 2022-02-28 13:15 UTC (permalink / raw) To: Nathan Bossart <[email protected]>; +Cc: SATYANARAYANA NARLAPURAM <[email protected]>; PostgreSQL Hackers <[email protected]> On Sat, Feb 26, 2022 at 9:37 PM Nathan Bossart <[email protected]> wrote: > > On Sat, Feb 26, 2022 at 02:17:50PM +0530, Bharath Rupireddy wrote: > > A global min LSN of SendRqstPtr of all the sync standbys can be > > calculated and the async standbys can send WAL up to global min LSN. > > This is unlike what the v1 patch does i.e. async standbys will wait > > until the sync standbys report flush LSN back to the primary. Problem > > with the global min LSN approach is that there can still be a small > > window where async standbys can get ahead of sync standbys. Imagine > > async standbys being closer to the primary than sync standbys and if > > the failover has to happen while the WAL at SendRqstPtr isn't received > > by the sync standbys, but the async standbys can receive them as they > > are closer. We hit the same problem that we are trying to solve with > > this patch. This is the reason, we are waiting till the sync flush LSN > > as it guarantees more transactional protection. > > Do you mean that the application of WAL gets ahead on your async standbys > or that the writing/flushing of WAL gets ahead? If synchronous_commit is > set to 'remote_write' or 'on', I think either approach can lead to > situations where the async standbys are ahead of the sync standbys with WAL > application. For example, a conflict between WAL replay and a query on > your sync standby could delay WAL replay, but the primary will not wait for > this conflict to resolve before considering a transaction synchronously > replicated and sending it to the async standbys. > > If writing/flushing WAL gets ahead on async standbys, I think something is > wrong with the patch. If you aren't sending WAL to async standbys until > it is synchronously replicated to the sync standbys, it should by > definition be impossible for this to happen. With the v1 patch [1], the async standbys will never get WAL ahead of sync standbys. That is guaranteed because the walsenders serving async standbys are allowed to send WAL only after the walsenders serving sync standbys receive the synchronous flush LSN. > > Do you think allowing async standbys optionally wait for either remote > > write or flush or apply or global min LSN of SendRqstPtr so that users > > can choose what they want? > > I'm not sure I follow the difference between "global min LSN of > SendRqstPtr" and remote write/flush/apply. IIUC you are saying that we > could use the LSN of what is being sent to sync standbys instead of the LSN > of what the primary considers synchronously replicated. I don't think we > should do that because it provides no guarantee that the WAL has even been > sent to the sync standbys before it is sent to the async standbys. Correct. > For > this feature, I think we always need to consider what the primary considers > synchronously replicated. My suggested approach doesn't change that. I'm > saying that instead of spinning in a loop waiting for the WAL to be > synchronously replicated, we just immediately send WAL up to the LSN that > is presently known to be synchronously replicated. As I said above, v1 patch does that i.e. async standbys wait until the sync standbys update their flush LSN. Flush LSN is this - flushLSN = walsndctl->lsn[SYNC_REP_WAIT_FLUSH]; which gets updated in SyncRepReleaseWaiters. Async standbys with their SendRqstPtr will wait in XLogSendPhysical or XLogSendLogical until SendRqstPtr <= flushLSN. I will address review comments raised by Hsu, John and send the updated patch for further review. Thanks. [1] https://www.postgresql.org/message-id/CALj2ACVUa8WddVDS20QmVKNwTbeOQqy4zy59NPzh8NnLipYZGw%40mail.gma... Regards, Bharath Rupireddy. ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Allow async standbys wait for sync replication (was: Disallow quorum uncommitted (with synchronous standbys) txns in logical replication subscribers) @ 2022-02-28 18:57 Nathan Bossart <[email protected]> parent: Bharath Rupireddy <[email protected]> 0 siblings, 1 reply; 13+ messages in thread From: Nathan Bossart @ 2022-02-28 18:57 UTC (permalink / raw) To: Bharath Rupireddy <[email protected]>; +Cc: SATYANARAYANA NARLAPURAM <[email protected]>; PostgreSQL Hackers <[email protected]> On Mon, Feb 28, 2022 at 06:45:51PM +0530, Bharath Rupireddy wrote: > On Sat, Feb 26, 2022 at 9:37 PM Nathan Bossart <[email protected]> wrote: >> For >> this feature, I think we always need to consider what the primary considers >> synchronously replicated. My suggested approach doesn't change that. I'm >> saying that instead of spinning in a loop waiting for the WAL to be >> synchronously replicated, we just immediately send WAL up to the LSN that >> is presently known to be synchronously replicated. > > As I said above, v1 patch does that i.e. async standbys wait until the > sync standbys update their flush LSN. > > Flush LSN is this - flushLSN = walsndctl->lsn[SYNC_REP_WAIT_FLUSH]; > which gets updated in SyncRepReleaseWaiters. > > Async standbys with their SendRqstPtr will wait in XLogSendPhysical or > XLogSendLogical until SendRqstPtr <= flushLSN. My feedback is specifically about this behavior. I don't think we should spin in XLogSend*() waiting for an LSN to be synchronously replicated. I think we should just choose the SendRqstPtr based on what is currently synchronously replicated. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Allow async standbys wait for sync replication (was: Disallow quorum uncommitted (with synchronous standbys) txns in logical replication subscribers) @ 2022-03-01 05:40 Bharath Rupireddy <[email protected]> parent: Nathan Bossart <[email protected]> 0 siblings, 1 reply; 13+ messages in thread From: Bharath Rupireddy @ 2022-03-01 05:40 UTC (permalink / raw) To: Nathan Bossart <[email protected]>; +Cc: SATYANARAYANA NARLAPURAM <[email protected]>; PostgreSQL Hackers <[email protected]> On Tue, Mar 1, 2022 at 12:27 AM Nathan Bossart <[email protected]> wrote: > > On Mon, Feb 28, 2022 at 06:45:51PM +0530, Bharath Rupireddy wrote: > > On Sat, Feb 26, 2022 at 9:37 PM Nathan Bossart <[email protected]> wrote: > >> For > >> this feature, I think we always need to consider what the primary considers > >> synchronously replicated. My suggested approach doesn't change that. I'm > >> saying that instead of spinning in a loop waiting for the WAL to be > >> synchronously replicated, we just immediately send WAL up to the LSN that > >> is presently known to be synchronously replicated. > > > > As I said above, v1 patch does that i.e. async standbys wait until the > > sync standbys update their flush LSN. > > > > Flush LSN is this - flushLSN = walsndctl->lsn[SYNC_REP_WAIT_FLUSH]; > > which gets updated in SyncRepReleaseWaiters. > > > > Async standbys with their SendRqstPtr will wait in XLogSendPhysical or > > XLogSendLogical until SendRqstPtr <= flushLSN. > > My feedback is specifically about this behavior. I don't think we should > spin in XLogSend*() waiting for an LSN to be synchronously replicated. I > think we should just choose the SendRqstPtr based on what is currently > synchronously replicated. Do you mean something like the following? /* Main loop of walsender process that streams the WAL over Copy messages. */ static void WalSndLoop(WalSndSendDataCallback send_data) { /* * Loop until we reach the end of this timeline or the client requests to * stop streaming. */ for (;;) { if (am_async_walsender && there_are_sync_standbys) { XLogRecPtr SendRqstLSN; XLogRecPtr SyncFlushLSN; SendRqstLSN = GetFlushRecPtr(NULL); LWLockAcquire(SyncRepLock, LW_SHARED); SyncFlushLSN = walsndctl->lsn[SYNC_REP_WAIT_FLUSH]; LWLockRelease(SyncRepLock); if (SendRqstLSN > SyncFlushLSN) continue; } if (!pq_is_send_pending()) send_data(); /* THIS IS WHERE XLogSendPhysical or XLogSendLogical gets called */ else WalSndCaughtUp = false; } Regards, Bharath Rupireddy. ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Allow async standbys wait for sync replication (was: Disallow quorum uncommitted (with synchronous standbys) txns in logical replication subscribers) @ 2022-03-01 06:05 Nathan Bossart <[email protected]> parent: Bharath Rupireddy <[email protected]> 0 siblings, 1 reply; 13+ messages in thread From: Nathan Bossart @ 2022-03-01 06:05 UTC (permalink / raw) To: Bharath Rupireddy <[email protected]>; +Cc: SATYANARAYANA NARLAPURAM <[email protected]>; PostgreSQL Hackers <[email protected]> On Tue, Mar 01, 2022 at 11:10:09AM +0530, Bharath Rupireddy wrote: > On Tue, Mar 1, 2022 at 12:27 AM Nathan Bossart <[email protected]> wrote: >> My feedback is specifically about this behavior. I don't think we should >> spin in XLogSend*() waiting for an LSN to be synchronously replicated. I >> think we should just choose the SendRqstPtr based on what is currently >> synchronously replicated. > > Do you mean something like the following? > > /* Main loop of walsender process that streams the WAL over Copy messages. */ > static void > WalSndLoop(WalSndSendDataCallback send_data) > { > /* > * Loop until we reach the end of this timeline or the client requests to > * stop streaming. > */ > for (;;) > { > if (am_async_walsender && there_are_sync_standbys) > { > XLogRecPtr SendRqstLSN; > XLogRecPtr SyncFlushLSN; > > SendRqstLSN = GetFlushRecPtr(NULL); > LWLockAcquire(SyncRepLock, LW_SHARED); > SyncFlushLSN = walsndctl->lsn[SYNC_REP_WAIT_FLUSH]; > LWLockRelease(SyncRepLock); > > if (SendRqstLSN > SyncFlushLSN) > continue; > } Not quite. Instead of "continue", I would set SendRqstLSN to SyncFlushLSN so that the WAL sender only sends up to the current synchronously replicated LSN. TBH there are probably other things that need to be considered (e.g., how do we ensure that the WAL sender sends the rest once it is replicated?), but I still think we should avoid spinning in the WAL sender waiting for WAL to be replicated. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Allow async standbys wait for sync replication @ 2022-03-01 07:34 Kyotaro Horiguchi <[email protected]> parent: Nathan Bossart <[email protected]> 0 siblings, 1 reply; 13+ messages in thread From: Kyotaro Horiguchi @ 2022-03-01 07:34 UTC (permalink / raw) To: [email protected]; +Cc: [email protected]; [email protected]; [email protected] (Now I understand what "async" mean here..) At Mon, 28 Feb 2022 22:05:28 -0800, Nathan Bossart <[email protected]> wrote in > On Tue, Mar 01, 2022 at 11:10:09AM +0530, Bharath Rupireddy wrote: > > On Tue, Mar 1, 2022 at 12:27 AM Nathan Bossart <[email protected]> wrote: > >> My feedback is specifically about this behavior. I don't think we should > >> spin in XLogSend*() waiting for an LSN to be synchronously replicated. I > >> think we should just choose the SendRqstPtr based on what is currently > >> synchronously replicated. > > > > Do you mean something like the following? > > > > /* Main loop of walsender process that streams the WAL over Copy messages. */ > > static void > > WalSndLoop(WalSndSendDataCallback send_data) > > { > > /* > > * Loop until we reach the end of this timeline or the client requests to > > * stop streaming. > > */ > > for (;;) > > { > > if (am_async_walsender && there_are_sync_standbys) > > { > > XLogRecPtr SendRqstLSN; > > XLogRecPtr SyncFlushLSN; > > > > SendRqstLSN = GetFlushRecPtr(NULL); > > LWLockAcquire(SyncRepLock, LW_SHARED); > > SyncFlushLSN = walsndctl->lsn[SYNC_REP_WAIT_FLUSH]; > > LWLockRelease(SyncRepLock); > > > > if (SendRqstLSN > SyncFlushLSN) > > continue; > > } The current trend is energy-savings. We never add a "wait for some fixed time then exit if the condition makes, otherwise repeat" loop for this kind of purpose where there's no guarantee that the loop exits quite shortly. Concretely we ought to rely on condition variables to do that. > Not quite. Instead of "continue", I would set SendRqstLSN to SyncFlushLSN > so that the WAL sender only sends up to the current synchronously I'm not sure, but doesn't that makes walsender falsely believes it have caught up to the bleeding edge of WAL? > replicated LSN. TBH there are probably other things that need to be > considered (e.g., how do we ensure that the WAL sender sends the rest once > it is replicated?), but I still think we should avoid spinning in the WAL > sender waiting for WAL to be replicated. It seems to me it would be something similar to SyncRepReleaseWaiters(). Or it could be possible to consolidate this feature into the function, I'm not sure, though. regards. -- Kyotaro Horiguchi NTT Open Source Software Center ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Allow async standbys wait for sync replication @ 2022-03-01 17:05 Nathan Bossart <[email protected]> parent: Kyotaro Horiguchi <[email protected]> 0 siblings, 1 reply; 13+ messages in thread From: Nathan Bossart @ 2022-03-01 17:05 UTC (permalink / raw) To: Kyotaro Horiguchi <[email protected]>; +Cc: [email protected]; [email protected]; [email protected] On Tue, Mar 01, 2022 at 04:34:31PM +0900, Kyotaro Horiguchi wrote: > At Mon, 28 Feb 2022 22:05:28 -0800, Nathan Bossart <[email protected]> wrote in >> replicated LSN. TBH there are probably other things that need to be >> considered (e.g., how do we ensure that the WAL sender sends the rest once >> it is replicated?), but I still think we should avoid spinning in the WAL >> sender waiting for WAL to be replicated. > > It seems to me it would be something similar to > SyncRepReleaseWaiters(). Or it could be possible to consolidate this > feature into the function, I'm not sure, though. Yes, perhaps the synchronous replication framework will need to alert WAL senders when the syncrep LSN advances so that the WAL is sent to the async standbys. I'm glossing over the details, but I think that should be the general direction. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Allow async standbys wait for sync replication @ 2022-03-01 17:39 Bharath Rupireddy <[email protected]> parent: Nathan Bossart <[email protected]> 0 siblings, 1 reply; 13+ messages in thread From: Bharath Rupireddy @ 2022-03-01 17:39 UTC (permalink / raw) To: Nathan Bossart <[email protected]>; +Cc: Kyotaro Horiguchi <[email protected]>; SATYANARAYANA NARLAPURAM <[email protected]>; PostgreSQL Hackers <[email protected]> On Tue, Mar 1, 2022 at 10:35 PM Nathan Bossart <[email protected]> wrote: > > On Tue, Mar 01, 2022 at 04:34:31PM +0900, Kyotaro Horiguchi wrote: > > At Mon, 28 Feb 2022 22:05:28 -0800, Nathan Bossart <[email protected]> wrote in > >> replicated LSN. TBH there are probably other things that need to be > >> considered (e.g., how do we ensure that the WAL sender sends the rest once > >> it is replicated?), but I still think we should avoid spinning in the WAL > >> sender waiting for WAL to be replicated. > > > > It seems to me it would be something similar to > > SyncRepReleaseWaiters(). Or it could be possible to consolidate this > > feature into the function, I'm not sure, though. > > Yes, perhaps the synchronous replication framework will need to alert WAL > senders when the syncrep LSN advances so that the WAL is sent to the async > standbys. I'm glossing over the details, but I think that should be the > general direction. It's doable. But we can't avoid async walsenders waiting for the flush LSN even if we take the SyncRepReleaseWaiters() approach right? I'm not sure (at this moment) what's the biggest advantage of this approach i.e. (1) backends waking up walsenders after flush lsn is updated vs (2) walsenders keep looking for the new flush lsn. > >> My feedback is specifically about this behavior. I don't think we should > >> spin in XLogSend*() waiting for an LSN to be synchronously replicated. I > >> think we should just choose the SendRqstPtr based on what is currently > >> synchronously replicated. > > > > Do you mean something like the following? > > > > /* Main loop of walsender process that streams the WAL over Copy messages. */ > > static void > > WalSndLoop(WalSndSendDataCallback send_data) > > { > > /* > > * Loop until we reach the end of this timeline or the client requests to > > * stop streaming. > > */ > > for (;;) > > { > > if (am_async_walsender && there_are_sync_standbys) > > { > > XLogRecPtr SendRqstLSN; > > XLogRecPtr SyncFlushLSN; > > > > SendRqstLSN = GetFlushRecPtr(NULL); > > LWLockAcquire(SyncRepLock, LW_SHARED); > > SyncFlushLSN = walsndctl->lsn[SYNC_REP_WAIT_FLUSH]; > > LWLockRelease(SyncRepLock); > > > > if (SendRqstLSN > SyncFlushLSN) > > continue; > > } > > Not quite. Instead of "continue", I would set SendRqstLSN to SyncFlushLSN > so that the WAL sender only sends up to the current synchronously > replicated LSN. TBH there are probably other things that need to be > considered (e.g., how do we ensure that the WAL sender sends the rest once > it is replicated?), but I still think we should avoid spinning in the WAL > sender waiting for WAL to be replicated. I did some more analysis on the above point: we can let XLogSendPhysical know up to which it can send WAL (SendRqstLSN). But, XLogSendLogical reads the WAL using XLogReadRecord mechanism with read_local_xlog_page page_read callback to which we can't really say SendRqstLSN. May be we have to do something like below: XLogSendPhysical: /* Figure out how far we can safely send the WAL. */ if (am_async_walsender && there_are_sync_standbys) { LWLockAcquire(SyncRepLock, LW_SHARED); SendRqstPtr = WalSndCtl->lsn[SYNC_REP_WAIT_FLUSH]; LWLockRelease(SyncRepLock); } /* Existing code path to determine SendRqstPtr */ else if (sendTimeLineIsHistoric) { } else if (am_cascading_walsender) { } else { /* * Streaming the current timeline on a primary. } XLogSendLogical: if (am_async_walsender && there_are_sync_standbys) { XLogRecPtr SendRqstLSN; XLogRecPtr SyncFlushLSN; SendRqstLSN = GetFlushRecPtr(NULL); LWLockAcquire(SyncRepLock, LW_SHARED); SyncFlushLSN = WalSndCtl->lsn[SYNC_REP_WAIT_FLUSH]; LWLockRelease(SyncRepLock); if (SendRqstLSN > SyncFlushLSN) return; } On Tue, Mar 1, 2022 at 7:35 AM Hsu, John <[email protected]> wrote: > > I too observed this once or twice. It looks like the walsender isn't > > detecting postmaster death in for (;;) with WalSndWait. Not sure if > > this is expected or true with other wait-loops in walsender code. Any > > more thoughts here? > > Unfortunately I haven't had a chance to dig into it more although iirc I hit it fairly often. I think I got what the issue is. Below does the trick. if (got_STOPPING) proc_exit(0); * If the server is shut down, checkpointer sends us * PROCSIG_WALSND_INIT_STOPPING after all regular backends have exited. I will take care of this in the next patch once the approach we take for this feature gets finalized. Regards, Bharath Rupireddy. ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Allow async standbys wait for sync replication @ 2022-03-01 21:27 Nathan Bossart <[email protected]> parent: Bharath Rupireddy <[email protected]> 0 siblings, 1 reply; 13+ messages in thread From: Nathan Bossart @ 2022-03-01 21:27 UTC (permalink / raw) To: Bharath Rupireddy <[email protected]>; +Cc: Kyotaro Horiguchi <[email protected]>; SATYANARAYANA NARLAPURAM <[email protected]>; PostgreSQL Hackers <[email protected]> On Tue, Mar 01, 2022 at 11:09:57PM +0530, Bharath Rupireddy wrote: > On Tue, Mar 1, 2022 at 10:35 PM Nathan Bossart <[email protected]> wrote: >> Yes, perhaps the synchronous replication framework will need to alert WAL >> senders when the syncrep LSN advances so that the WAL is sent to the async >> standbys. I'm glossing over the details, but I think that should be the >> general direction. > > It's doable. But we can't avoid async walsenders waiting for the flush > LSN even if we take the SyncRepReleaseWaiters() approach right? I'm > not sure (at this moment) what's the biggest advantage of this > approach i.e. (1) backends waking up walsenders after flush lsn is > updated vs (2) walsenders keep looking for the new flush lsn. I think there are a couple of advantages. For one, spinning is probably not the best from a resource perspective. There is no guarantee that the desired SendRqstPtr will ever be synchronously replicated, in which case the WAL sender would spin forever. Also, this approach might fit in better with the existing synchronous replication framework. When a WAL sender realizes that it can't send up to the current "flush" LSN because it's not synchronously replicated, it will request to be alerted when it is. In the meantime, it can send up to the latest syncrep LSN so that the async standby is as up-to-date as possible. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Allow async standbys wait for sync replication @ 2022-03-02 04:17 Bharath Rupireddy <[email protected]> parent: Nathan Bossart <[email protected]> 0 siblings, 1 reply; 13+ messages in thread From: Bharath Rupireddy @ 2022-03-02 04:17 UTC (permalink / raw) To: Nathan Bossart <[email protected]>; +Cc: Kyotaro Horiguchi <[email protected]>; SATYANARAYANA NARLAPURAM <[email protected]>; PostgreSQL Hackers <[email protected]> On Wed, Mar 2, 2022 at 2:57 AM Nathan Bossart <[email protected]> wrote: > > On Tue, Mar 01, 2022 at 11:09:57PM +0530, Bharath Rupireddy wrote: > > On Tue, Mar 1, 2022 at 10:35 PM Nathan Bossart <[email protected]> wrote: > >> Yes, perhaps the synchronous replication framework will need to alert WAL > >> senders when the syncrep LSN advances so that the WAL is sent to the async > >> standbys. I'm glossing over the details, but I think that should be the > >> general direction. > > > > It's doable. But we can't avoid async walsenders waiting for the flush > > LSN even if we take the SyncRepReleaseWaiters() approach right? I'm > > not sure (at this moment) what's the biggest advantage of this > > approach i.e. (1) backends waking up walsenders after flush lsn is > > updated vs (2) walsenders keep looking for the new flush lsn. > > I think there are a couple of advantages. For one, spinning is probably > not the best from a resource perspective. Just to be on the same page - by spinning do you mean - the async walsender waiting for the sync flushLSN in a for-loop with WaitLatch()? > There is no guarantee that the > desired SendRqstPtr will ever be synchronously replicated, in which case > the WAL sender would spin forever. The async walsenders will not exactly wait for SendRqstPtr LSN to be the flush lsn. Say, SendRqstPtr is 100 and the current sync FlushLSN is 95, they will have to wait until FlushLSN moves ahead of SendRqstPtr i.e. SendRqstPtr <= FlushLSN. I can't think of a scenario (right now) that doesn't move the sync FlushLSN at all. If there's such a scenario, shouldn't it be treated as a sync replication bug? > Also, this approach might fit in better > with the existing synchronous replication framework. When a WAL sender > realizes that it can't send up to the current "flush" LSN because it's not > synchronously replicated, it will request to be alerted when it is. I think you are referring to the way a backend calls SyncRepWaitForLSN and waits until any one of the walsender sets syncRepState to SYNC_REP_WAIT_COMPLETE in SyncRepWakeQueue. Firstly, SyncRepWaitForLSN blocking i.e. the backend spins/waits in for (;;) loop until its syncRepState becomes SYNC_REP_WAIT_COMPLETE. The backend doesn't do any other work but waits. So, spinning isn't avoided completely. Unless, I'm missing something, the existing syc repl queue (SyncRepQueue) mechanism doesn't avoid spinning in the requestors (backends) SyncRepWaitForLSN or in the walsenders SyncRepWakeQueue. > In the > meantime, it can send up to the latest syncrep LSN so that the async > standby is as up-to-date as possible. Just to be clear, there can exist the following scenarios: Firstly, SendRqstPtr is up to which a walsender can send WAL, it's not the scenario 1: async SendRqstPtr is 100, sync FlushLSN is 95 - async standbys will wait until the FlushLSN moves ahead, once SendRqstPtr <= FlushLSN, it sends out the WAL. scenario 2: async SendRqstPtr is 105, sync FlushLSN is 110 - async standbys will not wait, it just sends out the WAL up to SendRqstPtr i.e. LSN 105. scenario 3, same as scenario 2 but SendRqstPtr and FlushLSN is same: async SendRqstPtr is 105, sync FlushLSN is 105 - async standbys will not wait, it just sends out the WAL up to SendRqstPtr i.e. LSN 105. This way, the async standbys are always as up-to-date as possible with the sync FlushLSN. Are you referring to any other scenarios? Regards, Bharath Rupireddy. ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Allow async standbys wait for sync replication @ 2022-03-04 19:56 Nathan Bossart <[email protected]> parent: Bharath Rupireddy <[email protected]> 0 siblings, 1 reply; 13+ messages in thread From: Nathan Bossart @ 2022-03-04 19:56 UTC (permalink / raw) To: Bharath Rupireddy <[email protected]>; +Cc: Kyotaro Horiguchi <[email protected]>; SATYANARAYANA NARLAPURAM <[email protected]>; PostgreSQL Hackers <[email protected]> On Wed, Mar 02, 2022 at 09:47:09AM +0530, Bharath Rupireddy wrote: > On Wed, Mar 2, 2022 at 2:57 AM Nathan Bossart <[email protected]> wrote: >> I think there are a couple of advantages. For one, spinning is probably >> not the best from a resource perspective. > > Just to be on the same page - by spinning do you mean - the async > walsender waiting for the sync flushLSN in a for-loop with > WaitLatch()? Yes. >> Also, this approach might fit in better >> with the existing synchronous replication framework. When a WAL sender >> realizes that it can't send up to the current "flush" LSN because it's not >> synchronously replicated, it will request to be alerted when it is. > > I think you are referring to the way a backend calls SyncRepWaitForLSN > and waits until any one of the walsender sets syncRepState to > SYNC_REP_WAIT_COMPLETE in SyncRepWakeQueue. Firstly, SyncRepWaitForLSN > blocking i.e. the backend spins/waits in for (;;) loop until its > syncRepState becomes SYNC_REP_WAIT_COMPLETE. The backend doesn't do > any other work but waits. So, spinning isn't avoided completely. > > Unless, I'm missing something, the existing syc repl queue > (SyncRepQueue) mechanism doesn't avoid spinning in the requestors > (backends) SyncRepWaitForLSN or in the walsenders SyncRepWakeQueue. My point is that there are existing tools for alerting processes when an LSN is synchronously replicated and for waking up WAL senders. What I am proposing wouldn't involve spinning in XLogSendPhysical() waiting for synchronous replication. Like SyncRepWaitForLSN(), we'd register our LSN in the queue (SyncRepQueueInsert()), but we wouldn't sit in a separate loop waiting to be woken. Instead, SyncRepWakeQueue() would eventually wake up the WAL sender and trigger another iteration of WalSndLoop(). -- Nathan Bossart Amazon Web Services: https://aws.amazon.com ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Allow async standbys wait for sync replication @ 2022-03-17 17:05 Bharath Rupireddy <[email protected]> parent: Nathan Bossart <[email protected]> 0 siblings, 0 replies; 13+ messages in thread From: Bharath Rupireddy @ 2022-03-17 17:05 UTC (permalink / raw) To: Nathan Bossart <[email protected]>; +Cc: Kyotaro Horiguchi <[email protected]>; SATYANARAYANA NARLAPURAM <[email protected]>; PostgreSQL Hackers <[email protected]> On Sat, Mar 5, 2022 at 1:26 AM Nathan Bossart <[email protected]> wrote: > > My point is that there are existing tools for alerting processes when an > LSN is synchronously replicated and for waking up WAL senders. What I am > proposing wouldn't involve spinning in XLogSendPhysical() waiting for > synchronous replication. Like SyncRepWaitForLSN(), we'd register our LSN > in the queue (SyncRepQueueInsert()), but we wouldn't sit in a separate loop > waiting to be woken. Instead, SyncRepWakeQueue() would eventually wake up > the WAL sender and trigger another iteration of WalSndLoop(). While we continue to discuss the other better design at [1], FWIW, I would like to share a simpler patch that lets wal senders serving async standbys wait until sync standbys report the flush lsn. Obviously this is not an elegant way to solve the problem reported in this thread, as I have this patch ready long back, I wanted to share it here. Nathan, of course, this is not something you wanted. [1] https://www.postgresql.org/message-id/CALj2ACWCj60g6TzYMbEO07ZhnBGbdCveCrD413udqbRM0O59RA%40mail.gma... Regards, Bharath Rupireddy. Attachments: [application/x-patch] v2-0001-Allow-async-standbys-wait-for-sync-replication.patch (12.2K, ../../CALj2ACV=M6OpY4Ugf8hxa2+=EpRvv_Mh3W93Xr2gBCQLweWn0Q@mail.gmail.com/2-v2-0001-Allow-async-standbys-wait-for-sync-replication.patch) download | inline diff: From 8a74dbd8c3fe3631b6a2ecee3d8c7a1c98429341 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy <[email protected]> Date: Thu, 17 Mar 2022 17:01:53 +0000 Subject: [PATCH v2] Allow async standbys wait for sync replication --- doc/src/sgml/config.sgml | 22 +++ src/backend/replication/syncrep.c | 11 +- src/backend/replication/walsender.c | 157 ++++++++++++++++++ src/backend/utils/misc/guc.c | 9 + src/backend/utils/misc/postgresql.conf.sample | 3 + src/include/replication/syncrep.h | 7 + src/include/replication/walsender.h | 1 + 7 files changed, 202 insertions(+), 8 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 7a48973b3c..026e12f5ef 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -4247,6 +4247,28 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows </listitem> </varlistentry> + <varlistentry id="guc-async-standbys-wait-for-sync_replication" xreflabel="async_standbys_wait_for_sync_replication"> + <term><varname>async_standbys_wait_for_sync_replication</varname> (<type>boolean</type>) + <indexterm> + <primary><varname>async_standbys_wait_for_sync_replication</varname> configuration parameter</primary> + </indexterm> + </term> + <listitem> + <para> + When set, asynchronous standbys will wait until synchronous standbys + (that are either in quorum or priority based) receive WAL, flush and + acknowledge primary with the flush LSN. This behvaiour is particularly + important in the event of failover as it avoids manual steps required + on the asynchronous standbys that might go ahead of the synchronous + standbys. + </para> + <para> + This parameter can only be set in the <filename>postgresql.conf</filename> + file or on the server command line. + </para> + </listitem> + </varlistentry> + </variablelist> </sect2> diff --git a/src/backend/replication/syncrep.c b/src/backend/replication/syncrep.c index ce163b99e9..398c11579c 100644 --- a/src/backend/replication/syncrep.c +++ b/src/backend/replication/syncrep.c @@ -97,8 +97,6 @@ static bool announce_next_takeover = true; SyncRepConfigData *SyncRepConfig = NULL; static int SyncRepWaitMode = SYNC_REP_NO_WAIT; -static void SyncRepQueueInsert(int mode); -static void SyncRepCancelWait(void); static int SyncRepWakeQueue(bool all, int mode); static bool SyncRepGetSyncRecPtr(XLogRecPtr *writePtr, @@ -120,9 +118,6 @@ static int SyncRepGetStandbyPriority(void); static int standby_priority_comparator(const void *a, const void *b); static int cmp_lsn(const void *a, const void *b); -#ifdef USE_ASSERT_CHECKING -static bool SyncRepQueueIsOrderedByLSN(int mode); -#endif /* * =========================================================== @@ -335,7 +330,7 @@ SyncRepWaitForLSN(XLogRecPtr lsn, bool commit) * Usually we will go at tail of queue, though it's possible that we arrive * here out of order, so start at tail and work back to insertion point. */ -static void +void SyncRepQueueInsert(int mode) { PGPROC *proc; @@ -368,7 +363,7 @@ SyncRepQueueInsert(int mode) /* * Acquire SyncRepLock and cancel any wait currently in progress. */ -static void +void SyncRepCancelWait(void) { LWLockAcquire(SyncRepLock, LW_EXCLUSIVE); @@ -979,7 +974,7 @@ SyncRepUpdateSyncStandbysDefined(void) } #ifdef USE_ASSERT_CHECKING -static bool +bool SyncRepQueueIsOrderedByLSN(int mode) { PGPROC *proc = NULL; diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c index 2d0292a092..97d32a6294 100644 --- a/src/backend/replication/walsender.c +++ b/src/backend/replication/walsender.c @@ -125,6 +125,8 @@ int wal_sender_timeout = 60 * 1000; /* maximum time to send one WAL * data message */ bool log_replication_commands = false; +bool async_standbys_wait_for_sync_replication = true; + /* * State for WalSndWakeupRequest */ @@ -258,6 +260,8 @@ static bool TransactionIdInRecentPast(TransactionId xid, uint32 epoch); static void WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo, TimeLineID *tli_p); +/* called by wal sender serving asynchronous standby */ +static void AsynWalSndWaitForSyncRepLSN(XLogRecPtr lsn); /* Initialize walsender process before entering the main command loop */ void @@ -2771,6 +2775,8 @@ XLogSendPhysical(void) SendRqstPtr = GetFlushRecPtr(NULL); } + AsynWalSndWaitForSyncRepLSN(SendRqstPtr); + /* * Record the current system time as an approximation of the time at which * this WAL location was written for the purposes of lag tracking. @@ -2995,6 +3001,22 @@ XLogSendLogical(void) if (record != NULL) { + /* + * At this point, we do not know whether the current LSN (ReadRecPtr) + * is required by any of the logical decoding output plugins which is + * only known at the plugin level. If we were to decide whether to wait + * or not for the synchronous standbys flush LSN at the plugin level, + * we might have to pass extra information to it which doesn't sound an + * elegant way. + * + * Another way the output plugins can wait there before sending the WAL + * is by reading the flush LSN from the logical replication slots. + * + * Waiting here i.e. before even the logical decoding kicks in, makes + * the code clean. + */ + AsynWalSndWaitForSyncRepLSN(logical_decoding_ctx->reader->ReadRecPtr); + /* * Note the lack of any call to LagTrackerWrite() which is handled by * WalSndUpdateProgress which is called by output plugin through @@ -3789,3 +3811,138 @@ LagTrackerRead(int head, XLogRecPtr lsn, TimestampTz now) Assert(time != 0); return now - time; } + +/* + * This function is similar to SyncRepWaitForLSN() in syncrep.c. Only + * difference is in the way it waits with WalSndWait and exits when + * got_STOPPING or got_SIGUSR2 is set. + */ +void +AsynWalSndWaitForSyncRepLSN(XLogRecPtr lsn) +{ + int mode; + + /* + * Fast exit in case we are told to not wait. + */ + if (!async_standbys_wait_for_sync_replication) + return; + + /* + * Fast exit in case the wal sender is serving synchronous standby at the + * moment as it has no business here. + */ + if (MyWalSnd->sync_standby_priority > 0) + return; + + /* + * Fast exit if user has not requested sync replication, or there are no + * sync replication standby names defined. + * + * Since this routine gets called every commit time, it's important to + * exit quickly if sync replication is not requested. So we check + * WalSndCtl->sync_standbys_defined flag without the lock and exit + * immediately if it's false. If it's true, we need to check it again + * later while holding the lock, to check the flag and operate the sync + * rep queue atomically. This is necessary to avoid the race condition + * described in SyncRepUpdateSyncStandbysDefined(). On the other hand, if + * it's false, the lock is not necessary because we don't touch the queue. + */ + if (!SyncRepRequested() || + !((volatile WalSndCtlData *) WalSndCtl)->sync_standbys_defined) + return; + + mode = SYNC_REP_WAIT_FLUSH; + + Assert(SHMQueueIsDetached(&(MyProc->syncRepLinks))); + Assert(WalSndCtl != NULL); + + LWLockAcquire(SyncRepLock, LW_EXCLUSIVE); + Assert(MyProc->syncRepState == SYNC_REP_NOT_WAITING); + + /* + * We don't wait for sync rep if WalSndCtl->sync_standbys_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 (!WalSndCtl->sync_standbys_defined || + lsn <= WalSndCtl->lsn[mode]) + { + LWLockRelease(SyncRepLock); + return; + } + + /* + * Set our waitLSN so WALSender will know when to wake us, and add + * ourselves to the queue. + */ + MyProc->waitLSN = lsn; + MyProc->syncRepState = SYNC_REP_WAITING; + SyncRepQueueInsert(mode); + Assert(SyncRepQueueIsOrderedByLSN(mode)); + LWLockRelease(SyncRepLock); + + /* + * Wait for specified LSN to be confirmed. + * + * Each proc has its own wait latch, so we perform a normal latch + * check/wait loop here. + */ + for (;;) + { + /* Must reset the latch before testing state. */ + ResetLatch(MyLatch); + + CHECK_FOR_INTERRUPTS(); + + /* Process any requests or signals received recently */ + if (ConfigReloadPending) + { + ConfigReloadPending = false; + ProcessConfigFile(PGC_SIGHUP); + SyncRepInitConfig(); + } + + if (!async_standbys_wait_for_sync_replication) + break; + + if (MyWalSnd->sync_standby_priority > 0) + break; + + /* + * Acquiring the lock is not needed, the latch ensures proper + * barriers. If it looks like we're done, we must really be done, + * because once walsender changes the state to SYNC_REP_WAIT_COMPLETE, + * it will never update it again, so we can't be seeing a stale value + * in that case. + */ + if (MyProc->syncRepState == SYNC_REP_WAIT_COMPLETE) + break; + + /* Sleep until something happens or we time out */ + WalSndWait(WL_SOCKET_WRITEABLE | WL_SOCKET_READABLE, -1, + WAIT_EVENT_SYNC_REP); + + if (got_STOPPING || got_SIGUSR2) + { + SyncRepCancelWait(); + break; + } + } + + /* + * WalSender has checked our LSN and has removed us from queue. Clean up + * state and leave. It's OK to reset these shared memory fields without + * holding SyncRepLock, because any walsenders will ignore us anyway when + * we're not on the queue. We need a read barrier to make sure we see the + * changes to the queue link (this might be unnecessary without + * assertions, but better safe than sorry). + */ + pg_read_barrier(); + Assert(SHMQueueIsDetached(&(MyProc->syncRepLinks))); + MyProc->syncRepState = SYNC_REP_NOT_WAITING; + MyProc->waitLSN = 0; +} diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index e7f0a380e6..cd5f8b5a42 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -1388,6 +1388,15 @@ static struct config_bool ConfigureNamesBool[] = false, NULL, NULL, NULL }, + { + {"async_standbys_wait_for_sync_replication", PGC_SIGHUP, REPLICATION_SENDING, + gettext_noop("Sets whether asynchronous standbys should wait until synchronous standbys receive and flush WAL."), + NULL + }, + &async_standbys_wait_for_sync_replication, + true, + NULL, NULL, NULL + }, { {"debug_assertions", PGC_INTERNAL, PRESET_OPTIONS, gettext_noop("Shows whether the running server has assertion checks enabled."), diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index 4cf5b26a36..9e6dc5082b 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -308,6 +308,9 @@ #wal_sender_timeout = 60s # in milliseconds; 0 disables #track_commit_timestamp = off # collect timestamp of transaction commit # (change requires restart) +#async_standbys_wait_for_sync_replication = on # Specifies whether asynchronous + # standbys should wait until synchronous standbys receive and + # flush WAL # - Primary Server - diff --git a/src/include/replication/syncrep.h b/src/include/replication/syncrep.h index 27be230d77..b433528174 100644 --- a/src/include/replication/syncrep.h +++ b/src/include/replication/syncrep.h @@ -90,6 +90,13 @@ extern void SyncRepCleanupAtProcExit(void); /* called by wal sender */ extern void SyncRepInitConfig(void); extern void SyncRepReleaseWaiters(void); +extern void SyncRepQueueInsert(int mode); +extern void SyncRepCancelWait(void); + +#ifdef USE_ASSERT_CHECKING +extern bool SyncRepQueueIsOrderedByLSN(int mode); +#endif + /* called by wal sender and user backend */ extern int SyncRepGetCandidateStandbys(SyncRepStandbyData **standbys); diff --git a/src/include/replication/walsender.h b/src/include/replication/walsender.h index b1892e9e4b..61b113ae48 100644 --- a/src/include/replication/walsender.h +++ b/src/include/replication/walsender.h @@ -34,6 +34,7 @@ extern bool wake_wal_senders; extern int max_wal_senders; extern int wal_sender_timeout; extern bool log_replication_commands; +extern bool async_standbys_wait_for_sync_replication; extern void InitWalSender(void); extern bool exec_replication_command(const char *query_string); -- 2.25.1 ^ permalink raw reply [nested|flat] 13+ messages in thread
* [PATCH v1] Shorten pg_attribute_always_inline to pg_always_inline @ 2026-04-08 21:07 Andres Freund <[email protected]> 0 siblings, 0 replies; 13+ messages in thread From: Andres Freund @ 2026-04-08 21:07 UTC (permalink / raw) --- src/include/c.h | 8 +++--- src/include/executor/execScan.h | 8 +++--- src/include/portability/instr_time.h | 8 +++--- src/backend/access/heap/heapam.c | 2 +- src/backend/access/transam/xlog.c | 4 +-- src/backend/commands/copyfromparse.c | 32 ++++++++++++------------ src/backend/commands/copyto.c | 4 +-- src/backend/executor/execExprInterp.c | 36 +++++++++++++-------------- src/backend/executor/execTuples.c | 6 ++--- src/backend/executor/nodeHashjoin.c | 2 +- src/backend/executor/nodeSeqscan.c | 4 +-- src/backend/nodes/queryjumblefuncs.c | 6 ++--- src/backend/storage/buffer/bufmgr.c | 24 +++++++++--------- src/backend/utils/adt/json.c | 2 +- src/backend/utils/cache/catcache.c | 2 +- 15 files changed, 74 insertions(+), 74 deletions(-) diff --git a/src/include/c.h b/src/include/c.h index 88d13ec9993..ec74d78d72a 100644 --- a/src/include/c.h +++ b/src/include/c.h @@ -289,20 +289,20 @@ extern "C++" #endif /* - * Use "pg_attribute_always_inline" in place of "inline" for functions that + * Use "pg_always_inline" in place of "inline" for functions that * we wish to force inlining of, even when the compiler's heuristics would * choose not to. But, if possible, don't force inlining in unoptimized * debug builds. */ #if defined(__GNUC__) && defined(__OPTIMIZE__) /* GCC supports always_inline via __attribute__ */ -#define pg_attribute_always_inline __attribute__((always_inline)) inline +#define pg_always_inline __attribute__((always_inline)) inline #elif defined(_MSC_VER) /* MSVC has a special keyword for this */ -#define pg_attribute_always_inline __forceinline +#define pg_always_inline __forceinline #else /* Otherwise, the best we can do is to say "inline" */ -#define pg_attribute_always_inline inline +#define pg_always_inline inline #endif /* diff --git a/src/include/executor/execScan.h b/src/include/executor/execScan.h index 18b03235c3c..7f795b0b3fc 100644 --- a/src/include/executor/execScan.h +++ b/src/include/executor/execScan.h @@ -24,12 +24,12 @@ * This routine substitutes a test tuple if inside an EvalPlanQual recheck. * Otherwise, it simply executes the access method's next-tuple routine. * - * The pg_attribute_always_inline attribute allows the compiler to inline + * The pg_always_inline attribute allows the compiler to inline * this function into its caller. When EPQState is NULL, the EvalPlanQual * logic is completely eliminated at compile time, avoiding unnecessary * run-time checks and code for cases where EPQ is not required. */ -static pg_attribute_always_inline TupleTableSlot * +static pg_always_inline TupleTableSlot * ExecScanFetch(ScanState *node, EPQState *epqstate, ExecScanAccessMtd accessMtd, @@ -145,7 +145,7 @@ ExecScanFetch(ScanState *node, * conditions enforced by the access method. * * This function is an alternative to ExecScan, used when callers may omit - * 'qual' or 'projInfo'. The pg_attribute_always_inline attribute allows the + * 'qual' or 'projInfo'. The pg_always_inline attribute allows the * compiler to eliminate non-relevant branches at compile time, avoiding * run-time checks in those cases. * @@ -157,7 +157,7 @@ ExecScanFetch(ScanState *node, * positioned before the first qualifying tuple. * ---------------------------------------------------------------- */ -static pg_attribute_always_inline TupleTableSlot * +static pg_always_inline TupleTableSlot * ExecScanExtended(ScanState *node, ExecScanAccessMtd accessMtd, /* function returning a tuple */ ExecScanRecheckMtd recheckMtd, diff --git a/src/include/portability/instr_time.h b/src/include/portability/instr_time.h index 92558e234ac..b110008b747 100644 --- a/src/include/portability/instr_time.h +++ b/src/include/portability/instr_time.h @@ -368,7 +368,7 @@ pg_rdtscp(void) * only inlining the function partially. * See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=124795 */ -static pg_attribute_always_inline instr_time +static pg_always_inline instr_time pg_get_ticks(void) { if (likely(timing_tsc_enabled)) @@ -382,7 +382,7 @@ pg_get_ticks(void) return pg_get_ticks_system(); } -static pg_attribute_always_inline instr_time +static pg_always_inline instr_time pg_get_ticks_fast(void) { if (likely(timing_tsc_enabled)) @@ -398,13 +398,13 @@ pg_get_ticks_fast(void) #else -static pg_attribute_always_inline instr_time +static pg_always_inline instr_time pg_get_ticks(void) { return pg_get_ticks_system(); } -static pg_attribute_always_inline instr_time +static pg_always_inline instr_time pg_get_ticks_fast(void) { return pg_get_ticks_system(); diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index abfd8e8970a..4f373b86028 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -519,7 +519,7 @@ heap_setscanlimits(TableScanDesc sscan, BlockNumber startBlk, BlockNumber numBlk * multiple times, with constant arguments for all_visible, * check_serializable. */ -pg_attribute_always_inline +pg_always_inline static int page_collect_tuples(HeapScanDesc scan, Snapshot snapshot, Page page, Buffer buffer, diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index f85b5286086..d13c0353dce 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -1143,9 +1143,9 @@ XLogInsertRecord(XLogRecData *rdata, * * NB: Testing shows that XLogInsertRecord runs faster if this code is inlined; * however, because there are two call sites, the compiler is reluctant to - * inline. We use pg_attribute_always_inline here to try to convince it. + * inline. We use pg_always_inline here to try to convince it. */ -static pg_attribute_always_inline void +static pg_always_inline void ReserveXLogInsertLocation(int size, XLogRecPtr *StartPos, XLogRecPtr *EndPos, XLogRecPtr *PrevPtr) { diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c index 65fd5a0ab4f..500810577ad 100644 --- a/src/backend/commands/copyfromparse.c +++ b/src/backend/commands/copyfromparse.c @@ -144,22 +144,22 @@ static const char BinarySignature[11] = "PGCOPY\n\377\r\n\0"; /* non-export function prototypes */ static bool CopyReadLine(CopyFromState cstate, bool is_csv); -static pg_attribute_always_inline bool CopyReadLineText(CopyFromState cstate, - bool is_csv); +static pg_always_inline bool CopyReadLineText(CopyFromState cstate, + bool is_csv); static int CopyReadAttributesText(CopyFromState cstate); static int CopyReadAttributesCSV(CopyFromState cstate); static Datum CopyReadBinaryAttribute(CopyFromState cstate, FmgrInfo *flinfo, Oid typioparam, int32 typmod, bool *isnull); -static pg_attribute_always_inline bool CopyFromTextLikeOneRow(CopyFromState cstate, - ExprContext *econtext, - Datum *values, - bool *nulls, - bool is_csv); -static pg_attribute_always_inline bool NextCopyFromRawFieldsInternal(CopyFromState cstate, - char ***fields, - int *nfields, - bool is_csv); +static pg_always_inline bool CopyFromTextLikeOneRow(CopyFromState cstate, + ExprContext *econtext, + Datum *values, + bool *nulls, + bool is_csv); +static pg_always_inline bool NextCopyFromRawFieldsInternal(CopyFromState cstate, + char ***fields, + int *nfields, + bool is_csv); /* Low-level communications functions */ @@ -769,11 +769,11 @@ NextCopyFromRawFields(CopyFromState cstate, char ***fields, int *nfields) * * NOTE: force_not_null option are not applied to the returned fields. * - * We use pg_attribute_always_inline to reduce function call overhead + * We use pg_always_inline to reduce function call overhead * and to help compilers to optimize away the 'is_csv' condition when called * by internal functions such as CopyFromTextLikeOneRow(). */ -static pg_attribute_always_inline bool +static pg_always_inline bool NextCopyFromRawFieldsInternal(CopyFromState cstate, char ***fields, int *nfields, bool is_csv) { int fldct; @@ -946,10 +946,10 @@ CopyFromCSVOneRow(CopyFromState cstate, ExprContext *econtext, Datum *values, /* * Workhorse for CopyFromTextOneRow() and CopyFromCSVOneRow(). * - * We use pg_attribute_always_inline to reduce function call overhead + * We use pg_always_inline to reduce function call overhead * and to help compilers to optimize away the 'is_csv' condition. */ -static pg_attribute_always_inline bool +static pg_always_inline bool CopyFromTextLikeOneRow(CopyFromState cstate, ExprContext *econtext, Datum *values, bool *nulls, bool is_csv) { @@ -1463,7 +1463,7 @@ CopyReadLineTextSIMDHelper(CopyFromState cstate, bool is_csv, /* * CopyReadLineText - inner loop of CopyReadLine for text mode */ -static pg_attribute_always_inline bool +static pg_always_inline bool CopyReadLineText(CopyFromState cstate, bool is_csv) { char *copy_input_buf; diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c index f0e0147c665..2d91ca86b3d 100644 --- a/src/backend/commands/copyto.c +++ b/src/backend/commands/copyto.c @@ -294,10 +294,10 @@ CopyToCSVOneRow(CopyToState cstate, TupleTableSlot *slot) /* * Workhorse for CopyToTextOneRow() and CopyToCSVOneRow(). * - * We use pg_attribute_always_inline to reduce function call overhead + * We use pg_always_inline to reduce function call overhead * and to help compilers to optimize away the 'is_csv' condition. */ -static pg_attribute_always_inline void +static pg_always_inline void CopyToTextLikeOneRow(CopyToState cstate, TupleTableSlot *slot, bool is_csv) diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c index 3c4843cde86..2bb0a3d1f63 100644 --- a/src/backend/executor/execExprInterp.c +++ b/src/backend/executor/execExprInterp.c @@ -178,16 +178,16 @@ static Datum ExecJustHashInnerVarVirt(ExprState *state, ExprContext *econtext, b static Datum ExecJustHashOuterVarStrict(ExprState *state, ExprContext *econtext, bool *isnull); /* execution helper functions */ -static pg_attribute_always_inline void ExecAggPlainTransByVal(AggState *aggstate, - AggStatePerTrans pertrans, - AggStatePerGroup pergroup, - ExprContext *aggcontext, - int setno); -static pg_attribute_always_inline void ExecAggPlainTransByRef(AggState *aggstate, - AggStatePerTrans pertrans, - AggStatePerGroup pergroup, - ExprContext *aggcontext, - int setno); +static pg_always_inline void ExecAggPlainTransByVal(AggState *aggstate, + AggStatePerTrans pertrans, + AggStatePerGroup pergroup, + ExprContext *aggcontext, + int setno); +static pg_always_inline void ExecAggPlainTransByRef(AggState *aggstate, + AggStatePerTrans pertrans, + AggStatePerGroup pergroup, + ExprContext *aggcontext, + int setno); static char *ExecGetJsonValueItemString(JsonbValue *item, bool *resnull); /* @@ -2544,7 +2544,7 @@ get_cached_rowtype(Oid type_id, int32 typmod, */ /* implementation of ExecJust(Inner|Outer|Scan)Var */ -static pg_attribute_always_inline Datum +static pg_always_inline Datum ExecJustVarImpl(ExprState *state, TupleTableSlot *slot, bool *isnull) { ExprEvalStep *op = &state->steps[1]; @@ -2582,7 +2582,7 @@ ExecJustScanVar(ExprState *state, ExprContext *econtext, bool *isnull) } /* implementation of ExecJustAssign(Inner|Outer|Scan)Var */ -static pg_attribute_always_inline Datum +static pg_always_inline Datum ExecJustAssignVarImpl(ExprState *state, TupleTableSlot *inslot, bool *isnull) { ExprEvalStep *op = &state->steps[1]; @@ -2677,7 +2677,7 @@ ExecJustConst(ExprState *state, ExprContext *econtext, bool *isnull) } /* implementation of ExecJust(Inner|Outer|Scan)VarVirt */ -static pg_attribute_always_inline Datum +static pg_always_inline Datum ExecJustVarVirtImpl(ExprState *state, TupleTableSlot *slot, bool *isnull) { ExprEvalStep *op = &state->steps[0]; @@ -2720,7 +2720,7 @@ ExecJustScanVarVirt(ExprState *state, ExprContext *econtext, bool *isnull) } /* implementation of ExecJustAssign(Inner|Outer|Scan)VarVirt */ -static pg_attribute_always_inline Datum +static pg_always_inline Datum ExecJustAssignVarVirtImpl(ExprState *state, TupleTableSlot *inslot, bool *isnull) { ExprEvalStep *op = &state->steps[0]; @@ -2799,7 +2799,7 @@ ExecJustHashInnerVarWithIV(ExprState *state, ExprContext *econtext, } /* implementation of ExecJustHash(Inner|Outer)Var */ -static pg_attribute_always_inline Datum +static pg_always_inline Datum ExecJustHashVarImpl(ExprState *state, TupleTableSlot *slot, bool *isnull) { ExprEvalStep *fetchop = &state->steps[0]; @@ -2837,7 +2837,7 @@ ExecJustHashInnerVar(ExprState *state, ExprContext *econtext, bool *isnull) } /* implementation of ExecJustHash(Inner|Outer)VarVirt */ -static pg_attribute_always_inline Datum +static pg_always_inline Datum ExecJustHashVarVirtImpl(ExprState *state, TupleTableSlot *slot, bool *isnull) { ExprEvalStep *var = &state->steps[0]; @@ -5836,7 +5836,7 @@ ExecEvalAggOrderedTransTuple(ExprState *state, ExprEvalStep *op, } /* implementation of transition function invocation for byval types */ -static pg_attribute_always_inline void +static pg_always_inline void ExecAggPlainTransByVal(AggState *aggstate, AggStatePerTrans pertrans, AggStatePerGroup pergroup, ExprContext *aggcontext, int setno) @@ -5868,7 +5868,7 @@ ExecAggPlainTransByVal(AggState *aggstate, AggStatePerTrans pertrans, } /* implementation of transition function invocation for byref types */ -static pg_attribute_always_inline void +static pg_always_inline void ExecAggPlainTransByRef(AggState *aggstate, AggStatePerTrans pertrans, AggStatePerGroup pergroup, ExprContext *aggcontext, int setno) diff --git a/src/backend/executor/execTuples.c b/src/backend/executor/execTuples.c index f08982a43cc..cb47b4fda1b 100644 --- a/src/backend/executor/execTuples.c +++ b/src/backend/executor/execTuples.c @@ -72,8 +72,8 @@ static TupleDesc ExecTypeFromTLInternal(List *targetList, bool skipjunk); -static pg_attribute_always_inline void slot_deform_heap_tuple(TupleTableSlot *slot, HeapTuple tuple, uint32 *offp, - int reqnatts, bool support_cstring); +static pg_always_inline void slot_deform_heap_tuple(TupleTableSlot *slot, HeapTuple tuple, uint32 *offp, + int reqnatts, bool support_cstring); static inline void tts_buffer_heap_store_tuple(TupleTableSlot *slot, HeapTuple tuple, Buffer buffer, @@ -1013,7 +1013,7 @@ tts_buffer_heap_store_tuple(TupleTableSlot *slot, HeapTuple tuple, * emit code during inlining for cstring deforming when it's required. * cstrings can exist in MinimalTuples, but not in HeapTuples. */ -static pg_attribute_always_inline void +static pg_always_inline void slot_deform_heap_tuple(TupleTableSlot *slot, HeapTuple tuple, uint32 *offp, int reqnatts, bool support_cstring) { diff --git a/src/backend/executor/nodeHashjoin.c b/src/backend/executor/nodeHashjoin.c index 0b365d5b475..202dd866251 100644 --- a/src/backend/executor/nodeHashjoin.c +++ b/src/backend/executor/nodeHashjoin.c @@ -221,7 +221,7 @@ static void ExecParallelHashJoinPartitionOuter(HashJoinState *hjstate); * the other one is "outer". * ---------------------------------------------------------------- */ -static pg_attribute_always_inline TupleTableSlot * +static pg_always_inline TupleTableSlot * ExecHashJoinImpl(PlanState *pstate, bool parallel) { HashJoinState *node = castNode(HashJoinState, pstate); diff --git a/src/backend/executor/nodeSeqscan.c b/src/backend/executor/nodeSeqscan.c index 5bcb0a861d7..b8c528ca089 100644 --- a/src/backend/executor/nodeSeqscan.c +++ b/src/backend/executor/nodeSeqscan.c @@ -48,7 +48,7 @@ static TupleTableSlot *SeqNext(SeqScanState *node); * This is a workhorse for ExecSeqScan * ---------------------------------------------------------------- */ -static pg_attribute_always_inline TupleTableSlot * +static pg_always_inline TupleTableSlot * SeqNext(SeqScanState *node) { TableScanDesc scandesc; @@ -95,7 +95,7 @@ SeqNext(SeqScanState *node) /* * SeqRecheck -- access method routine to recheck a tuple in EvalPlanQual */ -static pg_attribute_always_inline bool +static pg_always_inline bool SeqRecheck(SeqScanState *node, TupleTableSlot *slot) { /* diff --git a/src/backend/nodes/queryjumblefuncs.c b/src/backend/nodes/queryjumblefuncs.c index 7c63766a51c..2ce27b9e552 100644 --- a/src/backend/nodes/queryjumblefuncs.c +++ b/src/backend/nodes/queryjumblefuncs.c @@ -232,7 +232,7 @@ DoJumble(JumbleState *jstate, Node *node) * * Note: Callers must ensure that size > 0. */ -static pg_attribute_always_inline void +static pg_always_inline void AppendJumbleInternal(JumbleState *jstate, const unsigned char *item, Size size) { @@ -308,7 +308,7 @@ AppendJumble(JumbleState *jstate, const unsigned char *value, Size size) * AppendJumbleNull * For jumbling NULL pointers */ -static pg_attribute_always_inline void +static pg_always_inline void AppendJumbleNull(JumbleState *jstate) { jstate->pending_nulls++; @@ -375,7 +375,7 @@ AppendJumble64(JumbleState *jstate, const unsigned char *value) * * Note: Callers must ensure that there's at least 1 pending NULL. */ -static pg_attribute_always_inline void +static pg_always_inline void FlushPendingNulls(JumbleState *jstate) { Assert(jstate->pending_nulls > 0); diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 3cc0b0bdd92..1f1198ec0f7 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -649,10 +649,10 @@ static inline BufferDesc *BufferAlloc(SMgrRelation smgr, static bool AsyncReadBuffers(ReadBuffersOperation *operation, int *nblocks_progress); static void CheckReadBuffersOperation(ReadBuffersOperation *operation, bool is_complete); -static pg_attribute_always_inline void TrackBufferHit(IOObject io_object, - IOContext io_context, - Relation rel, char persistence, SMgrRelation smgr, - ForkNumber forknum, BlockNumber blocknum); +static pg_always_inline void TrackBufferHit(IOObject io_object, + IOContext io_context, + Relation rel, char persistence, SMgrRelation smgr, + ForkNumber forknum, BlockNumber blocknum); static Buffer GetVictimBuffer(BufferAccessStrategy strategy, IOContext io_context); static void FlushUnlockedBuffer(BufferDesc *buf, SMgrRelation reln, IOObject io_object, IOContext io_context); @@ -1228,7 +1228,7 @@ ZeroAndLockBuffer(Buffer buffer, ReadBufferMode mode, bool already_valid) * already present, or false if more work is required to either read it in or * zero it. */ -static pg_attribute_always_inline Buffer +static pg_always_inline Buffer PinBufferForBlock(Relation rel, SMgrRelation smgr, char persistence, @@ -1281,7 +1281,7 @@ PinBufferForBlock(Relation rel, * * smgr is required, rel is optional unless using P_NEW. */ -static pg_attribute_always_inline Buffer +static pg_always_inline Buffer ReadBuffer_common(Relation rel, SMgrRelation smgr, char smgr_persistence, ForkNumber forkNum, BlockNumber blockNum, ReadBufferMode mode, @@ -1364,7 +1364,7 @@ ReadBuffer_common(Relation rel, SMgrRelation smgr, char smgr_persistence, return buffer; } -static pg_attribute_always_inline bool +static pg_always_inline bool StartReadBuffersImpl(ReadBuffersOperation *operation, Buffer *buffers, BlockNumber blockNum, @@ -1670,7 +1670,7 @@ CheckReadBuffersOperation(ReadBuffersOperation *operation, bool is_complete) * We track various stats related to buffer hits. Because this is done in a * few separate places, this helper exists for convenience. */ -static pg_attribute_always_inline void +static pg_always_inline void TrackBufferHit(IOObject io_object, IOContext io_context, Relation rel, char persistence, SMgrRelation smgr, ForkNumber forknum, BlockNumber blocknum) @@ -2184,7 +2184,7 @@ AsyncReadBuffers(ReadBuffersOperation *operation, int *nblocks_progress) * * No locks are held either at entry or exit. */ -static pg_attribute_always_inline BufferDesc * +static pg_always_inline BufferDesc * BufferAlloc(SMgrRelation smgr, char relpersistence, ForkNumber forkNum, BlockNumber blockNum, BufferAccessStrategy strategy, @@ -8277,7 +8277,7 @@ MarkDirtyAllUnpinnedBuffers(int32 *buffers_dirtied, * part of error handling, which in turn could lead to the buffer being * replaced while IO is ongoing. */ -static pg_attribute_always_inline void +static pg_always_inline void buffer_stage_common(PgAioHandle *ioh, bool is_write, bool is_temp) { uint64 *io_data; @@ -8521,7 +8521,7 @@ buffer_readv_encode_error(PgAioResult *result, * Helper for AIO readv completion callbacks, supporting both shared and temp * buffers. Gets called once for each buffer in a multi-page read. */ -static pg_attribute_always_inline void +static pg_always_inline void buffer_readv_complete_one(PgAioTargetData *td, uint8 buf_off, Buffer buffer, uint8 flags, bool failed, bool is_temp, bool *buffer_invalid, @@ -8672,7 +8672,7 @@ buffer_readv_complete_one(PgAioTargetData *td, uint8 buf_off, Buffer buffer, * * Shared between shared and local buffers, to reduce code duplication. */ -static pg_attribute_always_inline PgAioResult +static pg_always_inline PgAioResult buffer_readv_complete(PgAioHandle *ioh, PgAioResult prior_result, uint8 cb_data, bool is_temp) { diff --git a/src/backend/utils/adt/json.c b/src/backend/utils/adt/json.c index 0fee1b40d63..dccbe07cd2d 100644 --- a/src/backend/utils/adt/json.c +++ b/src/backend/utils/adt/json.c @@ -1528,7 +1528,7 @@ json_object_two_arg(PG_FUNCTION_ARGS) * escape_json_char * Inline helper function for escape_json* functions */ -static pg_attribute_always_inline void +static pg_always_inline void escape_json_char(StringInfo buf, char c) { switch (c) diff --git a/src/backend/utils/cache/catcache.c b/src/backend/utils/cache/catcache.c index 87ed5506460..baa98f18ffa 100644 --- a/src/backend/utils/cache/catcache.c +++ b/src/backend/utils/cache/catcache.c @@ -1080,7 +1080,7 @@ RehashCatCacheLists(CatCache *cp) * * Call CatalogCacheInitializeCache() if not yet done. */ -pg_attribute_always_inline +pg_always_inline static void ConditionalCatalogCacheInitializeCache(CatCache *cache) { -- 2.53.0.1.gb2826b52eb --nrl6kihukes5f4e7-- ^ permalink raw reply [nested|flat] 13+ messages in thread
end of thread, other threads:[~2026-04-08 21:07 UTC | newest] Thread overview: 13+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2022-02-26 16:07 Re: Allow async standbys wait for sync replication (was: Disallow quorum uncommitted (with synchronous standbys) txns in logical replication subscribers) Nathan Bossart <[email protected]> 2022-02-28 13:15 ` Bharath Rupireddy <[email protected]> 2022-02-28 18:57 ` Nathan Bossart <[email protected]> 2022-03-01 05:40 ` Bharath Rupireddy <[email protected]> 2022-03-01 06:05 ` Nathan Bossart <[email protected]> 2022-03-01 07:34 ` Re: Allow async standbys wait for sync replication Kyotaro Horiguchi <[email protected]> 2022-03-01 17:05 ` Re: Allow async standbys wait for sync replication Nathan Bossart <[email protected]> 2022-03-01 17:39 ` Re: Allow async standbys wait for sync replication Bharath Rupireddy <[email protected]> 2022-03-01 21:27 ` Re: Allow async standbys wait for sync replication Nathan Bossart <[email protected]> 2022-03-02 04:17 ` Re: Allow async standbys wait for sync replication Bharath Rupireddy <[email protected]> 2022-03-04 19:56 ` Re: Allow async standbys wait for sync replication Nathan Bossart <[email protected]> 2022-03-17 17:05 ` Re: Allow async standbys wait for sync replication Bharath Rupireddy <[email protected]> 2026-04-08 21:07 [PATCH v1] Shorten pg_attribute_always_inline to pg_always_inline Andres Freund <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox