Message-ID: <929a38f6-b9f9-4cc4-926a-ac972e769e17@enterprisedb.com>
Date: Wed, 24 Jan 2024 18:46:37 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: logical decoding and replication of sequences, take 2
To: Robert Haas <robertmhaas@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>,
 Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>,
 Dilip Kumar <dilipbalaut@gmail.com>,
 "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>,
 "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
 Masahiko Sawada <sawada.mshk@gmail.com>,
 Peter Eisentraut <peter.eisentraut@enterprisedb.com>
References: <c5e671bc-3400-c2f4-14d3-1ae06c462dad@enterprisedb.com>
 <CAFiTN-vAx-Y+19ROKOcWnGf7ix2VOTUebpzteaGw9XQyCAeK6g@mail.gmail.com>
 <b6f118e6-02b8-759c-db15-f9b25e805d65@enterprisedb.com>
 <CAFiTN-un8w_owWokm=425=ucSL90fHxJShHUq=Zip=mfWbt1Dw@mail.gmail.com>
 <CAA4eK1LFise9iN+NN=agrk4prR1qD+ebvzNjKAWUog2+hy3HxQ@mail.gmail.com>
 <CAFiTN-sYpyUBabxopJysqH3DAp4OZUCTi6m_qtgt8d32vDcWSA@mail.gmail.com>
 <CAExHW5szMs=Wzy1Ek00rkByQbTk57nK_UvMp+VYwT9Np6LnO5A@mail.gmail.com>
 <CAA4eK1L3rxS9zyvV-SOqOTx1Rqa_9B0UxuHWX3rHfUC7z6dc6w@mail.gmail.com>
 <CAExHW5uRGwr0hAC5Gr4EbA4Q=Odjk+8nFTmdsfB1Hk-9r-81DA@mail.gmail.com>
 <CAA4eK1+pkuJhGZNA9RHOGDBUF2hhmyUr6gVCOSCTZ3wDuGbzdg@mail.gmail.com>
 <CAExHW5tR+rmrarU1m0DCmhq0e7d00XDbdhcHJV+iGkcPTfo8hg@mail.gmail.com>
 <CAA4eK1J-r8Y00CYn6WT4vexZbG5OandDrm5gcb+9Y_CCKVUkag@mail.gmail.com>
 <577ec4fc-62c5-cb4a-de0f-66dee8232d00@enterprisedb.com>
 <09613730-5ee9-4cc3-82d8-f089be90aa64@enterprisedb.com>
 <CA+TgmoY9tCqc_Tg4Bp8zvxvzzzDJbnopvhyVU7CwWnO21Y=Z2Q@mail.gmail.com>
Content-Language: en-US
From: Tomas Vondra <tomas.vondra@enterprisedb.com>
In-Reply-To: 
 <CA+TgmoY9tCqc_Tg4Bp8zvxvzzzDJbnopvhyVU7CwWnO21Y=Z2Q@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/929a38f6-b9f9-4cc4-926a-ac972e769e17%40enterprisedb.com>
Precedence: bulk


On 1/23/24 21:47, Robert Haas wrote:
> On Thu, Jan 11, 2024 at 11:27 AM Tomas Vondra
> <tomas.vondra@enterprisedb.com> wrote:
>> 1) desirability: We want a built-in way to handle sequences in logical
>> replication. I think everyone agrees this is not a way to do distributed
>> sequences in an active-active setups, but that there are other use cases
>> that need this feature - typically upgrades / logical failover.
> 
> Yeah. I find it extremely hard to take seriously the idea that this
> isn't a valuable feature. How else are you supposed to do a logical
> failover without having your entire application break?
> 
>> 2) performance: There was concern about the performance impact, and that
>> it affects everyone, including those who don't replicate sequences (as
>> the overhead is mostly incurred before calls to output plugin etc.).
>>
>> The agreement was that the best way is to have a CREATE SUBSCRIPTION
>> option that would instruct the upstream to decode sequences. By default
>> this option is 'off' (because that's the no-overhead case), but it can
>> be enabled for each subscription.
> 
> Seems reasonable, at least unless and until we come up with something better.
> 
>> 3) correctness: The last point is about making "transactional" flag
>> correct when the snapshot state changes mid-transaction, originally
>> pointed out by Dilip [4]. Per [5] this however happens to work
>> correctly, because while we identify the change as 'non-transactional'
>> (which is incorrect), we immediately throw it again (so we don't try to
>> apply it, which would error-out).
> 
> I've said this before, but I still find this really scary. It's
> unclear to me that we can simply classify updates as transactional or
> non-transactional and expect things to work. If it's possible, I hope
> we have a really good explanation somewhere of how and why it's
> possible. If we do, can somebody point me to it so I can read it?
> 

I did try to explain how this works (and why) in a couple places:

1) the commit message
2) reorderbuffer header comment
3) ReorderBufferSequenceIsTransactional comment (and nearby)

It's possible this does not meet your expectations, ofc. Maybe there
should be a separate README for this - I haven't found anything like
that for logical decoding in general, which is why I did (1)-(3).

> To be possibly slightly more clear about my concern, I think the scary
> case is where we have transactional and non-transactional things
> happening to the same sequence in close temporal proximity, either
> within the same session or across two or more sessions.  If a
> non-transactional change can get reordered ahead of some transactional
> change upon which it logically depends, or behind some transactional
> change that logically depends on it, then we have trouble. I also
> wonder if there are any cases where the same operation is partly
> transactional and partly non-transactional.
> 

I certainly understand this concern, and to some extent I even share it.
Having to differentiate between transactional and non-transactional
changes certainly confused me more than once. It's especially confusing,
because the decoding implicitly changes the perceived ordering/atomicity
of the events.

That being said, I don't think it get reordered the way you're concerned
about. The "transactionality" is determined by relfilenode change, so
how could the reordering happen? We'd have to misidentify change in
either direction - and for nontransactional->transactional change that's
clearly not possible. There has to be a new relfilenode in that xact.

In the other direction (transactional->nontransactional), it can happen
if we fail to decode the relfilenode record. Which is what we discussed
earlier, but came to the conclusion that it actually works OK.

Of course, there might be bugs. I spent quite a bit of effort reviewing
and testing this, but there still might be something wrong. But I think
that applies to any feature.

What would be worse is some sort of thinko in the approach in general. I
don't have a good answer to that, unfortunately - I think it works, but
how would I know for sure? We explored multiple alternative approaches
and all of them crashed and burned ...


regards

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