Re: Logical Replication - behavior of ALTER PUBLICATION .. DROP TABLE and ALTER SUBSCRIPTION .. REFRESH PUBLICATION - Mailing list pgsql-hackers

From Bharath Rupireddy
Subject Re: Logical Replication - behavior of ALTER PUBLICATION .. DROP TABLE and ALTER SUBSCRIPTION .. REFRESH PUBLICATION
Date
Msg-id CALj2ACU-evX44Upr9LwK44OJqhNyd_w7Qvi3ZMsScT23NLGX5Q@mail.gmail.com
Whole thread Raw
In response to Re: Logical Replication - behavior of ALTER PUBLICATION .. DROP TABLE and ALTER SUBSCRIPTION .. REFRESH PUBLICATION  (Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>)
List pgsql-hackers
On Fri, Jan 22, 2021 at 2:59 PM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
> > 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].

I think, to be precise the issue is because of not setting pubactions
to false when if (!entry->replicate_valid) is true by the commit
69bd60672. In fact, it removed the correct check. Since found(denotes
whether the cache entry exists or not) will be true even after the
alter publication ... drop table, the cache entry->replicate_valid is
the right check which will be set to false in
rel_sync_cache_publication_cb. But this commit removed it.

And also the reason for not catching this bug during the testing of
69bd60672 commit's patch is that we don't have a test case for alter
publication ... drop table behaviour.

-       if (!found || !entry->replicate_valid)
+       if (!found)
+       {
+               /* immediately make a new entry valid enough to
satisfy callbacks */
+               entry->schema_sent = false;
+               entry->streamed_txns = NIL;
+               entry->replicate_valid = false;
+               entry->pubactions.pubinsert = entry->pubactions.pubupdate =
+                       entry->pubactions.pubdelete =
entry->pubactions.pubtruncate = false;
+               entry->publish_as_relid = InvalidOid;
+       }
+
+       /* Validate the entry */
+       if (!entry->replicate_valid)
        {
                List       *pubids = GetRelationPublications(relid);
                ListCell   *lc;
@@ -977,9 +987,6 @@ get_rel_sync_entry(PGOutputData *data, Oid relid)
                 * relcache considers all publications given relation
is in, but here
                 * we only need to consider ones that the subscriber requested.
                 */
-               entry->pubactions.pubinsert = entry->pubactions.pubupdate =
-                       entry->pubactions.pubdelete =
entry->pubactions.pubtruncate = false;
-

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



pgsql-hackers by date:

Previous
From: Bharath Rupireddy
Date:
Subject: Re: Logical Replication - behavior of ALTER PUBLICATION .. DROP TABLE and ALTER SUBSCRIPTION .. REFRESH PUBLICATION
Next
From: Peter Eisentraut
Date:
Subject: Re: [PATCH] Identify LWLocks in tracepoints