From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>, torikoshia <torikoshia(at)oss(dot)nttdata(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>, Kasahara Tatsuhito <kasahara(dot)tatsuhito(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de> |
Subject: | Re: Creating a function for exposing memory usage of backend process |
Date: | 2020-08-19 15:01:37 |
Message-ID: | 1414992.1597849297@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hadn't been paying attention to this thread up till now, but ...
Michael Paquier <michael(at)paquier(dot)xyz> writes:
> By the way, I was looking at the code that has been committed, and I
> think that it is awkward to have a SQL function in mcxt.c, which is a
> rather low-level interface. I think that this new code should be
> moved to its own file, one suggestion for a location I have being
> src/backend/utils/adt/mcxtfuncs.c.
I agree with that, but I think this patch has a bigger problem:
why bother at all? It seems like a waste of code space and future
maintenance effort, because there is no use-case. In the situations
where you need to know where the memory went, you are almost never
in a position to leisurely execute a query and send the results over
to your client. This certainly would be useless to figure out why
an already-running query is eating space, for instance.
The only situation I could imagine where this would have any use is
where there is long-term (cross-query) bloat in, say, CacheMemoryContext
--- but it's not even very helpful for that, since you can't examine
anything finer-grain than a memory context. Plus you need to be
running an interactive session, or else be willing to hack up your
application to try to get it to inspect the view (and log the
results somewhere) at useful times.
On top of all that, the functionality has Heisenberg problems,
because simply using it changes what you are trying to observe,
in complex and undocumented ways (not that the documentation
would be of any use to non-experts anyway).
My own thoughts about improving the debugging situation would've been
to create a way to send a signal to a session to make it dump its
current memory map to the postmaster log (not the client, since the
client is unlikely to be prepared to receive anything extraneous).
But this is nothing like that.
Given the lack of clear use-case, and the possibility (admittedly
not strong) that this is still somehow a security hazard, I think
we should revert it. If it stays, I'd like to see restrictions
on who can read the view.
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | ahsan hadi | 2020-08-19 15:12:27 | Re: Performing partition pruning using row value |
Previous Message | Julien Rouhaud | 2020-08-19 14:19:30 | Re: Feature improvement: can we add queryId for pg_catalog.pg_stat_activity view? |