Re: long-standing data loss bug in initial sync of logical replication

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, Nitin Motiani <nitinmotiani(at)google(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Subject: Re: long-standing data loss bug in initial sync of logical replication
Date: 2025-03-17 05:41:39
Message-ID: CAA4eK1JuCGnEsj_84w+EEbfhJgKKSYyZXPn1CK-TSOFV3KH5+g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Mar 17, 2025 at 6:53 AM Hayato Kuroda (Fujitsu)
<kuroda(dot)hayato(at)fujitsu(dot)com> wrote:
>
> OK, let me share patched for back branches. Mostly the same fix patched as master
> can be used for PG14-PG17, like attached.
>

A few comments:
==============
1.
+SnapBuildDistributeSnapshotAndInval(SnapBuild *builder, XLogRecPtr
lsn, TransactionId xid)
{
dlist_iter txn_i;
ReorderBufferTXN *txn;
+ ReorderBufferTXN *curr_txn;
+
+ curr_txn = ReorderBufferTXNByXid(builder->reorder, xid, false, NULL,
InvalidXLogRecPtr, false);

The above is used to access invalidations from curr_txn. I am thinking
about whether it would be better to expose a new function to get
invalidations for a txn based on xid instead of getting
ReorderBufferTXN. It would avoid any layering violation and misuse of
ReorderBufferTXN by other modules.

2. The patch has a lot of tests to verify the same points. Can't we
have one simple test using SQL API based on what Andres presented in
an email [1]?

3. I have made minor changes in the comments in the attached.

[1] - https://www.postgresql.org/message-id/20231119021830.d6t6aaxtrkpn743y%40awork3.anarazel.de

--
With Regards,
Amit Kapila.

Attachment Content-Type Size
v18-0001-amit.diff.txt text/plain 1.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Shubham Khanna 2025-03-17 05:57:58 Re: Enhance 'pg_createsubscriber' to retrieve databases automatically when no database is provided.
Previous Message Daniil Davydov 2025-03-17 05:11:46 Re: Forbid to DROP temp tables of other sessions