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

From: Dilip Kumar <dilipbalaut(at)gmail(dot)com>
To: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
Cc: Li Japin <japinli(at)hotmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Logical Replication - behavior of ALTER PUBLICATION .. DROP TABLE and ALTER SUBSCRIPTION .. REFRESH PUBLICATION
Date: 2021-01-13 11:45:30
Message-ID: CAFiTN-tKWJouqV0aOSfsrvFfzfdmA5cvFT-CT2xUsVKw1ALPHA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Jan 13, 2021 at 3:08 PM Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>
> On Wed, Jan 13, 2021 at 2:53 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> > > 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?
>
> IIUC, it's not possible to remove the cache entry inside
> rel_sync_cache_publication_cb, because we don't receive the relid of
> the alter publication .. dropped relation in the invalidation
> callback. See the below comment,
>
> /*
> * There is no way to find which entry in our cache the hash belongs to so
> * mark the whole cache as invalid.
> */
> hash_seq_init(&status, RelationSyncCache);
> while ((entry = (RelationSyncEntry *) hash_seq_search(&status)) != NULL)
> entry->replicate_valid = false;

So basically your point is that all the RelationSyncEntry are getting
invalidated right? And if so then in get_rel_sync_entry we will just
check the updated publication no?
I am referring to the below code. So ideally this should be already
working if the relcaches are getting invalidated?

get_rel_sync_entry
{
entry = (RelationSyncEntry *) hash_search(RelationSyncCache,
(void *) &relid,
HASH_ENTER, &found);
Assert(entry != NULL);

.....

/* Validate the entry */
if (!entry->replicate_valid)
{
List *pubids = GetRelationPublications(relid);
ListCell *lc;
Oid publish_as_relid = relid;
... here we will rechek the pulication
}

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Heikki Linnakangas 2021-01-13 11:53:39 Re: Yet another fast GiST build
Previous Message Amit Kapila 2021-01-13 11:37:50 Re: Single transaction in the tablesync worker?