From: Fujii Masao <masao.fujii@oss.nttdata.com>
To: torikoshia <torikoshia@oss.nttdata.com>
Cc: pgsql-hackers@postgresql.org
Subject: Re: Creating a function for exposing memory usage of backend process
Date: Wed, 1 Jul 2020 16:43:42 +0900
Message-ID: <e068e42d-8eda-fbe5-e84c-7188e9386778@oss.nttdata.com> (raw)
In-Reply-To: <6c828f3a1d44dba81999fc7990b8f1c5@oss.nttdata.com>
References: <72a656e0f71d0860161e0b3f67e4d771@oss.nttdata.com>
<398eb7cd-5f75-be1c-3897-36865b7bfabf@oss.nttdata.com>
<CA+TgmobkpFV0UB67kzXuD36--OFHwz1bs=L_6PZbD4nxKqUQMw@mail.gmail.com>
<3193c7d590f3212e543b63df977da758@oss.nttdata.com>
<78268b5f-53ed-4645-1abb-23c1030cce0a@oss.nttdata.com>
<6c828f3a1d44dba81999fc7990b8f1c5@oss.nttdata.com>
On 2020/07/01 14:48, torikoshia wrote:
> On Mon, Jun 29, 2020 at 3:13 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>
>> Could you add this patch to Commitfest 2020-07?
>
> Thanks for notifying, I've added it.
> BTW, I registered you as an author because this patch used
> lots of pg_cheat_funcs' codes.
>
> https://commitfest.postgresql.org/28/2622/
Thanks!
>
>> This patch provides only the function, but isn't it convenient to
>> provide the view like pg_shmem_allocations?
>
>> Sounds good. But isn't it better to document each column?
>> Otherwise, users cannot undersntad what "ident" column indicates.
>
> Agreed.
> Attached a patch for creating a view for local memory context
> and its explanation on the document.
Thanks for updating the patch!
You treat pg_stat_local_memory_contexts view as a dynamic statistics view.
But isn't it better to treat it as just system view like pg_shmem_allocations
or pg_prepared_statements because it's not statistics information? If yes,
we would need to rename the view, move the documentation from
monitoring.sgml to catalogs.sgml, and move the code from pgstat.c to
the more appropriate source file.
+ tupdesc = rsinfo->setDesc;
+ tupstore = rsinfo->setResult;
These seem not to be necessary.
+ /*
+ * It seems preferable to label dynahash contexts with just the hash table
+ * name. Those are already unique enough, so the "dynahash" part isn't
+ * very helpful, and this way is more consistent with pre-v11 practice.
+ */
+ if (ident && strcmp(name, "dynahash") == 0)
+ {
+ name = ident;
+ ident = NULL;
+ }
IMO it seems better to report both name and ident even in the case of
dynahash than report only ident (as name). We can easily understand
the context is used for dynahash when it's reported. But you think it's
better to report NULL rather than "dynahash"?
+/* ----------
+ * The max bytes for showing identifiers of MemoryContext.
+ * ----------
+ */
+#define MEMORY_CONTEXT_IDENT_SIZE 1024
Do we really need this upper size limit? Could you tell me why
this is necessary?
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
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: masao.fujii@oss.nttdata.com, torikoshia@oss.nttdata.com
Subject: Re: Creating a function for exposing memory usage of backend process
In-Reply-To: <e068e42d-8eda-fbe5-e84c-7188e9386778@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