public inbox for [email protected]  
help / color / mirror / Atom feed
From: Tomas Vondra <[email protected]>
To: Fujii Masao <[email protected]>
To: Amit Kapila <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: sequences vs. synchronous replication
Date: Wed, 22 Dec 2021 20:00:09 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<CAA4eK1KN-Pqe1y5w0uXU2Ttn8DK4WfqgwAbCRbcDt4yzQ88wRA@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>

On 12/22/21 18:50, Fujii Masao wrote:
> 
> 
> On 2021/12/22 21:11, Tomas Vondra wrote:
>> Interesting idea, but I think it has a couple of issues :-(
> 
> Thanks for the review!
> 
>> 1) We'd need to know the LSN of the last WAL record for any given 
>> sequence, and we'd need to communicate that between backends somehow. 
>> Which seems rather tricky to do without affecting performance.
> 
> How about using the page lsn for the sequence? nextval_internal() 
> already uses that to check whether it's less than or equal to checkpoint 
> redo location.
> 

Hmm, maybe.

> 
>> 2) SyncRepWaitForLSN() is used only in commit-like situations, and 
>> it's a simple wait, not a decision to write more WAL. Environments 
>> without sync replicas are affected by this too - yes, the data loss 
>> issue is not there, but the amount of WAL is still increased.
> 
> How about reusing only a part of code in SyncRepWaitForLSN()? Attached 
> is the PoC patch that implemented what I'm thinking.
> 
> 
>> IIRC sync_standby_names can change while a transaction is running, 
>> even just right before commit, at which point we can't just go back in 
>> time and generate WAL for sequences accessed earlier. But we still 
>> need to ensure the sequence is properly replicated.
> 
> Yes. In the PoC patch, SyncRepNeedsWait() still checks 
> sync_standbys_defined and uses SyncRepWaitMode. But they should not be 
> checked nor used because their values can be changed on the fly, as you 
> pointed out. Probably SyncRepNeedsWait() will need to be changed so that 
> it doesn't use them.
> 

Right. I think the data loss with sync standby is merely a symptom, not 
the root cause. We'd need to deduce the LSN for which to wait at commit.

> 
>> 3) I don't think it'd actually reduce the amount of WAL records in 
>> environments with many sessions (incrementing the same sequence). In 
>> those cases the WAL (generated by in-progress xact from another 
>> session) is likely to not be flushed, so we'd generate the extra WAL 
>> record. (And if the other backends would need flush LSN of this new 
>> WAL record, which would make it more likely they have to generate WAL 
>> too.)
> 
> With the PoC patch, only when previous transaction that executed 
> nextval() and caused WAL record is aborted, subsequent nextval() 
> generates additional WAL record. So this approach can reduce WAL volume 
> than other approach?
>  > In the PoC patch, to reduce WAL volume more, it might be better to make
> nextval_internal() update XactLastRecEnd and assign XID rather than 
> emitting new WAL record, when SyncRepNeedsWait() returns true.
> 

Yes, but I think there are other cases. For example the WAL might have 
been generated by another backend, in a transaction that might be still 
running. In which case I don't see how updating XactLastRecEnd in 
nextval_internal would fix this, right?

I did some experiments with increasing CACHE for the sequence, and that 
mostly eliminates the overhead. See the message I sent a couple minutes 
ago. IMHO that's a reasonable solution for the tiny number of people 
using nextval() in a way that'd be affected by this (i.e. without 
writing anything else in the xact).


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company





view thread (78+ messages)  latest in thread

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]
  Subject: Re: sequences vs. synchronous replication
  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