MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8;
 format=flowed
Content-Transfer-Encoding: 8bit
Date: Wed, 16 Apr 2025 23:19:59 +0900
From: torikoshia <torikoshia@oss.nttdata.com>
To: Daniel Gustafsson <daniel@yesql.se>, dgrowleyml@gmail.com
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Align memory context level numbering in
 pg_log_backend_memory_contexts()
In-Reply-To: <81B10568-B4A1-46C0-9809-2F8E06AB8C7B@yesql.se>
References: <395ea5d4fe190480efa95bf533485c70@oss.nttdata.com>
 <CAApHDvpE6JS9F5kg+W4+p9TsiqFsPE+6XqEtD0P0HKNBoa9tYA@mail.gmail.com>
 <81B10568-B4A1-46C0-9809-2F8E06AB8C7B@yesql.se>
User-Agent: Roundcube Webmail/1.4.11
Message-ID: <f56063dfad48f5121e5ab733b36ec46d@oss.nttdata.com>
Archived-At: 
 <https://www.postgresql.org/message-id/f56063dfad48f5121e5ab733b36ec46d%40oss.nttdata.com>
Precedence: bulk

On 2025-04-16 06:18, Daniel Gustafsson wrote:
>> On 15 Apr 2025, at 23:03, David Rowley <dgrowleyml@gmail.com> wrote:
> 
>> My vote is to make the levels 1-based in all locations where we output
>> the context information.
> 
> I agree with this, pg_get_process_memory_contexts() also use 1-based 
> levels
> fwiw.

+1.
I believe there's no particular issue with starting the level from 1 in 
pg_log_backend_memory_contexts().

Regarding the implementation:
In the initial patch attached, I naïvely incremented the level just 
before emitting the log line.
However, it might be cleaner to simply initialize the level variable to 
1 from the start. This could help avoid unnecessary confusion when 
debugging that part of the code.

Similarly, I noticed that in pg_get_process_memory_contexts(), the level 
is initialized to 0 in ProcessGetMemoryContextInterrupt(void):

     int level = 0;
     ..
     MemoryContextStatsInternal(c, level, 100, 100, &grand_totals, ..

If we want to be consistent, perhaps it would make sense to start from 1 
there as well.

BTW level variable has existed since before pg_backend_memory_contexts 
was introduced — it was originally used for functions that help inspect 
memory contexts via the debugger. Because of that, I think changing this 
would affect not only these functions codes but some older ones.


--
Atsushi Torikoshi
Seconded from NTT DATA GROUP CORPORATION to SRA OSS K.K.