Message-ID: <07301efe-2f6f-145c-dbfd-20991c89f586@enterprisedb.com>
Date: Wed, 14 Jun 2023 17:27:10 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.0
Subject: Re: TRAP: FailedAssertion("prev_first_lsn < cur_txn->first_lsn",
 File: "reorderbuffer.c", Line: 927, PID: 568639)
Content-Language: en-US
To: "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>,
 Amit Kapila <amit.kapila16@gmail.com>
Cc: Alexander Lakhin <exclusion@gmail.com>,
 Masahiko Sawada <sawada.mshk@gmail.com>,
 pgsql-hackers <pgsql-hackers@postgresql.org>
References: <a89b46b6-0239-2fd5-71a9-b19b1f7a7145@enterprisedb.com>
 <CAD21AoCf1Q+2feZ8u7JhHfOrcV0FX+8uRPZJA2chyTsYq-g0fA@mail.gmail.com>
 <CAA4eK1Lx=g09z2k9Teq9ca1eRzfpfxJwFdjyHNwgEKv69KWhrQ@mail.gmail.com>
 <f158d9ca-2057-2836-a522-0b1278be5a53@gmail.com>
 <99b45770-e3d8-4522-ea52-f4509931c65b@enterprisedb.com>
 <1f860e40-9f95-f0f8-ff4b-4b780f9151ad@gmail.com>
 <bedc421a-3cdd-ca5a-7b9e-bc6da989f500@enterprisedb.com>
 <ff248f9a-2bf8-33af-c276-1796725ca8ba@enterprisedb.com>
 <CAA4eK1Kro0rej=ZXMhcdjs+aYsZvNywu3-cqdRUtyAp4zqpVWw@mail.gmail.com>
 <45567656-fb0e-c844-48fb-f4c2a54dc2af@enterprisedb.com>
 <CAA4eK1+3+9V=6KSdsaCQjxh5unJVSzNWc_6m-nr9yP-uqpqgpw@mail.gmail.com>
 <OS0PR01MB57162DAF28227A05581957519455A@OS0PR01MB5716.jpnprd01.prod.outlook.com>
 <OS0PR01MB5716026D3B11228010946CF9945AA@OS0PR01MB5716.jpnprd01.prod.outlook.com>
 <e6389429-f7ff-6d62-8880-89d272f6d64e@enterprisedb.com>
 <OS0PR01MB5716461EBCEBDEB29D7F54B6945AA@OS0PR01MB5716.jpnprd01.prod.outlook.com>
From: Tomas Vondra <tomas.vondra@enterprisedb.com>
In-Reply-To: 
 <OS0PR01MB5716461EBCEBDEB29D7F54B6945AA@OS0PR01MB5716.jpnprd01.prod.outlook.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/07301efe-2f6f-145c-dbfd-20991c89f586%40enterprisedb.com>
Precedence: bulk


On 6/14/23 15:39, Zhijie Hou (Fujitsu) wrote:
> On Wednesday, June 14, 2023 5:05 PM Tomas Vondra <tomas.vondra@enterprisedb.com> wrote:
>> ...
>>
>> Also, can you try if we still stream the partial transaction with
>> create_logical_replication_slot building a full snapshot?
> 
> Yes, It can fix this problem because force create_logical_replication_slot
> build a full snapshot can avoid restoring the snapshot. But I am not sure if
> this is the best fix for this for the same reason(it's costly) mentioned by
> Amit[1].
> 

Costly compared to the current behavior? Sure, but considering the
current behavior is incorrect/broken, that seems rather irrelevant.
Incorrect behavior can be infinitely faster.

I doubt it's significantly costlier than just setting the "full
snapshot" flag when building the initial snapshot - sure, it will take
more time than now, but that's kinda the whole point. It seems to me the
problem is exactly that it *doesn't* wait long enough.

I may be misunderstanding the solution you proposed, but this:

    One idea to fix the partial change stream problem would be that we
    record all the running transaction's xid when restoring the snapshot
    in SnapBuildFindSnapshot(), and in the following decoding, we skip
    decoding changes for the recorded transaction

sounds pretty close to what building a correct snapshot actually does.

But maybe I'm wrong - ultimately, the way to compare those approaches
seems to be to prototype this proposal, and do some tests.

There's also the question of back branches, and it seems way simpler to
just flip a flag and disable broken optimization than doing fairly
complex stuff to save it.

I'd also point out that (a) this only affects the initial snapshot, not
every time we start the decoding context, and (b) the slots created from
walsender already do that with (unless when copy_data=false).

So if needs_full_snapshot=true fixes the issue, I'd just do that as the
first step - in master and backpatches. And then if we want to salvage
the optimization, we can try fixing it in master.


regards

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