Re: lsyscache: free IndexAmRoutine objects returned by GetIndexAmRoutineByAmId()

Matthias van de Meent <boekewurm+postgres@gmail.com>

From: Matthias van de Meent <boekewurm+postgres@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Álvaro Herrera <alvherre@kurilemu.de>, Chao Li <li.evan.chao@gmail.com>, Postgres hackers <pgsql-hackers@lists.postgresql.org>
Date: 2025-12-30T18:03:30Z
Lists: pgsql-hackers
On Tue, 30 Dec 2025 at 16:25, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Matthias van de Meent <boekewurm+postgres@gmail.com> writes:
> > On Tue, 30 Dec 2025 at 15:15, Álvaro Herrera <alvherre@kurilemu.de> wrote:
> >> One thing we can perhaps do is (in assert-enabled builds) to detect
> >> whether memory usage for that context has increased during
> >> InitIndexAmRoutine and raise a warning if so.  Then extension authors
> >> would realize this and have a chance to fix it promptly.
>
> > Hmm, wouldn't we be able to detect changes in
> > MemoryContextMemConsumed(ctx, counters) with one before and one after
> > GetIndexAmRoutine(), such as included below?
>
> I don't think we can do this, because there are effects that the
> amhandler doesn't have control over.  In particular, if we have to
> load its pg_proc row into syscache during fmgr_info, I don't think
> that is positively guaranteed not to leak anything.  (This isn't
> a factor for built-in AMs, which will take the fast path in
> fmgr_info, but it will be an issue for extensions.)
>
> I am not terribly concerned by one-time leaks of that sort, so
> I don't really feel an urge to try to complain about them.

If it's difficult to filter out one-time leaks into the context caused
by e.g. fmgr infra, then -indeed- it's probably not worth the effort.

In which case, v3 LGTM.

Kind regards,

Matthias van de Meent
Databricks (https://www.databricks.com)