Re: Logical Replication - behavior of ALTER PUBLICATION .. DROP TABLE and ALTER SUBSCRIPTION .. REFRESH PUBLICATION

On Fri, Jan 22, 2021 at 2:00 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> Thanks for the patch. Few comments:
> +
> +# Test replication with multiple publications for subscription
> +
>
> While checking the existing tests in 001_rep_changes.pl, I came across
> the below test which has multiple publications, won't that suffice the
> need?
>
> "CREATE SUBSCRIPTION tap_sub CONNECTION '$publisher_connstr'
> PUBLICATION tap_pub, tap_pub_ins_only"

Yeah I saw that test case earlier, but we couldn't catch the error with it.

Our earlier fix was to set pubactions to false whenever publish is
false in get_rel_sync_entry

foreach(lc, data->publications)
{
//publish is set to false when the given relation's publication is
not in the list of publications provided by subscription i.e.
data->publications
if(!publish)
{
//set all entry->pubactions to false
}
}

The existing use case:
The publication tap_pub has tables tab_rep, tab_full, tab_full2,
tab_mixed, tab_include, tab_nothing, tab_full_pk
The publication tap_pub_ins_only has table tab_ins
CREATE SUBSCRIPTION tap_sub CONNECTION '$publisher_connstr'
PUBLICATION tap_pub, tap_pub_ins_only;
INSERT INTO tab_ins SELECT generate_series(1,50);
And the first insertion after the subscription is created happens into
the table tab_ins which is with tap_pub_ins_only i.e. 2nd publication
in the list of publications specified in the create subscription
statement.

The existing use case analysis:
given relation tab_ins's publication is tap_pub_ins_only
data->publications list contains - tap_pub, tap_pub_ins_only
loop 1: data->publications list 1st element is tap_pub and relation
tab_ins's publication is tap_pub_ins_only, so publish is false,
pubations are set to false.
loop 2: data->publications list 2nd element is tap_pub_ins_only and
relation tab_ins's publication is also tap_pub_ins_only, publish is
true, pubations are set to true,
loop is exited with entry->pubactions set to true, hence the
replication happens for the table.

japin's use case [1]:
But, in the japin use case[1] where we saw regression with the initial
version of the fix, the first insertion happens into the table t1
which is with publication mypub1 i.e. 1st in the list of publications
associated with the subscription.

japin's use case [1] analysis:
given relation t1's publication is mypub1
data->publications list contains - mypub1, mypub2
loop 1: data->publications list 1st element is mypub1 and relation
t1's publication is also mypub1, so publish is true, pubations.insert
is set to true, other actions are false because the publication mypub1
is created WITH (publish='insert');, so the loop can't get exited from
below break condition.
if (entry->pubactions.pubinsert && entry->pubactions.pubupdate &&
entry->pubactions.pubdelete && entry->pubactions.pubtruncate)
break;
loop 2: data->publications list 2nd element is mypub2 and relation
t1's publication is mypub1, so publish is false, all pubations are set
to false.
loop is exited with all entry->pubactions set to false, hence the
replication doesn't happen for the table t1.

From the above analysis, it turns out that the existing use case
didn't catch the regression, that is why we had to add a new test
case.

[1]
Step (1)
-- On publisher
CREATE TABLE t1 (a int);
CREATE TABLE t2 (a int);
CREATE PUBLICATION mypub1 FOR TABLE t1 WITH (publish='insert');
CREATE PUBLICATION mypub2 FOR TABLE t2;

Step (2)
-- On subscriber
CREATE TABLE t1 (a int);
CREATE TABLE t2 (a int);
CREATE SUBSCRIPTION mysub1 CONNECTION 'host=localhost port=5432
dbname=postgres' PUBLICATION mypub1, mypub2;

Step (3)
-- On publisher
INSERT INTO t1 VALUES (1);

Step (4)
-- On subscriber
SELECT * FROM t1;

> BTW, have we tried to check if this problem exists in back-branches?
> It seems to me the problem has been recently introduced by commit
> 69bd60672a. I am telling this by looking at code and have yet not
> performed any testing so I could be wrong.

Yes you are right. Looks like the above commit is causing the issue. I
reverted that commit and did not see the drop publication bug, see use
case [1].

[1]
-- On publisher
CREATE TABLE t1 (a int);
INSERT INTO t1 VALUES (1);
CREATE PUBLICATION mypub1 FOR TABLE t1;

-- On subscriber
CREATE TABLE t1 (a int);
CREATE SUBSCRIPTION mysub1 CONNECTION 'host=localhost port=5432
dbname=postgres' PUBLICATION mypub1;

-- On publisher
INSERT INTO t1 VALUES (2);

-- On subscriber
SELECT * FROM t1;

-- On publisher
ALTER PUBLICATION mypub1 DROP TABLE t1;
INSERT INTO t1 VALUES (3);

-- On subscriber
SELECT * FROM t1; --> we should not see value 3, if the alter
publication ... drop table worked correctly.

-- On publisher
INSERT INTO t1 SELECT * FROM generate_series(4, 88);

-- On subscriber
SELECT * FROM t1;

With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com