Re: Fix for segfault in logical replication on master

Поиск
Список
Период
Сортировка
От Amit Kapila
Тема Re: Fix for segfault in logical replication on master
Дата
Msg-id CAA4eK1+-d7-8OiEoRPTL+obuT3mU8Wg0dDqwNTMMpVVgp7d+FA@mail.gmail.com
обсуждение исходный текст
Ответ на Re: Fix for segfault in logical replication on master  (Japin Li <japinli@hotmail.com>)
Ответы Re: Fix for segfault in logical replication on master  (Japin Li <japinli@hotmail.com>)
Список pgsql-hackers
On Mon, Jun 21, 2021 at 4:09 PM Japin Li <japinli@hotmail.com> wrote:
>
> On Mon, 21 Jun 2021 at 17:54, Amit Kapila <amit.kapila16@gmail.com> wrote:
> > On Mon, Jun 21, 2021 at 2:06 PM Japin Li <japinli@hotmail.com> wrote:
> >>
> >> On Mon, 21 Jun 2021 at 16:22, Amit Kapila <amit.kapila16@gmail.com> wrote:
> >> > On Mon, Jun 21, 2021 at 1:30 PM Japin Li <japinli@hotmail.com> wrote:
> >> >>
> >> >> On Sat, 19 Jun 2021 at 17:18, Amit Kapila <amit.kapila16@gmail.com> wrote:
> >> >> > On Fri, Jun 18, 2021 at 9:18 AM Amit Kapila <amit.kapila16@gmail.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.



В списке pgsql-hackers по дате отправления:

Предыдущее
От: Amit Kapila
Дата:
Сообщение: Re: Optionally automatically disable logical replication subscriptions on error
Следующее
От: Amit Kapila
Дата:
Сообщение: Re: Doc patch for Logical Replication Message Formats (PG14)