Re: Replica Identity check of partition table on subscriber

On Mon, Jun 13, 2022 at 2:20 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
>
> On Sat, Jun 11, 2022 at 10:36 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > On Fri, Jun 10, 2022 at 2:26 PM Amit Langote <amitlangote09(at)gmail(dot)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.