Horton

Re: Replica Identity check of partition table on subscriber

Amit Kapila <amit.kapila16@gmail.com>

View thread View on postgr.es
From: Amit Kapila <amit.kapila16@gmail.com>
To: Amit Langote <amitlangote09@gmail.com>
Cc: "shiy.fnst@fujitsu.com" <shiy.fnst@fujitsu.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>
Date: 2022-06-13T09:14:18Z
Lists: pgsql-hackers 
On Mon, Jun 13, 2022 at 2:20 PM Amit Langote <amitlangote09@gmail.com> wrote:
>
> On Sat, Jun 11, 2022 at 10:36 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > On Fri, Jun 10, 2022 at 2:26 PM Amit Langote <amitlangote09@gmail.com> wrote:
> > >
> > > +logicalrep_partmap_invalidate
> > >
> > > I wonder why not call this logicalrep_partmap_update() to go with
> > > logicalrep_relmap_update()?  It seems confusing to have
> > > logicalrep_partmap_invalidate() right next to
> > > logicalrep_partmap_invalidate_cb().
> > >
> >
> > I am thinking about why we need to update the relmap in this new
> > function logicalrep_partmap_invalidate()? I think it may be better to
> > do it in logicalrep_partition_open() when actually required,
> > otherwise, we end up doing a lot of work that may not be of use unless
> > the corresponding partition is accessed. Also, it seems awkward to me
> > that we do the same thing in this new function
> > logicalrep_partmap_invalidate() and then also in
> > logicalrep_partition_open() under different conditions.
>
> Both logicalrep_rel_open() and logicalrel_partition_open() only ever
> touch the local Relation, never the LogicalRepRelation.
>

We do make the copy of remote rel in  logicalrel_partition_open() when
the entry is not found. I feel the same should happen when remote
relation is reset/invalidated by the RELATION message.

>  Updating the
> latter is the responsibility of logicalrep_relmap_update(), which is
> there to support handling of the RELATION message by
> apply_handle_relation().  Given that we make a separate copy of the
> parent's LogicalRepRelMapEntry for each partition to put into the
> corresponding LogicalRepPartMapEntry, those copies must be updated as
> well when a RELATION message targeting the parent's entry arrives.  So
> it seems fine that the patch is making it the
> logicalrep_relmap_update()'s responsibility to update the partition
> copies using the new logicalrep_partition_invalidate/update()
> subroutine.
>

I think we can do that way as well but do you see any benefit in it?
The way I am suggesting will avoid the effort of updating the remote
rel copy till we try to access that particular partition.

-- 
With Regards,
Amit Kapila.

Commits

  1. Fix memory leak due to LogicalRepRelMapEntry.attrmap.

  2. Fix stale values in partition map entries on subscribers.

  3. Fix partition table's REPLICA IDENTITY checking on the subscriber.

  4. Fix data inconsistency between publisher and subscriber.

  5. Fix cache look-up failures while applying changes in logical replication.

  6. Add logical replication support to replicate into partitioned tables