Message-ID: <27f005a6-63f7-2ca9-2ab3-92fe9110e224@enterprisedb.com>
Date: Fri, 24 Dec 2021 11:40:20 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.2.0
Subject: Re: sequences vs. synchronous replication
Content-Language: en-US
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: masao.fujii@oss.nttdata.com, tgl@sss.pgh.pa.us,
 peter.eisentraut@enterprisedb.com, pgsql-hackers@lists.postgresql.org
References: <a58f1c38-10eb-7d7f-372c-cfb95007d54e@enterprisedb.com>
 <20211224.143725.1750248643492340056.horikyota.ntt@gmail.com>
 <ad83e515-44f6-4691-8fe0-503b94499ed5@enterprisedb.com>
 <20211224.170451.116141220257426861.horikyota.ntt@gmail.com>
From: Tomas Vondra <tomas.vondra@enterprisedb.com>
In-Reply-To: <20211224.170451.116141220257426861.horikyota.ntt@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/27f005a6-63f7-2ca9-2ab3-92fe9110e224%40enterprisedb.com>
Precedence: bulk


On 12/24/21 09:04, Kyotaro Horiguchi wrote:
>>> ...
>>> So, strictly speaking, that is a violation of the constraint I
>>> mentioned regardless whether the transaction is committed or
>>> not. However we have technical limitations as below.
>>>
>>
>> I don't follow. What violates what?
>>
>> If the transaction commits (and gets a confirmation from sync
>> replica), the modified WAL logging prevents duplicate values. It does
>> nothing for uncommitted transactions. Seems like an improvement to me.
> 
> Sorry for the noise. I misunderstand that ROLLBACK is being changed to
> rollback sequences.
> 

No problem, this part of the code is certainly rather confusing due to 
several layers of caching and these WAL-logging optimizations.

>> No idea. IMHO from the correctness / behavior point of view, the
>> modified logging is an improvement. The only issue is the additional
>> overhead, and I think the cache addresses that quite well.
> 
> Now I understand the story here.
> 
> I agree that the patch is improvment from the current behavior.
> I agree that the overhead is eventually-nothing for WAL-emitting workloads.
> 

OK, thanks.

> Still, as Fujii-san concerns, I'm afraid that some people may suffer
> the degradation the patch causes.  I wonder it is acceptable to get
> back the previous behavior by exposing SEQ_LOG_VALS itself or a
> boolean to do that, as a 'not-recommended-to-use' variable.
> 

Maybe, but what would such workload look like? Based on the tests I did, 
such workload probably can't generate any WAL. The amount of WAL added 
by the change is tiny, the regression is caused by having to flush WAL.

The only plausible workload I can think of is just calling nextval, and 
the cache pretty much fixes that.

FWIW I plan to explore the idea of looking at sequence page LSN, and 
flushing up to that position.

regards

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