Disallow quorum uncommitted (with synchronous standbys) txns in logical replication subscribers

Consider a cluster formation where we have a Primary(P), Sync Replica(S1),
and multiple async replicas for disaster recovery and read scaling (within
the region and outside the region). In this setup, S1 is the preferred
failover target in an event of the primary failure. When a transaction is
committed on the primary, it is not acknowledged to the client until the
primary gets an acknowledgment from the sync standby that the WAL is
flushed to the disk (assume synchrnous_commit configuration is
remote_flush). However, walsenders corresponds to the async replica on the
primary don't wait for the flush acknowledgment from the primary and send
the WAL to the async standbys (also any logical replication/decoding
clients). So it is possible for the async replicas and logical client ahead
of the sync replica. If a failover is initiated in such a scenario, to
bring the formation into a healthy state we have to either

1. run the pg_rewind on the async replicas for them to reconnect with
the new primary or
2. collect the latest WAL across the replicas and feed the standby.

Both these operations are involved, error prone, and can cause multiple
minutes of downtime if done manually. In addition, there is a window where
the async replicas can show the data that was neither acknowledged to the
client nor committed on standby. Logical clients if they are ahead may need
to reseed the data as no easy rewind option for them.

I would like to propose a GUC send_Wal_after_quorum_committed which when
set to ON, walsenders corresponds to async standbys and logical replication
workers wait until the LSN is quorum committed on the primary before
sending it to the standby. This not only simplifies the post failover steps
but avoids unnecessary downtime for the async replicas. Thoughts?

Thanks,
Satya

On Sun, Dec 5, 2021 at 8:35 PM Bharath Rupireddy <
bharath.rupireddyforpostgres@gmail.com> wrote:

On Wed, 2022-01-05 at 23:59 -0800, SATYANARAYANA NARLAPURAM wrote:

I would like to propose a GUC send_Wal_after_quorum_committed which
when set to ON, walsenders corresponds to async standbys and logical
replication workers wait until the LSN is quorum committed on the
primary before sending it to the standby. This not only simplifies
the post failover steps but avoids unnecessary downtime for the async
replicas. Thoughts?

Do we need a GUC? Or should we just always require that sync rep is
satisfied before sending to async replicas?

It feels like the sync quorum should always be ahead of the async
replicas. Unless I'm missing a use case, or there is some kind of
performance gotcha.

Regards,
Jeff Davis

On Thu, Jan 6, 2022 at 11:24 PM Jeff Davis <pgsql@j-davis.com> wrote:

On Wed, 2022-01-05 at 23:59 -0800, SATYANARAYANA NARLAPURAM wrote:

I would like to propose a GUC send_Wal_after_quorum_committed which
when set to ON, walsenders corresponds to async standbys and logical
replication workers wait until the LSN is quorum committed on the
primary before sending it to the standby. This not only simplifies
the post failover steps but avoids unnecessary downtime for the async
replicas. Thoughts?

Do we need a GUC? Or should we just always require that sync rep is
satisfied before sending to async replicas?

I proposed a GUC to not introduce a behavior change by default. I have no
strong opinion on having a GUC or making the proposed behavior default,
would love to get others' perspectives as well.

It feels like the sync quorum should always be ahead of the async
replicas. Unless I'm missing a use case, or there is some kind of
performance gotcha.

I couldn't think of a case that can cause serious performance issues but
will run some experiments on this and post the numbers.

At Thu, 6 Jan 2022 23:55:01 -0800, SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com> wrote in

On Thu, Jan 6, 2022 at 11:24 PM Jeff Davis <pgsql@j-davis.com> wrote:

On Wed, 2022-01-05 at 23:59 -0800, SATYANARAYANA NARLAPURAM wrote:

I would like to propose a GUC send_Wal_after_quorum_committed which
when set to ON, walsenders corresponds to async standbys and logical
replication workers wait until the LSN is quorum committed on the
primary before sending it to the standby. This not only simplifies
the post failover steps but avoids unnecessary downtime for the async
replicas. Thoughts?

