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

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?

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