Re: Fix for segfault in logical replication on master

On Mon, Jun 21, 2021 at 4:09 PM Japin Li <japinli(at)hotmail(dot)com> wrote:
>
> On Mon, 21 Jun 2021 at 17:54, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > On Mon, Jun 21, 2021 at 2:06 PM Japin Li <japinli(at)hotmail(dot)com> wrote:
> >>
> >> On Mon, 21 Jun 2021 at 16:22, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >> > On Mon, Jun 21, 2021 at 1:30 PM Japin Li <japinli(at)hotmail(dot)com> wrote:
> >> >>
> >> >> On Sat, 19 Jun 2021 at 17:18, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >> >> > On Fri, Jun 18, 2021 at 9:18 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >> >>
> >> >> Or we can free the memory owned by indexoidlist after check whether it is NIL,
> >> >> because we do not use it in the later.
> >> >>
> >> >
> >> > Valid point. But I am thinking do we really need to fetch and check
> >> > indexoidlist here?
> >>
> >> IMO, we shold not fetch and check the indexoidlist here, since we do not
> >> use it. However, we should use RelationGetIndexList() to update the
> >> reladion->rd_replidindex, so we should fetch the indexoidlist, maybe we
> >> can use the following code:
> >>
> >> indexoidlist = RelationGetIndexList(relation);
> >> list_free(indexoidlist);
> >>
> >> Or does there any function that only update the relation->rd_replidindex
> >> or related fields, but do not fetch the indexoidlist?
> >>
> >
> > How about RelationGetReplicaIndex? It fetches the indexlist only when
> > required and frees it immediately. But otherwise, currently, there
> > shouldn't be any memory leak because we allocate this in "logical
> > replication output context" which is reset after processing each
> > change message, see pgoutput_change.
>
> Thanks for your explanation. It might not be a memory leak, however it's
> a little confuse when we free the memory of the indexoidlist in one place,
> but not free it in another place.
>
> I attached a patch to fix this. Any thoughts?
>

Your patch appears to be on the lines we discussed but I would prefer
to get it done after Beta2 as this is just a minor code improvement.
Can you please send the change as a patch file instead of copy-pasting
the diff at the end of the email?

--
With Regards,
Amit Kapila.