Do we need a GUC? Or should we just always require that sync rep is
satisfied before sending to async replicas?

I proposed a GUC to not introduce a behavior change by default. I have no
strong opinion on having a GUC or making the proposed behavior default,
would love to get others' perspectives as well.

It feels like the sync quorum should always be ahead of the async
replicas. Unless I'm missing a use case, or there is some kind of
performance gotcha.

I couldn't think of a case that can cause serious performance issues but
will run some experiments on this and post the numbers.

I think Jeff is saying that "quorum commit" already by definition
means that all out-of-quorum standbys are behind of the
quorum-standbys. I agree to that in a dictionary sense. But I can
think of the case where the response from the top-runner standby
vanishes or gets caught somewhere on network for some reason. In that
case the primary happily checks quorum ignoring the top-runner.

To avoid that misdecision, I can guess two possible "solutions".

One is to serialize WAL sending (of course it is unacceptable at all)
or aotehr is to send WAL to all standbys at once then make the
decision after making sure receiving replies from all standbys (this
is no longer quorum commit in another sense..)

So I'm afraid that there's no sensible solution to avoid the
hiding-forerunner problem on quorum commit.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

On Fri, Jan 7, 2022 at 12:27 AM Kyotaro Horiguchi <horikyota.ntt@gmail.com>
wrote:

At Thu, 6 Jan 2022 23:55:01 -0800, SATYANARAYANA NARLAPURAM <
satyanarlapuram@gmail.com> wrote in

On Thu, Jan 6, 2022 at 11:24 PM Jeff Davis <pgsql@j-davis.com> wrote:

On Wed, 2022-01-05 at 23:59 -0800, SATYANARAYANA NARLAPURAM wrote:

I would like to propose a GUC send_Wal_after_quorum_committed which
when set to ON, walsenders corresponds to async standbys and logical
replication workers wait until the LSN is quorum committed on the
primary before sending it to the standby. This not only simplifies
the post failover steps but avoids unnecessary downtime for the async
replicas. Thoughts?

Do we need a GUC? Or should we just always require that sync rep is
satisfied before sending to async replicas?

I proposed a GUC to not introduce a behavior change by default. I have no
strong opinion on having a GUC or making the proposed behavior default,
would love to get others' perspectives as well.

It feels like the sync quorum should always be ahead of the async
replicas. Unless I'm missing a use case, or there is some kind of
performance gotcha.

I couldn't think of a case that can cause serious performance issues but
will run some experiments on this and post the numbers.

I think Jeff is saying that "quorum commit" already by definition
means that all out-of-quorum standbys are behind of the
quorum-standbys. I agree to that in a dictionary sense. But I can
think of the case where the response from the top-runner standby
vanishes or gets caught somewhere on network for some reason. In that
case the primary happily checks quorum ignoring the top-runner.

To avoid that misdecision, I can guess two possible "solutions".

One is to serialize WAL sending (of course it is unacceptable at all)
or aotehr is to send WAL to all standbys at once then make the
decision after making sure receiving replies from all standbys (this
is no longer quorum commit in another sense..)

There is no need to serialize sending the WAL among sync standbys. The only
serialization required is first to all the sync replicas and then to sync
replicas if any. Once an LSN is quorum committed, no failover subsystem
initiates an automatic failover such that the LSN is lost (data loss)

So I'm afraid that there's no sensible solution to avoid the
hiding-forerunner problem on quorum commit.

Could you elaborate on the problem here?

On 1/6/22, 11:25 PM, "Jeff Davis" <pgsql@j-davis.com> wrote:

On Wed, 2022-01-05 at 23:59 -0800, SATYANARAYANA NARLAPURAM wrote:

I would like to propose a GUC send_Wal_after_quorum_committed which
when set to ON, walsenders corresponds to async standbys and logical
replication workers wait until the LSN is quorum committed on the
primary before sending it to the standby. This not only simplifies
the post failover steps but avoids unnecessary downtime for the async
replicas. Thoughts?

Do we need a GUC? Or should we just always require that sync rep is
satisfied before sending to async replicas?

It feels like the sync quorum should always be ahead of the async
replicas. Unless I'm missing a use case, or there is some kind of
performance gotcha.

I don't have a strong opinion on whether there needs to be a GUC, but
+1 for the ability to enforce sync quorum before sending WAL to async
standbys. I think that would be a reasonable default behavior.

Nathan

On Fri, Jan 7, 2022 at 12:54 PM Jeff Davis <pgsql@j-davis.com> wrote:

On Wed, 2022-01-05 at 23:59 -0800, SATYANARAYANA NARLAPURAM wrote:

I would like to propose a GUC send_Wal_after_quorum_committed which
when set to ON, walsenders corresponds to async standbys and logical
replication workers wait until the LSN is quorum committed on the
primary before sending it to the standby. This not only simplifies
the post failover steps but avoids unnecessary downtime for the async
replicas. Thoughts?

Do we need a GUC? Or should we just always require that sync rep is
satisfied before sending to async replicas?

It feels like the sync quorum should always be ahead of the async
replicas. Unless I'm missing a use case, or there is some kind of
performance gotcha.

IMO, having GUC is a reasonable choice because some users might be
okay with it if their async replicas are ahead of the sync ones or
they would have dealt with this problem already in their HA solutions
or they don't want their async replicas to fall behind by the primary
(most of the times).

If there are long running txns on the primary and the async standbys
were to wait until quorum commit from sync standbys, won't they fall
behind the primary by too much? This isn't a problem at all if we
think from the perspective that async replicas are anyways prone to
falling behind by the primary. But, if the primary is having long
running txns continuously, the async replicas would eventually fall
behind more and more. Is there a way we can send the WAL records to
both sync and async replicas together but the async replicas won't
apply those WAL records until primary tells the standbys that quorum
commit is obtained? If the quorum commit isn't obtained by the
primary, the async replicas can ignore to apply the WAL records and
discard them.

Regards,
Bharath Rupireddy.

On Sat, 2022-01-08 at 00:13 +0530, Bharath Rupireddy wrote:

If there are long running txns on the primary and the async standbys
were to wait until quorum commit from sync standbys, won't they fall
behind the primary by too much?

No, because replication is based on LSNs, not transactions.

With the proposed change: an LSN can be replicated to all sync replicas
as soon as it's durable on the primary; and an LSN can be replicated to
all async replicas as soon as it's durable on the primary *and* the
sync rep quorum is satisfied.

Regards,
Jeff Davis

Hi,

On 2022-01-06 23:24:40 -0800, Jeff Davis wrote:

It feels like the sync quorum should always be ahead of the async
replicas. Unless I'm missing a use case, or there is some kind of
performance gotcha.

I don't see how it can *not* cause a major performance / latency
gotcha. You're deliberately delaying replication after all?

Synchronous replication doesn't guarantee *anything* about the ability for to
fail over for other replicas. Nor would it after what's proposed here -
another sync replica would still not be guaranteed to be able to follow the
newly promoted primary.

To me this just sounds like trying to shoehorn something into syncrep that
it's not made for.

Greetings,

Andres Freund

On Fri, 2022-01-07 at 12:22 -0800, Andres Freund wrote:

I don't see how it can *not* cause a major performance / latency
gotcha. You're deliberately delaying replication after all?

Are there use cases where someone wants sync rep, and also wants their
read replicas to get ahead of the sync rep quorum?

If the use case doesn't exist, it doesn't make sense to talk about how
well it performs.

another sync replica would still not be guaranteed to be able to
follow the
newly promoted primary.

If you only promote the furthest-ahead sync replica (which is what you
should be doing if you have quorum commit), why wouldn't that work?

To me this just sounds like trying to shoehorn something into syncrep
that
it's not made for.

What *is* sync rep made for?

The only justification in the docs is around durability:

"[sync rep] extends that standard level of durability offered by a
transaction commit... [sync rep] can provide a much higher level of
durability..."

If we take that at face value, then it seems logical to say that async
read replicas should not get ahead of sync replicas.

Regards,
Jeff Davis

Hi,

On 2022-01-07 14:36:46 -0800, Jeff Davis wrote:

On Fri, 2022-01-07 at 12:22 -0800, Andres Freund wrote:

I don't see how it can *not* cause a major performance / latency
gotcha. You're deliberately delaying replication after all?

Are there use cases where someone wants sync rep, and also wants their
read replicas to get ahead of the sync rep quorum?

Yes. Not in the sense of being ahead of the sync replicas, but in the sense of
being as cought up as possible, and to keep the lost WAL in case of crashes as
low as possible.

another sync replica would still not be guaranteed to be able to
follow the
newly promoted primary.

If you only promote the furthest-ahead sync replica (which is what you
should be doing if you have quorum commit), why wouldn't that work?

Remove "sync" from the above sentence, and the sentence holds true for
combinations of sync/async replicas as well.

To me this just sounds like trying to shoehorn something into syncrep
that
it's not made for.

What *is* sync rep made for?

The only justification in the docs is around durability:

"[sync rep] extends that standard level of durability offered by a
transaction commit... [sync rep] can provide a much higher level of
durability..."

What is being proposed here doesn't increase durability. It *reduces* it -
it's less likely that WAL is replicated before a crash.

This is a especially relevant in cases where synchronous_commit=on vs local is
used selectively - after this change the durability of local changes is very
substantially *reduced* because they have to wait for the sync replicas before
also replicated to async replicas, but the COMMIT doesn't wait for
replication. So this "feature" just reduces the durability of such commits.

The performance overhead of syncrep is high enough that plenty real-world
usages cannot afford to use it for all transactions. And that's normally fine
from a business logic POV - often the majority of changes aren't that
important. It's non-trivial from an application implementation POV though, but
that's imo a separate concern.

If we take that at face value, then it seems logical to say that async
read replicas should not get ahead of sync replicas.

I don't see that. This presumes that WAL replicated to async replicas is
somehow bad. But pg_rewind exist, async replicas can be promoted and WAL from
the async replicas can be transferred to the synchronous replicas if only
those should be promoted.

Greetings,

Andres Freund

On Fri, 2022-01-07 at 14:54 -0800, Andres Freund wrote:

If you only promote the furthest-ahead sync replica (which is what
you
should be doing if you have quorum commit), why wouldn't that work?

Remove "sync" from the above sentence, and the sentence holds true
for
combinations of sync/async replicas as well.

Technically that's true, but it seems like a bit of a strange use case.
I would think people doing that would just include those async replicas
in the sync quorum instead.

The main case I can think of for a mix of sync and async replicas are
if they are just managed differently. For instance, the sync replica
quorum is managed for a core part of the system, strategically
allocated on good hardware in different locations to minimize the
chance of dependent failures; while the async read replicas are
optional for taking load off the primary, and may appear/disappear in
whatever location and on whatever hardware is most convenient.

But if an async replica can get ahead of the sync rep quorum, then the
most recent transactions can appear in query results, so that means the
WAL shouldn't be lost, and the async read replicas become a part of the
durability model.

If the async read replica can't be promoted because it's not suitable
(due to location, hardware, whatever), then you need to frantically
copy the final WAL records out to an instance in the sync rep quorum.
That requires extra ceremony for every failover, and might be dubious
depending on how safe the WAL on your async read replicas is, and
whether there are dependent failure risks.

Yeah, I guess there could be some use case woven amongst those caveats,
but I'm not sure if anyone is actually doing that combination of things
safely today. If someone is, it would be interesting to know more about
that use case.

The proposal in this thread is quite a bit simpler: manage your sync
quorum and your async read replicas separately, and keep the sync rep
quorum ahead.

