Re: Table refer leak in logical replication

Amit Langote <amitlangote09@gmail.com>

From: Amit Langote <amitlangote09@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Amit Kapila <amit.kapila16@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>, "tanghy.fnst@fujitsu.com" <tanghy.fnst@fujitsu.com>, "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>, "shiy.fnst@fujitsu.com" <shiy.fnst@fujitsu.com>
Date: 2021-04-20T05:48:35Z
Lists: pgsql-hackers
Thanks for taking a look.

On Tue, Apr 20, 2021 at 2:09 PM Michael Paquier <michael@paquier.xyz> wrote:
> On Mon, Apr 19, 2021 at 09:44:05PM +0900, Amit Langote wrote:
> > Okay, how about the attached then?
>
> create_estate_for_relation() returns an extra resultRelInfo that's
> also saved within es_opened_result_relations.  Wouldn't is be simpler
> to take the first element from es_opened_result_relations instead?
> Okay, that's a nit and you are documenting things in a sufficient way,
> but that just seemed duplicated to me.

Manipulating the contents of es_opened_result_relations directly in
worker.c is admittedly a "hack", which I am reluctant to have other
places participating in.  As originally designed, that list is to
speed up ExecCloseResultRelations(), not as a place to access result
relations from.  The result relations targeted over the course of
execution of a query (update/delete) or a (possibly multi-tuple in the
future) replication apply operation will not be guaranteed to be added
to the list in any particular order, so assuming where a result
relation of interest can be found in the list is bound to be unstable.

--
Amit Langote
EDB: http://www.enterprisedb.com



Commits

  1. Add more tests with triggers on partitions for logical replication

  2. Fix relation leak for subscribers firing triggers in logical replication

  3. Create ResultRelInfos later in InitPlan, index them by RT index.