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

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

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

To: 'Shlok Kyal' <shlok.kyal.oss@gmail.com>

Cc: Masahiko Sawada <sawada.mshk@gmail.com>, vignesh C <vignesh21@gmail.com>, Nitin Motiani <nitinmotiani@google.com>, Andres Freund <andres@anarazel.de>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Amit Kapila <amit.kapila16@gmail.com>

Date: 2024-10-03T10:15:38Z

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

add_invalidations.diffs (application/octet-stream)

Dear Shlok-san,

Thanks for updating the patch. Here are comments.

1.
I feel the name of SnapBuildDistributeNewCatalogSnapshot() should be updated because it
distributes two objects: catalog snapshot and invalidation messages. Do you have good one
in your mind? I considered "SnapBuildDistributeNewCatalogSnapshotAndInValidations" or
"SnapBuildDistributeItems" but seems not good :-(.

2.
Hmm, still, it is overengineering for me to add a new type of invalidation message
only for the publication. According to the ExecRenameStmt() we can implement an
arbitrary rename function like RenameConstraint() and RenameDatabase().
Regaring the ALTER PUBLICATION OWNER TO, I feel adding CacheInvalidateRelcacheAll()
and InvalidatePublicationRels() is enough.


I attached a PoC which implements above. It could pass tests on my env. Could you
please see it tell me how you think?

Best regards,
Hayato Kuroda
FUJITSU LIMITED