From 9f21be08e884c75089e939bae86a47eca176e0af Mon Sep 17 00:00:00 2001 From: Amit Kapila Date: Thu, 10 Apr 2025 12:31:14 +0530 Subject: Fix data loss in logical replication. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Data loss can happen when the DDLs like ALTER PUBLICATION ... ADD TABLE ... or ALTER TYPE ... that don't take a strong lock on table happens concurrently to DMLs on the tables involved in the DDL. This happens because logical decoding doesn't distribute invalidations to concurrent transactions and those transactions use stale cache data to decode the changes. The problem becomes bigger because we keep using the stale cache even after those in-progress transactions are finished and skip the changes required to be sent to the client. This commit fixes the issue by distributing invalidation messages from catalog-modifying transactions to all concurrent in-progress transactions. This allows the necessary rebuild of the catalog cache when decoding new changes after concurrent DDL. We observed performance regression primarily during frequent execution of *publication DDL* statements that modify the published tables. The regression is minor or nearly nonexistent for DDLs that do not affect the published tables or occur infrequently, making this a worthwhile cost to resolve a longstanding data loss issue. An alternative approach considered was to take a strong lock on each affected table during publication modification. However, this would only address issues related to publication DDLs (but not the ALTER TYPE ...) and require locking every relation in the database for publications created as FOR ALL TABLES, which is impractical. The bug exists in all supported branches, but we are backpatching till 14. The fix for 13 requires somewhat bigger changes than this fix, so the fix for that branch is still under discussion. Reported-by: hubert depesz lubaczewski Reported-by: Tomas Vondra Author: Shlok Kyal Author: Hayato Kuroda Reviewed-by: Zhijie Hou Reviewed-by: Masahiko Sawada Reviewed-by: Amit Kapila Tested-by: Benoit Lobréau Backpatch-through: 14 Discussion: https://postgr.es/m/de52b282-1166-1180-45a2-8d8917ca74c6@enterprisedb.com Discussion: https://postgr.es/m/CAD21AoAenVqiMjpN-PvGHL1N9DWnHSq673bfgr6phmBUzx=kLQ@mail.gmail.com --- src/backend/replication/logical/reorderbuffer.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) (limited to 'src/backend/replication/logical/reorderbuffer.c') diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c index d5cde20a1c9..835180e12f6 100644 --- a/src/backend/replication/logical/reorderbuffer.c +++ b/src/backend/replication/logical/reorderbuffer.c @@ -5200,3 +5200,26 @@ restart: *cmax = ent->cmax; return true; } + +/* + * Count invalidation messages of specified transaction. + * + * Returns number of messages, and msgs is set to the pointer of the linked + * list for the messages. + */ +uint32 +ReorderBufferGetInvalidations(ReorderBuffer *rb, TransactionId xid, + SharedInvalidationMessage **msgs) +{ + ReorderBufferTXN *txn; + + txn = ReorderBufferTXNByXid(rb, xid, false, NULL, InvalidXLogRecPtr, + false); + + if (txn == NULL) + return 0; + + *msgs = txn->invalidations; + + return txn->ninvalidations; +} -- cgit v1.2.3