Message-ID: <241d6c00-7997-480f-80c1-636b28d457cf@enterprisedb.com>
Date: Thu, 15 Feb 2024 21:26:59 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: logical decoding and replication of sequences, take 2
Content-Language: en-US
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>
 <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>
 <929a38f6-b9f9-4cc4-926a-ac972e769e17@enterprisedb.com>
 <CA+TgmoaPTZaFTrDmvmH69Wq40RXor4VxCL99s1irfdKkMZsYjA@mail.gmail.com>
 <996e4d3c-648a-46d0-86d2-6465f53ddc58@enterprisedb.com>
 <CA+TgmoaSCu=bLtU7Zyth=h2Lgtzxv=xfRwLYt2ZtSKA=UN2SnQ@mail.gmail.com>
 <5bbadb71-b51e-45a1-9acc-d54f5e8850b6@enterprisedb.com>
 <CA+Tgmoaxertx9iuSeGOoocvq=5GTgZyXZvZBf4w24mQpyz7LRA@mail.gmail.com>
From: Tomas Vondra <tomas.vondra@enterprisedb.com>
In-Reply-To: <CA+Tgmoaxertx9iuSeGOoocvq=5GTgZyXZvZBf4w24mQpyz7LRA@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Archived-At: <https://www.postgresql.org/message-id/241d6c00-7997-480f-80c1-636b28d457cf%40enterprisedb.com>
Precedence: bulk

On 2/15/24 05:16, Robert Haas wrote:
> On Wed, Feb 14, 2024 at 10:21 PM Tomas Vondra
> <tomas.vondra@enterprisedb.com> wrote:
>> The way I think about non-transactional sequence changes is as if they
>> were tiny transactions that happen "fully" (including commit) at the LSN
>> where the LSN change is logged.
> 
> 100% this.
> 
>> It does not mean we can arbitrarily reorder the changes, it only means
>> the changes are applied as if they were independent transactions (but in
>> the same order as they were executed originally). Both with respect to
>> the other non-transactional changes, and to "commits" of other stuff.
> 
> Right, this is very important and I agree completely.
> 
> I'm feeling more confident about this now that I heard you say that
> stuff -- this is really the key issue I've been worried about since I
> first looked at this, and I wasn't sure that you were in agreement,
> but it sounds like you are. I think we should (a) fix the locking bug
> I found (but that can be independent of this patch) and (b) make sure
> that this patch documents the points from the quoted material above so
> that everyone who reads the code (and maybe tries to enhance it) is
> clear on what the assumptions are.
> 
> (I haven't checked whether it documents that stuff or not. I'm just
> saying it should, because I think it's a subtlety that someone might
> miss.)
> 

Thanks for thinking about these issues with reordering events. Good we
seem to be in agreement and that you feel more confident about this.
I'll check if there's a good place to document this.

For me, the part that I feel most uneasy about is the decoding while the
snapshot is still being built (and can flip to consistent snapshot
between the relfilenode creation and sequence change, confusing the
logic that decides which changes are transactional).

It seems "a bit weird" that we either keep the "simple" logic that may
end up with incorrect "non-transactional" result, but happens to then
work fine because we immediately discard the change.

But it still feels better than the alternative, which requires us to
start decoding stuff (relfilenode creation) before building a proper
snapshot is consistent, which we didn't do before - or at least not in
this particular way. While I don't have a practical example where it
would cause trouble now, I have a nagging feeling it might easily cause
trouble in the future by making some new features harder to implement.


regards

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