public inbox for [email protected]
help / color / mirror / Atom feedFrom: [email protected] <[email protected]>
To: [email protected] <[email protected]>
To: [email protected] <[email protected]>
Cc: Amit Langote <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Dilip Kumar <[email protected]>
Cc: Greg Nancarrow <[email protected]>
Cc: vignesh C <[email protected]>
Cc: Amit Kapila <[email protected]>
Subject: RE: Data is copied twice when specifying both child and parent table in publication
Date: Tue, 19 Apr 2022 08:53:09 +0000
Message-ID: <OSZPR01MB63109BE4F5521C542A75AC9BFDF29@OSZPR01MB6310.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <OS0PR01MB5716546D48042D44C3F0675794F29@OS0PR01MB5716.jpnprd01.prod.outlook.com>
References: <CAA4eK1+RyFHLv1K+3qCdRmqEO0bz68Ga=Tzomvq4Rc40HnNO8A@mail.gmail.com>
<CAA4eK1Jy126np5v8wc-k-MTbnneYDhXrjdC0oNriU8ibGcMdGg@mail.gmail.com>
<OS0PR01MB5716DC2982CC735FDE388804940B9@OS0PR01MB5716.jpnprd01.prod.outlook.com>
<OS3PR01MB6275CD6236DEB022C0785E9B9EE69@OS3PR01MB6275.jpnprd01.prod.outlook.com>
<OS0PR01MB5716546D48042D44C3F0675794F29@OS0PR01MB5716.jpnprd01.prod.outlook.com>
On Tue, Apr 19, 2022 3:05 PM [email protected] <[email protected]> wrote:
>
> > -----Original Message-----
> > From: Wang, Wei/王 威 <[email protected]>
> On Thursday, April 7, 2022 11:08 AM
> >
> > On Thur, Mar 10, 2021 at 10:08 AM [email protected] wrote:
> > > Hi,
> > >
> > > When reviewing some logical replication related features. I noticed another
> > > possible problem if the subscriber subscribes multiple publications which
> > > publish parent and child table.
> > >
> > > For example:
> > >
> > > ----pub
> > > create table t (a int, b int, c int) partition by range (a);
> > > create table t_1 partition of t for values from (1) to (10);
> > >
> > > create publication pub1 for table t
> > > with (PUBLISH_VIA_PARTITION_ROOT);
> > > create publication pub2 for table t_1
> > > with (PUBLISH_VIA_PARTITION_ROOT);
> > >
> > > ----sub
> > > ---- prepare table t and t_1
> > > CREATE SUBSCRIPTION sub CONNECTION 'port=10000 dbname=postgres'
> > > PUBLICATION pub1, pub2;
> > >
> > > select * from pg_subscription_rel ;
> > > srsubid | srrelid | srsubstate | srsublsn
> > > ---------+---------+------------+-----------
> > > 16391 | 16385(t) | r | 0/150D100
> > > 16391 | 16388(t_1) | r | 0/150D138
> > >
> > > If subscribe two publications one of them publish parent table with
> > > (pubviaroot=true) and another publish child table. Both the parent table and
> > > child table will exist in pg_subscription_rel which also means we will do
> > > initial copy for both tables.
> > >
> > > But after initial copy, we only publish change with the schema of the parent
> > > table(t). It looks a bit inconsistent.
> > >
> > > Based on the document of PUBLISH_VIA_PARTITION_ROOT option. I think
> > the
> > > expected behavior could be we only store the top most parent(table t) in
> > > pg_subscription_rel and do initial copy for it if pubviaroot is on. I haven't
> > > thought about how to fix this and will investigate this later.
> > Hi,
> > I try to fix this bug. Attach the patch.
> >
> > The current HEAD get table list for one publication by invoking function
> > pg_get_publication_tables. If multiple publications are subscribed, then this
> > function is invoked multiple times. So option PUBLISH_VIA_PARTITION_ROOT
> > works
> > independently on every publication, I think it does not work correctly on
> > different publications of the same subscription.
> >
> > So I fix this bug by the following two steps:
> > First step,
> > I get oids of subscribed tables by publication list. Then for tables with the
> > same topmost root table, I filter them base on the option
> > PUBLISH_VIA_PARTITION_ROOT(see new function filter_partitions_oids).
> > After filtering, I get the final oid list.
> > Second step,
> > I get the required informations(nspname and relname) base on the oid list of
> > first step.
>
> Thanks for updating the patch.
> I confirmed that the bug is fixed by this patch.
>
> One suggestion is that can we simplify the code by moving the logic of checking
> the ancestor into the SQL ?. For example, we could filter the outpout of
> pg_publication_tables by adding A WHERE clause which checks whether the table
> is a partition and if its ancestor is also in the output. I think we can also
> filter the needless partition in this approach.
>
I agreed with you and I tried to fix this problem in a simpler way. What we want
is to exclude the partitioned table whose ancestor is also need to be
replicated, so how about implementing that by using the following SQL when
getting the table list from publisher?
SELECT DISTINCT ns.nspname, c.relname
FROM pg_catalog.pg_publication_tables t
JOIN pg_catalog.pg_namespace ns ON ns.nspname = t.schemaname
JOIN pg_catalog.pg_class c ON c.relname = t.tablename AND c.relnamespace = ns.oid
WHERE t.pubname IN ('p0','p2')
AND (c.relispartition IS FALSE OR NOT EXISTS (SELECT 1 FROM pg_partition_ancestors(c.oid)
WHERE relid IN ( SELECT DISTINCT (schemaname||'.'||tablename)::regclass::oid
FROM pg_catalog.pg_publication_tables t
WHERE t.pubname IN ('p0','p2') ) AND relid != c.oid));
Please find the attached patch which used this approach, I also merged the test
in Wang's patch into it.
Regards,
Shi yu
Attachments:
[application/octet-stream] v2-0001-Fix-data-replicated-twice-when-specifying-PUBLISH.patch (5.6K, ../OSZPR01MB63109BE4F5521C542A75AC9BFDF29@OSZPR01MB6310.jpnprd01.prod.outlook.com/2-v2-0001-Fix-data-replicated-twice-when-specifying-PUBLISH.patch)
download | inline diff:
From 8f302f5dd254853f7d0fd855ac8328a79abad38a Mon Sep 17 00:00:00 2001
From: "shiy.fnst" <[email protected]>
Date: Tue, 19 Apr 2022 11:33:33 +0800
Subject: [PATCH v2] Fix data replicated twice when specifying
PUBLISH_VIA_PARTITION_ROOT option.
If there are two publications that publish the parent table and the child table
separately, and both specify the option PUBLISH_VIA_PARTITION_ROOT, when
subscribing to both publications using one subscription, the data is replicated
twice in inital copy. What we expect is to be copied only once.
To fix this, we exclude the partitioned table whose ancestor belongs to
specified publications when getting the table list from publisher.
---
src/backend/commands/subscriptioncmds.c | 26 +++++++---
src/test/subscription/t/100_bugs.pl | 69 +++++++++++++++++++++++++
2 files changed, 89 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index b94236f74d..8f63dfe047 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -1759,20 +1759,34 @@ static List *
fetch_table_list(WalReceiverConn *wrconn, List *publications)
{
WalRcvExecResult *res;
- StringInfoData cmd;
+ StringInfoData cmd,
+ pub_names;
TupleTableSlot *slot;
Oid tableRow[2] = {TEXTOID, TEXTOID};
List *tablelist = NIL;
+ initStringInfo(&pub_names);
+ get_publications_str(publications, &pub_names, true);
+
+ /*
+ * Get the list of tables from publisher, the partitioned table whose
+ * ancestor is also in this list should be ignored, otherwise the initial
+ * date in the partitioned table would be replicated twice.
+ */
initStringInfo(&cmd);
- appendStringInfoString(&cmd, "SELECT DISTINCT t.schemaname, t.tablename\n"
- " FROM pg_catalog.pg_publication_tables t\n"
- " WHERE t.pubname IN (");
- get_publications_str(publications, &cmd, true);
- appendStringInfoChar(&cmd, ')');
+ appendStringInfo(&cmd, "SELECT DISTINCT ns.nspname, c.relname\n"
+ " FROM pg_catalog.pg_publication_tables t\n"
+ " JOIN pg_catalog.pg_namespace ns ON ns.nspname = t.schemaname\n"
+ " JOIN pg_catalog.pg_class c ON c.relname = t.tablename AND c.relnamespace = ns.oid\n"
+ " WHERE t.pubname IN (%s)\n"
+ " AND (c.relispartition IS FALSE OR NOT EXISTS (SELECT 1 FROM pg_partition_ancestors(c.oid)\n"
+ " WHERE relid IN ( SELECT DISTINCT (schemaname || '.' || tablename)::regclass::oid\n"
+ " FROM pg_catalog.pg_publication_tables t\n"
+ " WHERE t.pubname IN (%s) ) AND relid != c.oid))", pub_names.data, pub_names.data);
res = walrcv_exec(wrconn, cmd.data, 2, tableRow);
pfree(cmd.data);
+ pfree(pub_names.data);
if (res->status != WALRCV_OK_TUPLES)
ereport(ERROR,
diff --git a/src/test/subscription/t/100_bugs.pl b/src/test/subscription/t/100_bugs.pl
index 11ba473715..5b11e06f02 100644
--- a/src/test/subscription/t/100_bugs.pl
+++ b/src/test/subscription/t/100_bugs.pl
@@ -307,4 +307,73 @@ is( $node_subscriber->safe_psql(
$node_publisher->stop('fast');
$node_subscriber->stop('fast');
+# https://www.postgresql.org/message-id/OS0PR01MB5716DC2982CC735FDE388804940B9%40OS0PR01MB5716.jpnprd01.prod.outlook.com
+
+# The bug was that if there are two publications that publish the parent table
+# and the child table separately, and both specify the option
+# PUBLISH_VIA_PARTITION_ROOT, when subscribing to both publications with one
+# subscription, the data is replicated twice. What we expect is to be copied
+# only once.
+$node_publisher = PostgreSQL::Test::Cluster->new('publisher4');
+$node_publisher->init(allows_streaming => 'logical');
+$node_publisher->start;
+
+$node_subscriber = PostgreSQL::Test::Cluster->new('subscriber4');
+$node_subscriber->init(allows_streaming => 'logical');
+$node_subscriber->start;
+
+# create tables pub and sub
+$node_publisher->safe_psql('postgres',
+ "CREATE TABLE viaroot_partition(a int) PARTITION BY RANGE(a)"
+);
+$node_publisher->safe_psql('postgres',
+ "CREATE TABLE viaroot_partitioned(LIKE viaroot_partition)"
+);
+$node_publisher->safe_psql('postgres',
+ "ALTER TABLE viaroot_partition ATTACH PARTITION viaroot_partitioned DEFAULT"
+);
+$node_subscriber->safe_psql('postgres',
+ "CREATE TABLE viaroot_partition(a int) PARTITION BY RANGE(a)"
+);
+$node_subscriber->safe_psql('postgres',
+ "CREATE TABLE viaroot_partitioned(LIKE viaroot_partition)"
+);
+$node_subscriber->safe_psql('postgres',
+ "ALTER TABLE viaroot_partition ATTACH PARTITION viaroot_partitioned DEFAULT"
+);
+
+# insert some initial data
+$node_publisher->safe_psql('postgres',
+ "INSERT INTO viaroot_partition (a) VALUES (1), (2), (3)"
+);
+
+# create pub/sub
+$publisher_connstr = $node_publisher->connstr . ' dbname=postgres';
+$node_publisher->safe_psql('postgres',
+ "CREATE PUBLICATION pub_viaroot1 FOR TABLE viaroot_partition WITH (PUBLISH_VIA_PARTITION_ROOT=TRUE)"
+);
+$node_publisher->safe_psql('postgres',
+ "CREATE PUBLICATION pub_viaroot2 FOR TABLE viaroot_partitioned WITH (PUBLISH_VIA_PARTITION_ROOT=TRUE)"
+);
+$node_subscriber->safe_psql('postgres',
+ "CREATE SUBSCRIPTION sub_viaroot CONNECTION '$publisher_connstr' PUBLICATION pub_viaroot1, pub_viaroot2"
+);
+
+$node_publisher->wait_for_catchup('sub_viaroot');
+
+# Also wait for initial table sync to finish
+$node_subscriber->poll_query_until('postgres', $synced_query)
+ or die "Timed out while waiting for subscriber to synchronize data";
+
+# Check expected replicated result.
+is( $node_subscriber->safe_psql(
+ 'postgres', "SELECT * FROM viaroot_partition"),
+ qq(1
+2
+3),
+ "check initial data copy from table viaroot_partition");
+
+$node_publisher->stop('fast');
+$node_subscriber->stop('fast');
+
done_testing();
--
2.18.4
view thread (35+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: RE: Data is copied twice when specifying both child and parent table in publication
In-Reply-To: <OSZPR01MB63109BE4F5521C542A75AC9BFDF29@OSZPR01MB6310.jpnprd01.prod.outlook.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox