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

On Wed, 13 Jan 2021 at 17:23, Dilip Kumar wrote:
> On Tue, Jan 12, 2021 at 4:47 PM Li Japin <japinli(at)hotmail(dot)com> wrote:
>>
>>
>> On Jan 12, 2021, at 5:47 PM, japin <japinli(at)hotmail(dot)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(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>>
>>
>> On Tue, Jan 12, 2021 at 9:05 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>>
>>
>> On Mon, Jan 11, 2021 at 6:51 PM Bharath Rupireddy
>> <bharath(dot)rupireddyforpostgres(at)gmail(dot)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.

--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.