From: torikoshia <torikoshia@oss.nttdata.com>
To: Fujii Masao <masao.fujii@oss.nttdata.com>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Kasahara Tatsuhito <kasahara.tatsuhito@gmail.com>
Cc: Andres Freund <andres@anarazel.de>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Cc: Robert Haas <robertmhaas@gmail.com>
Subject: Re: Creating a function for exposing memory usage of backend process
Date: Mon, 24 Aug 2020 21:56:06 +0900
Message-ID: <ae118b75ede9327c6dc8e9e457639dc9@oss.nttdata.com> (raw)
In-Reply-To: <07da4267-74bf-4a6b-3312-4ab7c1e72f66@oss.nttdata.com>
References: <261eebda3e1909475162b968e5a4ad2b@oss.nttdata.com>
<66ae5b88-208d-96fa-c20a-fb539b597859@oss.nttdata.com>
<20200819135545.GC19121@paquier.xyz>
<1414992.1597849297@sss.pgh.pa.us>
<20200819225343.c7rxteydi35nhrdp@alap3.anarazel.de>
<1554676.1597886946@sss.pgh.pa.us>
<20200820014301.5pr7qjugnvpovu62@alap3.anarazel.de>
<d97ad430-444d-9ec8-21dd-a1584569309b@oss.nttdata.com>
<CAP0=ZVJ5VZsUYLFkc-E5_JHTc1ZdemX28VgQqUXJLTywC=9hfA@mail.gmail.com>
<a8f3c385e9edeba4833c712270d6e6d9@oss.nttdata.com>
<20200822121641.GE2108@paquier.xyz>
<c9515ae497b4a077360a31547782acb8@oss.nttdata.com>
<07da4267-74bf-4a6b-3312-4ab7c1e72f66@oss.nttdata.com>
On 2020-08-24 13:13, Fujii Masao wrote:
> On 2020/08/24 13:01, torikoshia wrote:
>> On 2020-08-22 21:18, Michael Paquier wrote:
>>
>> Thanks for reviewing!
>>
>>> On Fri, Aug 21, 2020 at 11:27:06PM +0900, torikoshia wrote:
>>>> OK. Added a regression test on sysviews.sql.
>>>> (0001-Added-a-regression-test-for-pg_backend_memory_contex.patch)
>>>>
>>>> Fujii-san gave us an example, but I added more simple one
>>>> considering
>>>> the simplicity of other tests on that.
>>>
>>> What you have sent in 0001 looks fine to me. A small test is much
>>> better than nothing.
>
> +1
>
> But as I proposed upthread, what about a bit complicated test as
> follows,
> e.g., to confirm that the internal logic for level works expectedly?
>
> SELECT name, ident, parent, level, total_bytes >= free_bytes FROM
> pg_backend_memory_contexts WHERE level = 0;
OK!
Attached an updated patch.
>
>
>>>
>>>> Added a patch for relocating the codes to mcxtfuncs.c.
>>>> (patches/0001-Rellocated-the-codes-for-pg_backend_memory_contexts-.patch)
>
> Thanks for the patch! Looks good to me.
> Barring any objection, I will commit this patch at first.
>
>
>>>
>>> The same code is moved around line-by-line.
>>>
>>>> Of course, this restriction makes pg_backend_memory_contexts hard to
>>>> use
>>>> when the user of the target session is not granted pg_monitor
>>>> because the
>>>> scope of this view is session local.
>>>>
>>>> In this case, I imagine additional operations something like
>>>> temporarily
>>>> granting pg_monitor to that user.
>>>
>>> Hmm. I am not completely sure either that pg_monitor is the best fit
>>> here, because this view provides information about a bunch of
>>> internal
>>> structures. Something that could easily be done though is to revoke
>>> the access from public, and then users could just set up GRANT
>>> permissions post-initdb, with pg_monitor as one possible choice.
>>> This
>>> is the safest path by default, and this stuff is of a caliber similar
>>> to pg_shmem_allocations in terms of internal contents.
>>
>> I think this is a better way than what I did in
>> 0001-Rellocated-the-codes-for-pg_backend_memory_contexts-.patch.
>
> You mean
> 0001-Restrict-the-access-to-pg_backend_memory_contexts-to.patch?
Oops, I meant
0001-Restrict-the-access-to-pg_backend_memory_contexts-to.patch.
>
>
>> Attached a patch.
>
> Thanks for updating the patch! This also looks good to me.
>
>
>>> It seems to me that you are missing one "REVOKE ALL on
>>> pg_backend_memory_contexts FROM PUBLIC" in patch 0003.
>>>
>>> By the way, if that was just for me, I would remove used_bytes, which
>>> is just a computation from the total and free numbers. I'll defer
>>> that point to Fujii-san.
>
> Yeah, I was just thinking that displaying also used_bytes was useful,
> but this might be inconsistent with the other views' ways.
>
> Regards,
Regards,
--
Atsushi Torikoshi
NTT DATA CORPORATION
Attachments:0002-Added-a-regression-test-for-pg_backend_memory_contex.patch (text/x-diff, 1.9K) download
From 335b9eb0c60a7f12debd4c45d435888109b2bfcf Mon Sep 17 00:00:00 2001
From: Atsushi Torikoshi <torikoshia@oss.nttdata.com>
Date: Mon, 24 Aug 2020 21:28:20 +0900
Subject: [PATCH] Added a regression test for pg_backend_memory_contexts.
---
src/test/regress/expected/sysviews.out | 9 +++++++++
src/test/regress/sql/sysviews.sql | 5 +++++
2 files changed, 14 insertions(+)
diff --git a/src/test/regress/expected/sysviews.out b/src/test/regress/expected/sysviews.outindex 06c4c3e476..1cffc3349d 100644--- a/src/test/regress/expected/sysviews.out+++ b/src/test/regress/expected/sysviews.out@@ -19,6 +19,15 @@ select count(*) >= 0 as ok from pg_available_extensions;
t
(1 row)
+-- The entire output of pg_backend_memory_contexts is not stable,+-- we test only the existance and basic condition of TopMemoryContext.+select name, ident, parent, level, total_bytes >= free_bytes+ from pg_backend_memory_contexts where level = 0;+ name | ident | parent | level | ?column? +------------------+-------+--------+-------+----------+ TopMemoryContext | | | 0 | t+(1 row)+
-- At introduction, pg_config had 23 entries; it may grow
select count(*) > 20 as ok from pg_config;
ok
diff --git a/src/test/regress/sql/sysviews.sql b/src/test/regress/sql/sysviews.sqlindex 28e412b735..ac4a0e1cbb 100644--- a/src/test/regress/sql/sysviews.sql+++ b/src/test/regress/sql/sysviews.sql@@ -12,6 +12,11 @@ select count(*) >= 0 as ok from pg_available_extension_versions;
select count(*) >= 0 as ok from pg_available_extensions;
+-- The entire output of pg_backend_memory_contexts is not stable,+-- we test only the existance and basic condition of TopMemoryContext.+select name, ident, parent, level, total_bytes >= free_bytes+ from pg_backend_memory_contexts where level = 0;+
-- At introduction, pg_config had 23 entries; it may grow
select count(*) > 20 as ok from pg_config;
--
2.18.1
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, masao.fujii@oss.nttdata.com, michael@paquier.xyz, kasahara.tatsuhito@gmail.com, andres@anarazel.de, tgl@sss.pgh.pa.us, robertmhaas@gmail.com
Subject: Re: Creating a function for exposing memory usage of backend process
In-Reply-To: <ae118b75ede9327c6dc8e9e457639dc9@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