On Wed, Jan 13, 2021 at 3:06 PM japin <japi...@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 <japi...@hotmail.com> wrote: > >> > >> > >> On Jan 12, 2021, at 5:47 PM, japin <japi...@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.rupireddyforpostg...@gmail.com> wrote: > >> > >> > >> On Tue, Jan 12, 2021 at 9:05 AM Amit Kapila <amit.kapil...@gmail.com> > >> wrote: > >> > >> > >> On Mon, Jan 11, 2021 at 6:51 PM Bharath Rupireddy > >> <bharath.rupireddyforpostg...@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