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

On Wed, Jan 13, 2021 at 3:06 PM japin <japinli@hotmail.com> wrote:
>
>
> On Wed, 13 Jan 2021 at 17:23, Dilip Kumar wrote:
> > On Tue, Jan 12, 2021 at 4:47 PM Li Japin <japinli@hotmail.com> wrote:
> >>
> >>
> >> On Jan 12, 2021, at 5:47 PM, japin <japinli@hotmail.com> wrote:
> >>
> >>
> >> On Tue, 12 Jan 2021 at 14:38, Amit Kapila wrote:
> >>
> >> On Tue, Jan 12, 2021 at 11:39 AM Bharath Rupireddy
> >> <bharath.rupireddyforpostgres@gmail.com> wrote:
> >>
> >>
> >> On Tue, Jan 12, 2021 at 9:05 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >>
> >>
> >> On Mon, Jan 11, 2021 at 6:51 PM Bharath Rupireddy
> >> <bharath.rupireddyforpostgres@gmail.com> wrote:
> >>
> >>
> >> Hi,
> >>
> >> While providing thoughts on the design in [1], I found a strange
> >> behaviour with the $subject. The use case is shown below as a sequence
> >> of steps that need to be run on publisher and subscriber to arrive at
> >> the strange behaviour.  In step 5, the table is dropped from the
> >> publication and in step 6, the refresh publication is run on the
> >> subscriber, from here onwards, the expectation is that no further
> >> inserts into the publisher table have to be replicated on to the
> >> subscriber, but the opposite happens i.e. the inserts are still
> >> replicated to the subscriber. ISTM as a bug. Let me know if I'm
> >> missing anything.
> >>
> >>
> >> Did you try to investigate what's going on? Can you please check what
> >> is the behavior if, after step-5, you restart the subscriber and
> >> separately try creating a new subscription (maybe on a different
> >> server) for that publication after step-5 and see if that allows the
> >> relation to be replicated? AFAIU, in AlterSubscription_refresh, we
> >> remove such dropped rels and stop their corresponding apply workers
> >> which should stop the further replication of such relations but that
> >> doesn't seem to be happening in your case.
> >>
> >>
> >> Here's my analysis:
> >> 1) in the publisher, alter publication drop table successfully
> >> removes(PublicationDropTables) the table from the catalogue
> >> pg_publication_rel
> >> 2) in the subscriber, alter subscription refresh publication
> >> successfully removes the table from the catalogue pg_subscription_rel
> >> (AlterSubscription_refresh->RemoveSubscriptionRel)
> >> so far so good
> >>
> >>
> >> Here, it should register the worker to stop on commit, and then on
> >> commit it should call AtEOXact_ApplyLauncher to stop the apply worker.
> >> Once the apply worker is stopped, the corresponding WALSender will
> >> also be stopped. Something here is not happening as per expected
> >> behavior.
> >>
> >> 3) after the insertion into the table in the publisher(remember that
> >> it's dropped from the publication in (1)), the walsender process is
> >> unable detect that the table has been dropped from the publication
> >> i.e. it doesn't look at the pg_publication_rel catalogue or some
> >> other, but it only does is_publishable_relation() check which returns
> >> true in pgoutput_change(). Maybe the walsender should look at the
> >> catalogue pg_publication_rel in is_publishable_relation()?
> >>
> >>
> >> We must be somewhere checking pg_publication_rel before sending the
> >> decoded change because otherwise, we would have sent the changes for
> >> the table which are not even part of this publication. I think you can
> >> try to create a separate table that is not part of the publication
> >> under test and see how the changes for that are filtered.
> >>
> >>
> >> I find that pgoutput_change() use a hash table RelationSyncCache to
> >> cache the publication info for tables.  When we drop tables from the
> >> publication, the RelationSyncCache doesn't updated, so it replicate
> >> records.
> >>
> >>
> >> IIUC the logical replication only replicate the tables in publication, I think
> >> when the tables that aren't in publication should not be replicated.
> >>
> >> Attached the patch that fixes it.  Thought?
> >>
> >
> > Instead of doing this, I would expect that the RelationSyncCache entry
> > should be removed when the relation is dropped from the publication.
> > So if that is done then it will reload the publication and then it
> > will not find that that relation as published and it will ignore the
> > changes.  But the patch doesn't seem to be exactly on that line.  Am I
> > missing something here?
>
> Even if we remove RelationSyncCache entry when the relation is dropped from
> the publication, when the relation is change (insert/update), AFAIK it also
> create an entry in RelationSyncCache, so removing the RelationSyncCache entry
> is unnecessary.
>

That is true but the entry->replicate_valid will be false and that
will be only set to true in get_rel_sync_entry.  So now based on the
new publication the pubaction will be updated ?

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com