RE: Table refer leak in logical replication

From: "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>
To: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Cc: "tanghy(dot)fnst(at)fujitsu(dot)com" <tanghy(dot)fnst(at)fujitsu(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "shiy(dot)fnst(at)fujitsu(dot)com" <shiy(dot)fnst(at)fujitsu(dot)com>
Subject: RE: Table refer leak in logical replication
Date: 2021-04-06 04:01:13
Message-ID: OS0PR01MB57160AE18728204E6B2E1FAC94769@OS0PR01MB5716.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> > > insert into test values (6);
> > >
> > > It seems an issue about reference leak. Anyone can fix this?
> >
> > It seems ExecGetTriggerResultRel will reopen the target table because it
> cannot find an existing one.
> > Storing the opened table in estate->es_opened_result_relations seems
> solves the problem.
>
> It seems like commit 1375422c is related to this bug. The commit introduced a
> new function ExecInitResultRelation() that sets both
> estate->es_result_relations and estate->es_opened_result_relations. I
> think it's better to use ExecInitResultRelation() rather than directly setting
> estate->es_opened_result_relations. It might be better to do that in
> create_estate_for_relation() though. Please find an attached patch.
>
> Since this issue happens on only HEAD and it seems an oversight of commit
> 1375422c, I don't think regression tests for this are essential.

It seems we can not only use ExecInitResultRelation.
In function ExecInitResultRelation, it will use ExecGetRangeTableRelation which
will also open the target table and store the rel in "Estate->es_relations".
We should call ExecCloseRangeTableRelations at the end of apply_handle_xxx to
close the rel in "Estate->es_relations".

Attaching the patch with this change.

Best regards,
houzj

Attachment Content-Type Size
fix_relcache_leak_in_lrworker.diff application/octet-stream 2.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jaime Casanova 2021-04-06 04:02:54 document that brin's autosummarize parameter is off by default
Previous Message David Rowley 2021-04-06 03:58:08 Re: Binary search in ScalarArrayOpExpr for OR'd constant arrays