From: Tomas Vondra <tomas@vondra.me>
To: Tom Lane <tgl@sss.pgh.pa.us>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Daniel Gustafsson <dgustafsson@postgresql.org>
Cc: Andres Freund <andres@anarazel.de>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Cc: Rahila Syed <rahilasyed90@gmail.com>
Subject: Re: pgsql: Add function to get memory context stats for processes
Date: Sat, 26 Apr 2025 23:07:46 +0200
Message-ID: <91f216ec-e367-42a5-8586-f554c4c28574@vondra.me> (raw)
In-Reply-To: <1428549.1745432074@sss.pgh.pa.us>
References: <E1u24yn-003Ggy-01@gemulon.postgresql.org>
<CA+Tgmoaey-kOP1k5FaUnQFd1fR0majVebWcL8ogfLbG_nt-Ytg@mail.gmail.com>
<zppwd3scfznxhptdwm765l73wttegi5mibaxyxdwihlmzg3khs@fdhzkq3uxyct>
<CA+Tgmob7yyONbdv-DRRrWxQPL9AsXFxzs3OH1F6vxSc14prLFA@mail.gmail.com>
<rkbjmh4fipn5wzf4kqnge3p2szvhd7kgxgccoho5dtntojmkvt@uppvdxpywiba>
<CA+TgmoZgOwiF1auY+wxUBC+7GcjKqXFBf7D97sF4WL7HER4LgA@mail.gmail.com>
<8EBFBFBC-DAC2-4521-AE16-069F7FD8EFBC@postgresql.org>
<CA+Tgmob1HFe-EbDmLs-xoQQfWAj8aB_cPToW-OOfhs4mFLfizg@mail.gmail.com>
<1428549.1745432074@sss.pgh.pa.us>
On 4/23/25 20:14, Tom Lane wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> My primary concern about the patch is that
>> ProcessGetMemoryContextInterrupt() can be called from any
>> CHECK_FOR_INTERRUPTS() and calls lots of DSA functions, including
>> dsa_create() and, via PublishMemoryContext(), dsa_allocate0(). I'm
>> shocked to hear that you and Andres think that's safe to do at any
>> current or future CHECK_FOR_INTERRUPTS() anywhere in the code; but
>> Andres seems very confident that it's fine, so perhaps I should just
>> stop worrying and be happy that we have the feature.
>
> Just for the record, it sounds quite unsafe to me too. I could
> credit it being all right to examine the process' MemoryContext data
> structures, but calling dsa_create() from CFI seems really insane.
> Way too many moving parts there.
>
Maybe I'm oblivious to some very obvious issues, but why is this so
different from everything else that is already called from
ProcessInterrupts()? Perhaps dsa_create() is more complex compared to
the other stuff (haven't checked), but why would it be unsafe?
The one risk I can think of is a risk of recursion - if any of the
functions called from ProcessGetMemoryContextInterrupt() does CFI, could
that be a problem if there's another pending signal. I see some other
handlers (e.g. ProcessParallelApplyMessages) handle this by explicitly
holding interrupts. Should ProcessGetMemoryContextInterrupt() do the
same thing?
In any case, if DSA happens to not be the right way to transfer this,
what should we use instead? The only thing I can think of is some sort
of pre-allocated chunk of shared memory.
regards
--
Tomas Vondra
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-committers@postgresql.org
Cc: tomas@vondra.me, tgl@sss.pgh.pa.us, robertmhaas@gmail.com, dgustafsson@postgresql.org, andres@anarazel.de, pgsql-hackers@lists.postgresql.org, rahilasyed90@gmail.com
Subject: Re: pgsql: Add function to get memory context stats for processes
In-Reply-To: <91f216ec-e367-42a5-8586-f554c4c28574@vondra.me>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox