public inbox for [email protected]
help / color / mirror / Atom feedFrom: Hsu, John <[email protected]>
To: Nathan Bossart <[email protected]>
To: Bharath Rupireddy <[email protected]>
Cc: SATYANARAYANA NARLAPURAM <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Allow async standbys wait for sync replication (was: Disallow quorum uncommitted (with synchronous standbys) txns in logical replication subscribers)
Date: Fri, 25 Feb 2022 13:52:03 -0800
Message-ID: <[email protected]> (raw)
In-Reply-To: <20220225193819.GB662561@nathanxps13>
References: <CALj2ACX-xO-ZenQt1MWazj0Z3ziSXBMr24N_X2c0dYysPQghrw@mail.gmail.com>
<CAHg+QDc-3bPT52yqB-_J-_8bc6DzpxVVzvQMA99XV_0tqOR9wg@mail.gmail.com>
<CALj2ACVUa8WddVDS20QmVKNwTbeOQqy4zy59NPzh8NnLipYZGw@mail.gmail.com>
<20220225193819.GB662561@nathanxps13>
Hello,
On 2/25/22 11:38 AM, Nathan Bossart wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
>
>
>
> On Fri, Feb 25, 2022 at 08:31:37PM +0530, Bharath Rupireddy wrote:
>> Thanks Satya and others for the inputs. Here's the v1 patch that
>> basically allows async wal senders to wait until the sync standbys
>> report their flush lsn back to the primary. Please let me know your
>> thoughts.
> I haven't had a chance to look too closely yet, but IIUC this adds a new
> function that waits for synchronous replication. This new function
> essentially spins until the synchronous LSN has advanced.
>
> I don't think it's a good idea to block sending any WAL like this. AFAICT
> it is possible that there will be a lot of synchronously replicated WAL
> that we can send, and it might just be the last several bytes that cannot
> yet be replicated to the asynchronous standbys. І believe this patch will
> cause the server to avoid sending _any_ WAL until the synchronous LSN
> advances.
>
> Perhaps we should instead just choose the SendRqstPtr based on the current
> synchronous LSN. Presumably there are other things we'd need to consider,
> but in general, I think we ought to send as much WAL as possible for a
> given call to XLogSendPhysical().
I think you're right that we'll avoid sending any WAL until sync_lsn
advances. We could setup a contrived situation where the async-walsender
never advances because it terminates before the flush_lsn of the
synchronous_node catches up. And when the async-walsender restarts,
it'll start with the latest flushed on the primary and we could go into
a perpetual loop.
I took a look at the patch and tested basic streaming with async
replicas ahead of the synchronous standby and with logical clients as
well and it works as expected.
>
> ereport(LOG,
> (errmsg("async standby WAL sender with request LSN %X/%X
is waiting as sync standbys are ahead with flush LSN %X/%X",
> LSN_FORMAT_ARGS(flushLSN),
LSN_FORMAT_ARGS(sendRqstPtr)),
> errhidestmt(true)));
I think this log formatting is incorrect.
s/sync standbys are ahead/sync standbys are behind/ and I think you need
to swap flushLsn and sendRqstPtr
When a walsender is waiting for the lsn on the synchronous replica to
advance and a database stop is issued to the writer, the pg_ctl stop
isn't able to proceed and the database seems to never shutdown.
> Assert(priority >= 0);
What's the point of the assert here?
Also the comments/code refer to AsyncStandbys, however it's also used
for logical clients, which may or may not be standbys. Don't feel too
strongly about the naming here but something to note.
> if (!ShouldWaitForSyncRepl())
> return;
> ...
> for (;;)
> {
> // rest of work
> }
If we had a walsender already waiting for an ack, and the conditions of
ShouldWaitForSyncRepl() change, such as disabling
async_standbys_wait_for_sync_replication or synchronous replication
it'll still wait since we never re-check the condition.
postgres=# select wait_event from pg_stat_activity where wait_event like
'AsyncWal%';
wait_event
--------------------------------------
AsyncWalSenderWaitForSyncReplication
AsyncWalSenderWaitForSyncReplication
AsyncWalSenderWaitForSyncReplication
(3 rows)
postgres=# show synchronous_standby_names;
synchronous_standby_names
---------------------------
(1 row)
postgres=# show async_standbys_wait_for_sync_replication;
async_standbys_wait_for_sync_replication
------------------------------------------
off
(1 row)
> LWLockAcquire(SyncRepLock, LW_SHARED);
> flushLSN = walsndctl->lsn[SYNC_REP_WAIT_FLUSH];
> LWLockRelease(SyncRepLock);
Should we configure this similar to the user's setting of
synchronous_commit instead of just flush? (SYNC_REP_WAIT_WRITE,
SYNC_REP_WAIT_APPLY)
Thanks,
John H
view thread (4+ messages)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Allow async standbys wait for sync replication (was: Disallow quorum uncommitted (with synchronous standbys) txns in logical replication subscribers)
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox