Horton

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

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

View thread
From: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
To: 'Amit Kapila' <amit.kapila16@gmail.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-17T08:37:33Z
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 →

  1. Fix typo in test file name added in commit 4909b38af0.

  2. Fix data loss in logical replication.

  3. Avoid invalidating all RelationSyncCache entries on publication rename.

  4. Remove obsolete RECHECK keyword completely

  5. Backport BackgroundPsql perl test module

Attachments

Dear Amit,

> 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.

Sounds reasonable. I introduced new function ReorderBufferGetInvalidations() which
obtains the number of invalidations and its list. ReorderBufferTXN() is not exported
anymore.

> 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]?

You meant that we need to test only the case reported by the Andres, right? New
version did like that. To make the test faster test was migrated to isolation tester
instead of the TAP test.

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

Thanks, I included.

PSA new version for PG 14-master. Special thanks for Hou to minimize the test code.

Best regards,
Hayato Kuroda
FUJITSU LIMITED