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

Amit Kapila <amit.kapila16@gmail.com>

From: Amit Kapila <amit.kapila16@gmail.com>

To: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>

Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>, Shlok Kyal <shlok.kyal.oss@gmail.com>, vignesh C <vignesh21@gmail.com>, Nitin Motiani <nitinmotiani@google.com>, Andres Freund <andres@anarazel.de>, Masahiko Sawada <sawada.mshk@gmail.com>

Date: 2025-03-17T05:41:39Z

Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

Fix typo in test file name added in commit 4909b38af0.
- 50b8ad30f754 18.0 landed
- d96206f259d6 17.5 landed
- 9987c94662c2 16.9 landed
- 90bc4523fd47 15.13 landed
- bb1bc9fa962e 14.18 landed
- 4164d6976316 13.21 landed
Fix data loss in logical replication.
- 247ee94150b6 13.21 landed
- 4909b38af034 18.0 landed
- cadaf0ac4637 17.5 landed
- 9a2f8b4f01d5 16.9 landed
- 9f21be08e884 15.13 landed
- 0434033e8bb5 14.18 landed
Avoid invalidating all RelationSyncCache entries on publication rename.
- 3abe9dc18892 18.0 cited
Remove obsolete RECHECK keyword completely
- 7da1bdc2c2f1 18.0 cited
Backport BackgroundPsql perl test module
- 187b8991f70f 16.4 cited

Attachments

v18-0001-amit.diff.txt (text/plain)

On Mon, Mar 17, 2025 at 6:53 AM Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.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.