To me this just sounds like trying to shoehorn something into
syncrep
that
it's not made for.

What *is* sync rep made for?

This was a sincere question and an answer would be helpful. I think
many of the discussions about sync rep get derailed because people have
different ideas about when and how it should be used, and the
documentation is pretty light.

This is a especially relevant in cases where synchronous_commit=on vs
local is
used selectively

That's an interesting point.

However, it's hard for me to reason about "kinda durable" and "a little
more durable" and I'm not sure how many people would care about that
distinction.

I don't see that. This presumes that WAL replicated to async replicas
is
somehow bad.

Simple case: primary and async read replica are in the same server
rack. Sync replicas are geographically distributed with quorum commit.
Read replica gets the WAL first (because it's closest), starts
answering queries that include that WAL, and then the entire rack
catches fire. Now you've returned results to the client, but lost the
transactions.

Regards,
Jeff Davis

On Fri, Jan 7, 2022 at 4:52 PM Jeff Davis <pgsql@j-davis.com> wrote:

On Fri, 2022-01-07 at 14:54 -0800, Andres Freund wrote:

If you only promote the furthest-ahead sync replica (which is what
you
should be doing if you have quorum commit), why wouldn't that work?

Remove "sync" from the above sentence, and the sentence holds true
for
combinations of sync/async replicas as well.

Technically that's true, but it seems like a bit of a strange use case.
I would think people doing that would just include those async replicas
in the sync quorum instead.

The main case I can think of for a mix of sync and async replicas are
if they are just managed differently. For instance, the sync replica
quorum is managed for a core part of the system, strategically
allocated on good hardware in different locations to minimize the
chance of dependent failures; while the async read replicas are
optional for taking load off the primary, and may appear/disappear in
whatever location and on whatever hardware is most convenient.

But if an async replica can get ahead of the sync rep quorum, then the
most recent transactions can appear in query results, so that means the
WAL shouldn't be lost, and the async read replicas become a part of the
durability model.

If the async read replica can't be promoted because it's not suitable
(due to location, hardware, whatever), then you need to frantically
copy the final WAL records out to an instance in the sync rep quorum.
That requires extra ceremony for every failover, and might be dubious
depending on how safe the WAL on your async read replicas is, and
whether there are dependent failure risks.

This may not even be possible always as described in the scenario below.

Yeah, I guess there could be some use case woven amongst those caveats,
but I'm not sure if anyone is actually doing that combination of things
safely today. If someone is, it would be interesting to know more about
that use case.

The proposal in this thread is quite a bit simpler: manage your sync
quorum and your async read replicas separately, and keep the sync rep
quorum ahead.

To me this just sounds like trying to shoehorn something into
syncrep
that
it's not made for.

What *is* sync rep made for?

This was a sincere question and an answer would be helpful. I think
many of the discussions about sync rep get derailed because people have
different ideas about when and how it should be used, and the
documentation is pretty light.

This is a especially relevant in cases where synchronous_commit=on vs
local is
used selectively

That's an interesting point.

However, it's hard for me to reason about "kinda durable" and "a little
more durable" and I'm not sure how many people would care about that
distinction.

I don't see that. This presumes that WAL replicated to async replicas
is
somehow bad.

Simple case: primary and async read replica are in the same server
rack. Sync replicas are geographically distributed with quorum commit.
Read replica gets the WAL first (because it's closest), starts
answering queries that include that WAL, and then the entire rack
catches fire. Now you've returned results to the client, but lost the
transactions.

Another similar example is, in a multi-AZ HA setup, primary and sync
replicas are deployed in two different availability zones and the async
replicas for reads can be in any availability zone and assume the async
replica and primary land in the same AZ. Primary availability zone going
down leads to both primary and async replica going down at the same time.
This async replica could be ahead of sync replica and WAL can't be
collected as both primary and async replica failed together.

At Fri, 7 Jan 2022 09:44:15 -0800, SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com> wrote in

On Fri, Jan 7, 2022 at 12:27 AM Kyotaro Horiguchi <horikyota.ntt@gmail.com>
wrote:

One is to serialize WAL sending (of course it is unacceptable at all)
or aotehr is to send WAL to all standbys at once then make the
decision after making sure receiving replies from all standbys (this
is no longer quorum commit in another sense..)

There is no need to serialize sending the WAL among sync standbys. The only
serialization required is first to all the sync replicas and then to sync
replicas if any. Once an LSN is quorum committed, no failover subsystem
initiates an automatic failover such that the LSN is lost (data loss)

Sync standbys on PostgreSQL is ex post facto. When a certain set of
standbys have first reported catching-up for a commit, they are called
"sync standbys".

We can maintain a fixed set of sync standbys based on the set of
sync-standbys at a past commits, but that implies performance
degradation even if not a single standby is gone.

If we send WAL only to the fixed-set of sync standbys, when any of the
standbys is gone, the primary is forced to wait until some timeout
expires. The same commit would finish immediately if WAL had been
sent also to out-of-quorum standbys.

So I'm afraid that there's no sensible solution to avoid the
hiding-forerunner problem on quorum commit.

Could you elaborate on the problem here?

If a primary have received response for LSN=X from N standbys, that
fact doesn't guarantee that none of the other standbys reached the
same LSN. If one of the yet-unresponded standbys already reached
LSN=X+10 but its response does not arrived to the primary for some
reasons, the true-fastest standby is hiding from primary.

Even if the primary examines the responses from all standbys, it is
uncertain if the responses reflect the truly current state of the
standbys. Thus if we want to guarantee that no unresponded standby is
going beyond LSN=X, there's no means other than we refrain from
sending WAL beyond X. In that case, we need to serialize the period
from WAL-sending to response-reception, which would lead to critical
performance degradation.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

On Thu, Jan 6, 2022 at 1:29 PM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

Consider a cluster formation where we have a Primary(P), Sync Replica(S1), and multiple async replicas for disaster recovery and read scaling (within the region and outside the region). In this setup, S1 is the preferred failover target in an event of the primary failure. When a transaction is committed on the primary, it is not acknowledged to the client until the primary gets an acknowledgment from the sync standby that the WAL is flushed to the disk (assume synchrnous_commit configuration is remote_flush). However, walsenders corresponds to the async replica on the primary don't wait for the flush acknowledgment from the primary and send the WAL to the async standbys (also any logical replication/decoding clients). So it is possible for the async replicas and logical client ahead of the sync replica. If a failover is initiated in such a scenario, to bring the formation into a healthy state we have to either

run the pg_rewind on the async replicas for them to reconnect with the new primary or
collect the latest WAL across the replicas and feed the standby.

Both these operations are involved, error prone, and can cause multiple minutes of downtime if done manually. In addition, there is a window where the async replicas can show the data that was neither acknowledged to the client nor committed on standby. Logical clients if they are ahead may need to reseed the data as no easy rewind option for them.

I would like to propose a GUC send_Wal_after_quorum_committed which when set to ON, walsenders corresponds to async standbys and logical replication workers wait until the LSN is quorum committed on the primary before sending it to the standby. This not only simplifies the post failover steps but avoids unnecessary downtime for the async replicas. Thoughts?

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've done pgbench testing to see if the patch causes any problems. I
ran tests two times, there isn't much difference in the txns per
seconds (tps), although there's a delay in the async standby receiving
the WAL, after all, that's the feature we are pursuing.

[1]: HEAD or WITHOUT PATCH: ./pgbench -c 10 -t 500 -P 10 testdb transaction type: <builtin: TPC-B (sort of)> scaling factor: 100 query mode: simple number of clients: 10 number of threads: 1 number of transactions per client: 500 number of transactions actually processed: 5000/5000 latency average = 247.395 ms latency stddev = 74.409 ms initial connection time = 13.622 ms tps = 39.713114 (without initial connection time)
HEAD or WITHOUT PATCH:
./pgbench -c 10 -t 500 -P 10 testdb
transaction type: <builtin: TPC-B (sort of)>
scaling factor: 100
query mode: simple
number of clients: 10
number of threads: 1
number of transactions per client: 500
number of transactions actually processed: 5000/5000
latency average = 247.395 ms
latency stddev = 74.409 ms
initial connection time = 13.622 ms
tps = 39.713114 (without initial connection time)

PATCH:
./pgbench -c 10 -t 500 -P 10 testdb
transaction type: <builtin: TPC-B (sort of)>
scaling factor: 100
query mode: simple
number of clients: 10
number of threads: 1
number of transactions per client: 500
number of transactions actually processed: 5000/5000
latency average = 251.757 ms
latency stddev = 72.846 ms
initial connection time = 13.025 ms
tps = 39.315862 (without initial connection time)

TEST SETUP:
primary in region 1
async standby 1 in the same region as that of the primary region 1
i.e. close to primary
sync standby 1 in region 2
sync standby 2 in region 3
an archive location in a region different from the primary and
standbys regions, region 4
Note that I intentionally kept sync standbys in regions far from
primary because it allows sync standbys to receive WAL a bit late by
default, which works well for our testing.

PGBENCH SETUP:
./psql -d postgres -c "drop database testdb"
./psql -d postgres -c "create database testdb"
./pgbench -i -s 100 testdb
./psql -d testdb -c "\dt"
./psql -d testdb -c "SELECT pg_size_pretty(pg_database_size('testdb'))"
./pgbench -c 10 -t 500 -P 10 testdb

Regards,
Bharath Rupireddy.

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've done pgbench testing to see if the patch causes any problems. I
ran tests two times, there isn't much difference in the txns per
seconds (tps), although there's a delay in the async standby receiving
the WAL, after all, that's the feature we are pursuing.

I'm curious what a longer pgbench run looks like when the synchronous
replicas are in the same region. That is probably a more realistic
use-case.

--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

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

On Sat, Feb 26, 2022 at 1:08 AM Nathan Bossart <nathandbossart@gmail.com> wrote:

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().

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 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've done pgbench testing to see if the patch causes any problems. I
ran tests two times, there isn't much difference in the txns per
seconds (tps), although there's a delay in the async standby receiving
the WAL, after all, that's the feature we are pursuing.

I'm curious what a longer pgbench run looks like when the synchronous
replicas are in the same region. That is probably a more realistic
use-case.

We are performing more tests, I will share the results once done.

Regards,
Bharath Rupireddy.

On Sat, Feb 26, 2022 at 3:22 AM Hsu, John <hsuchen@amazon.com> wrote:

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.

The async walsender looks at flush LSN from
walsndctl->lsn[SYNC_REP_WAIT_FLUSH]; after it comes up and decides to
send the WAL up to it. If there are no sync replicats after it comes
up (users can make sync standbys async without postmaster restart
because synchronous_standby_names is effective with SIGHUP), then it
doesn't wait at all and continues to send WAL. I don't see any problem
with it. Am I missing something here?

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.

Thanks for reviewing and testing the patch.

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

I will correct it. "async standby WAL sender with request LSN %X/%X is
waiting as sync standbys are ahead with flush LSN %X/%X",
LSN_FORMAT_ARGS(sendRqstP), LSN_FORMAT_ARGS(flushLSN). I will think
more about having better wording of these messages, any suggestions
here?

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.

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?

Assert(priority >= 0);

What's the point of the assert here?

Just for safety. I can remove it as the sync_standby_priority can
never be negative.

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.

I will try to be more informative by adding something like "async
standbys and logical replication subscribers".

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.

Yeah, I will add the checks inside the async walsender wait-loop.

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)

As I said upthread, we can allow 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 open to more thoughts
here.

Regards,
Bharath Rupireddy.