From: torikoshia <torikoshia@oss.nttdata.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
To: Fujii Masao <masao.fujii@oss.nttdata.com>
Cc: Georgios Kokolatos <gkokolatos@protonmail.com>
Cc: pgsql-hackers@postgresql.org
Subject: Re: Get memory contexts of an arbitrary backend process
Date: Fri, 04 Dec 2020 19:16:12 +0900
Message-ID: <83ff0aff04fffc95cbd1fc1637a0fccc@oss.nttdata.com> (raw)
In-Reply-To: <1685539.1606959410@sss.pgh.pa.us>
References: <0271f440ac77f2a4180e0e56ebd944d1@oss.nttdata.com>
<270ec80dbbebfb5c6f2b9fdd7a7858b5@oss.nttdata.com>
<160502083340.7362.4386116585349115146.pgcf@coridan.postgresql.org>
<064d510f2d33edc37dfd6b8f3b6bd30e@oss.nttdata.com>
<d42d3186-d647-9af4-7c1c-d6ffca8bff40@oss.nttdata.com>
<1685539.1606959410@sss.pgh.pa.us>
On 2020-12-03 10:36, Tom Lane wrote:
> Fujii Masao <masao.fujii@oss.nttdata.com> writes:
>> I'm starting to study how this feature behaves. At first, when I
>> executed
>> the following query, the function never returned. ISTM that since
>> the autovacuum launcher cannot respond to the request of memory
>> contexts dump, the function keeps waiting infinity. Is this a bug?
>> Probably we should exclude non-backend proceses from the target
>> processes to dump? Sorry if this was already discussed.
>
>> SELECT pg_get_backend_memory_contexts(pid) FROM pg_stat_activity;
Thanks for trying it!
It was not discussed explicitly, and I was going to do it later
as commented.
> + /* TODO: Check also whether backend or not. */
>
> FWIW, I think this patch is fundamentally unsafe. It's got a
> lot of the same problems that I complained about w.r.t. the
> nearby proposal to allow cross-backend stack trace dumping.
> It does avoid the trap of thinking that it can do work in
> a signal handler, but instead it supposes that it can do
> work involving very high-level objects such as shared hash tables
> in anyplace that might execute CHECK_FOR_INTERRUPTS. That's
> never going to be safe: the only real expectation the system
> has is that CHECK_FOR_INTERRUPTS is called at places where our
> state is sane enough that a transaction abort can clean up.
> Trying to do things like taking LWLocks is going to lead to
> deadlocks or worse. We need not even get into the hard questions,
> such as what happens when one process or the other exits
> unexpectedly.
Thanks for reviewing!
I may misunderstand something, but the dumper works not at
CHECK_FOR_INTERRUPTS but during the client read, i.e.,
ProcessClientReadInterrupt().
Is it also unsafe?
BTW, since there was a comment that the shared hash table
used too much memory, I'm now rewriting this patch not to use
the shared hash table but a simpler static shared memory struct.
> I also find the idea that this should be the same SQL function
> as pg_get_backend_memory_contexts to be a seriously bad decision.
> That means that it's not possible to GRANT the right to examine
> only your own process's memory --- with this proposal, that means
> granting the right to inspect every other process as well.
>
> Beyond that, the fact that there's no way to restrict the capability
> to just, say, other processes owned by the same user means that
> it's not really safe to GRANT to non-superusers anyway. Even with
> such a restriction added, things are problematic, since for example
> it would be possible to inquire into the workings of a
> security-definer function executing in another process that
> nominally is owned by your user.
I'm going to change the function name and restrict the executor to
superusers. Is it enough?
Regards,
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-hackers@postgresql.org
Cc: torikoshia@oss.nttdata.com, tgl@sss.pgh.pa.us, masao.fujii@oss.nttdata.com, gkokolatos@protonmail.com
Subject: Re: Get memory contexts of an arbitrary backend process
In-Reply-To: <83ff0aff04fffc95cbd1fc1637a0fccc@oss.nttdata.com>
* 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