pg.ddx.io pgsql-committers@postgresql.org mailing list archivehelp / color / mirror / Atom feed
pgsql: Fix data loss in logical replication. 6+ messages / 1 participants [nested] [flat]
* pgsql: Fix data loss in logical replication. @ 2025-04-10 08:01 Amit Kapila <akapila@postgresql.org> 0 siblings, 0 replies; 6+ messages in thread From: Amit Kapila @ 2025-04-10 08:01 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix data loss in logical replication. 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 <depesz@depesz.com> Reported-by: Tomas Vondra <tomas.vondra@enterprisedb.com> Author: Shlok Kyal <shlok.kyal.oss@gmail.com> Author: Hayato Kuroda <kuroda.hayato@fujitsu.com> Reviewed-by: Zhijie Hou <houzj.fnst@fujitsu.com> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Tested-by: Benoit Lobréau <benoit.lobreau@dalibo.com> 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 Branch ------ REL_15_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/9f21be08e884c75089e939bae86a47eca176e0af Modified Files -------------- contrib/test_decoding/Makefile | 2 +- .../expected/invalidation_distrubution.out | 20 +++++++ .../specs/invalidation_distrubution.spec | 32 +++++++++++ src/backend/replication/logical/reorderbuffer.c | 23 ++++++++ src/backend/replication/logical/snapbuild.c | 67 +++++++++++++++++----- src/include/replication/reorderbuffer.h | 4 ++ 6 files changed, 133 insertions(+), 15 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Fix data loss in logical replication. @ 2025-04-10 08:01 Amit Kapila <akapila@postgresql.org> 0 siblings, 0 replies; 6+ messages in thread From: Amit Kapila @ 2025-04-10 08:01 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix data loss in logical replication. 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 <depesz@depesz.com> Reported-by: Tomas Vondra <tomas.vondra@enterprisedb.com> Author: Shlok Kyal <shlok.kyal.oss@gmail.com> Author: Hayato Kuroda <kuroda.hayato@fujitsu.com> Reviewed-by: Zhijie Hou <houzj.fnst@fujitsu.com> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Tested-by: Benoit Lobréau <benoit.lobreau@dalibo.com> 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 Branch ------ REL_17_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/cadaf0ac46374360e178f4ac120a7e29438b4dc0 Modified Files -------------- contrib/test_decoding/Makefile | 2 +- .../expected/invalidation_distrubution.out | 20 +++++++ contrib/test_decoding/meson.build | 1 + .../specs/invalidation_distrubution.spec | 32 +++++++++++ src/backend/replication/logical/reorderbuffer.c | 23 ++++++++ src/backend/replication/logical/snapbuild.c | 67 +++++++++++++++++----- src/include/replication/reorderbuffer.h | 4 ++ 7 files changed, 134 insertions(+), 15 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Fix data loss in logical replication. @ 2025-04-10 08:01 Amit Kapila <akapila@postgresql.org> 0 siblings, 0 replies; 6+ messages in thread From: Amit Kapila @ 2025-04-10 08:01 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix data loss in logical replication. 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 <depesz@depesz.com> Reported-by: Tomas Vondra <tomas.vondra@enterprisedb.com> Author: Shlok Kyal <shlok.kyal.oss@gmail.com> Author: Hayato Kuroda <kuroda.hayato@fujitsu.com> Reviewed-by: Zhijie Hou <houzj.fnst@fujitsu.com> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Tested-by: Benoit Lobréau <benoit.lobreau@dalibo.com> 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 Branch ------ REL_16_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/9a2f8b4f01d5a40b8b080f6b108ae314a6730cec Modified Files -------------- contrib/test_decoding/Makefile | 2 +- .../expected/invalidation_distrubution.out | 20 +++++++ contrib/test_decoding/meson.build | 1 + .../specs/invalidation_distrubution.spec | 32 +++++++++++ src/backend/replication/logical/reorderbuffer.c | 23 ++++++++ src/backend/replication/logical/snapbuild.c | 67 +++++++++++++++++----- src/include/replication/reorderbuffer.h | 4 ++ 7 files changed, 134 insertions(+), 15 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Fix data loss in logical replication. @ 2025-04-10 08:01 Amit Kapila <akapila@postgresql.org> 0 siblings, 0 replies; 6+ messages in thread From: Amit Kapila @ 2025-04-10 08:01 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix data loss in logical replication. 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 <depesz@depesz.com> Reported-by: Tomas Vondra <tomas.vondra@enterprisedb.com> Author: Shlok Kyal <shlok.kyal.oss@gmail.com> Author: Hayato Kuroda <kuroda.hayato@fujitsu.com> Reviewed-by: Zhijie Hou <houzj.fnst@fujitsu.com> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Tested-by: Benoit Lobréau <benoit.lobreau@dalibo.com> 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 Branch ------ REL_14_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/0434033e8bb535dd809e97c24202be5ef620aaff Modified Files -------------- contrib/test_decoding/Makefile | 3 +- .../expected/invalidation_distrubution.out | 20 +++++++ .../specs/invalidation_distrubution.spec | 32 +++++++++++ src/backend/replication/logical/reorderbuffer.c | 23 ++++++++ src/backend/replication/logical/snapbuild.c | 67 +++++++++++++++++----- src/include/replication/reorderbuffer.h | 4 ++ 6 files changed, 134 insertions(+), 15 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Fix data loss in logical replication. @ 2025-04-10 08:01 Amit Kapila <akapila@postgresql.org> 0 siblings, 0 replies; 6+ messages in thread From: Amit Kapila @ 2025-04-10 08:01 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix data loss in logical replication. 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 <depesz@depesz.com> Reported-by: Tomas Vondra <tomas.vondra@enterprisedb.com> Author: Shlok Kyal <shlok.kyal.oss@gmail.com> Author: Hayato Kuroda <kuroda.hayato@fujitsu.com> Reviewed-by: Zhijie Hou <houzj.fnst@fujitsu.com> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Tested-by: Benoit Lobréau <benoit.lobreau@dalibo.com> 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 Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/4909b38af034fa4d2c67c5c71fd8509f870c1695 Modified Files -------------- contrib/test_decoding/Makefile | 2 +- .../expected/invalidation_distrubution.out | 20 +++++++ contrib/test_decoding/meson.build | 1 + .../specs/invalidation_distrubution.spec | 32 ++++++++++ src/backend/replication/logical/reorderbuffer.c | 23 ++++++++ src/backend/replication/logical/snapbuild.c | 68 +++++++++++++++++----- src/include/replication/reorderbuffer.h | 4 ++ 7 files changed, 135 insertions(+), 15 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Fix data loss in logical replication. @ 2025-04-24 05:10 Amit Kapila <akapila@postgresql.org> 0 siblings, 0 replies; 6+ messages in thread From: Amit Kapila @ 2025-04-24 05:10 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix data loss in logical replication. This commit is a backpatch of commit 4909b38af0 for 13. 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. The fix for 13 is different from what we did in branches 14 and above, such that for 13, the concurrent DDL changes (from DDL types mentioned earlier) will be visible for any newly started transactions. To make them visible in concurrent transactions, we need to introduce a new change type REORDER_BUFFER_CHANGE_INVALIDATION, already present in branches 14 and greater. We decided not to take the risk of a bigger change and fix the issue partially in 13. Reported-by: hubert depesz lubaczewski <depesz@depesz.com> Reported-by: Tomas Vondra <tomas.vondra@enterprisedb.com> Author: Shlok Kyal <shlok.kyal.oss@gmail.com> Author: Hayato Kuroda <kuroda.hayato@fujitsu.com> Reviewed-by: Zhijie Hou <houzj.fnst@fujitsu.com> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Tested-by: Benoit Lobréau <benoit.lobreau@dalibo.com> Discussion: https://postgr.es/m/de52b282-1166-1180-45a2-8d8917ca74c6@enterprisedb.com Discussion: https://postgr.es/m/CAD21AoAenVqiMjpN-PvGHL1N9DWnHSq673bfgr6phmBUzx=kLQ@mail.gmail.com Discussion: https://postgr.es/m/CAD21AoAhU3kp8shYqP=ExiFDZ9sZxpFb5WzLa0p+vEe5j+7CWQ@mail.gmail.com Branch ------ REL_13_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/247ee94150b6fe8906da51afadbedf8acf3c17cf Modified Files -------------- contrib/test_decoding/Makefile | 2 +- .../expected/invalidation_distrubution.out | 20 +++++++ .../specs/invalidation_distrubution.spec | 32 +++++++++++ src/backend/replication/logical/reorderbuffer.c | 64 +++++++++++++++++++--- src/backend/replication/logical/snapbuild.c | 63 +++++++++++++++++---- src/include/replication/reorderbuffer.h | 4 ++ 6 files changed, 164 insertions(+), 21 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2025-04-24 05:10 UTC | newest] Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2025-04-10 08:01 pgsql: Fix data loss in logical replication. Amit Kapila <akapila@postgresql.org> 2025-04-10 08:01 pgsql: Fix data loss in logical replication. Amit Kapila <akapila@postgresql.org> 2025-04-10 08:01 pgsql: Fix data loss in logical replication. Amit Kapila <akapila@postgresql.org> 2025-04-10 08:01 pgsql: Fix data loss in logical replication. Amit Kapila <akapila@postgresql.org> 2025-04-10 08:01 pgsql: Fix data loss in logical replication. Amit Kapila <akapila@postgresql.org> 2025-04-24 05:10 pgsql: Fix data loss in logical replication. Amit Kapila <akapila@postgresql.org>
This inbox is served by DDX for PostgreSQL; see mirroring instructions for how to clone and mirror all data and code used for this inbox