Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1rQkRo-003Jq9-Nm for pgsql-hackers@arkaria.postgresql.org; Fri, 19 Jan 2024 08:41:57 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.94.2) (envelope-from ) id 1rQkRn-0087jc-98 for pgsql-hackers@arkaria.postgresql.org; Fri, 19 Jan 2024 08:41:55 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1rQkRm-0087jT-W0 for pgsql-hackers@lists.postgresql.org; Fri, 19 Jan 2024 08:41:54 +0000 Received: from oss.nttdata.com ([49.212.34.109]) by makus.postgresql.org with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1rQkRj-002HzZ-GT for pgsql-hackers@lists.postgresql.org; Fri, 19 Jan 2024 08:41:53 +0000 Received: from oss.nttdata.com (localhost [127.0.0.1]) by oss.nttdata.com (Postfix) with ESMTPA id D7C6A6059B; Fri, 19 Jan 2024 17:41:45 +0900 (JST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at oss.nttdata.com MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Date: Fri, 19 Jan 2024 17:41:45 +0900 From: torikoshia To: Melih Mutlu Cc: Andres Freund , Stephen Frost , PostgreSQL Hackers Subject: Re: Parent/child context relation in pg_get_backend_memory_contexts() In-Reply-To: References: <20231012162309.4wmc2nb5tkt6pzml@awork3.anarazel.de> <2f34c14847fc03752f0bca7d97aeae27@oss.nttdata.com> User-Agent: Roundcube Webmail/1.4.11 Message-ID: <18d210bb7010f87cdb0019fa846d45ea@oss.nttdata.com> X-Sender: torikoshia@oss.nttdata.com List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk On 2024-01-16 18:41, Melih Mutlu wrote: > Hi, > > Thanks for reviewing. > > torikoshia , 10 Oca 2024 Çar, 09:37 > tarihinde şunu yazdı: > >>> + >>> + >> role="column_definition"> >>> + context_id int4 >>> + >>> + >>> + Current context id. Note that the context id is a >> temporary id >>> and may >>> + change in each invocation >>> + >>> + >>> + >>> + >>> + >> role="column_definition"> >>> + path int4[] >>> + >>> + >>> + Path to reach the current context from TopMemoryContext. >>> Context ids in >>> + this list represents all parents of the current context. >> This >>> can be >>> + used to build the parent and child relation >>> + >>> + >>> + >>> + >>> + >> role="column_definition"> >>> + total_bytes_including_children >>> int8 >>> + >>> + >>> + Total bytes allocated for this memory context including >> its >>> children >>> + >>> + >> >> These columns are currently added to the bottom of the table, but it >> may >> be better to put semantically similar items close together and >> change >> the insertion position with reference to other system views. For >> example, >> >> - In pg_group and pg_user, 'id' is placed on the line following >> 'name', >> so 'context_id' be placed on the line following 'name' >> - 'path' is similar with 'parent' and 'level' in that these are >> information about the location of the context, 'path' be placed to >> next >> to them. >> >> If we do this, orders of columns in the system view should be the >> same, >> I think. > > I've done what you suggested. Also moved > "total_bytes_including_children" right after "total_bytes". > >> 14dd0f27d have introduced new macro foreach_int. >> It seems to be able to make the code a bit simpler and the commit >> log >> says this macro is primarily intended for use in new code. For >> example: > > Makes sense. Done. Thanks for updating the patch! > + Current context id. Note that the context id is a temporary id > and may > + change in each invocation > + > + It clearly states that the context id is temporary, but I am a little concerned about users who write queries that refer to this view multiple times without using CTE. If you agree, how about adding some description like below you mentioned before? > We still need to use cte since ids are not persisted and might change > in > each run of pg_backend_memory_contexts. Materializing the result can > prevent any inconsistencies due to id change. Also it can be even good > for > performance reasons as well. We already have additional description below the table which explains each column of the system view. For example pg_locks: https://www.postgresql.org/docs/devel/view-pg-locks.html Also giving an example query something like this might be useful. -- show all the parent context names of ExecutorState with contexts as ( select * from pg_backend_memory_contexts ) select name from contexts where array[context_id] <@ (select path from contexts where name = 'ExecutorState'); -- Regards, -- Atsushi Torikoshi NTT DATA Group Corporation