Subject: Re: Align memory context level numbering in pg_log_backend_memory_contexts()

From: torikoshia <[email-masked]>
Date: Wed, 16 Apr 2025 23:19:59 +0900

On 2025-04-16 06:18, Daniel Gustafsson wrote:
>> On 15 Apr 2025, at 23:03, David Rowley <[email-masked]> 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.

---

From: Rahila Syed <[email-masked]>
Date: Thu, 17 Apr 2025 17:55:04 +0530

Hi,

> The attached patch is how I think we should do it.
>

Thank you for the patch.
I tested this patch and it works fine. I agree with the changes made in it.

Regarding v2 patch,
-                       int                     level = 0;

Retaining the level variable will enhance the code readability, IMO.

Thank you,
Rahila Syed

---

From: torikoshia <[email-masked]>
Date: Thu, 17 Apr 2025 23:27:42 +0900

Thanks for your review, Melih and Rahila.

> Retaining the level variable will enhance the code readability, IMO.

As for the level variable, this change comes from the v1 patch, and I 
don't have a strong opinion about it.
However, if we decide to keep the level variable here, it might be more 
consistent to also define it in MemoryContextStatsDetail().

-- 
Regards,

---

From: David Rowley <[email-masked]>
Date: Fri, 18 Apr 2025 09:11:38 +1200

> Retaining the level variable will enhance the code readability, IMO.

When I read that, I suspected it might have been leftover from a
refactor during the development that was forgotten about. There'd be
thousands of places in our code base that you could make the
readability argument for, including the max_level and max_children
parameters at the same call-site. But those didn't get the same
treatment.

I've now pushed the latest patch. Thanks for the reviews.

David

---

From: Fujii Masao <[email-masked]>
Date: Fri, 18 Apr 2025 17:54:04 +0900

> I've now pushed the latest patch. Thanks for the reviews.

Shouldn't the example output of pg_log_backend_memory_contexts() in
the documentation also be updated to use 1-based numbering for consistency?
Patch attached.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
From 7b3aa9de467005e540b2246ed89c9e43fe08f6bd Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email-masked]>
Date: Fri, 18 Apr 2025 17:45:29 +0900
Subject: [PATCH v1] doc: Fix memory context level in
 pg_log_backend_memory_contexts() example.

Commit d9e03864b6b changed the memory context level numbers shown by
pg_log_backend_memory_contexts() to be 1-based. However, the example in
the documentation was not updated and still used 0-based numbering.

This commit updates the example to match the current 1-based output.
---
 doc/src/sgml/func.sgml | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 1c5cfee25d1..574a544d9fa 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -28922,16 +28922,16 @@ One message for each memory context will be logged. For example:
 <screen>
 LOG:  logging memory contexts of PID 10377
 STATEMENT:  SELECT pg_log_backend_memory_contexts(pg_backend_pid());
-LOG:  level: 0; TopMemoryContext: 80800 total in 6 blocks; 14432 free (5 chunks); 66368 used
-LOG:  level: 1; pgstat TabStatusArray lookup hash table: 8192 total in 1 blocks; 1408 free (0 chunks); 6784 used
-LOG:  level: 1; TopTransactionContext: 8192 total in 1 blocks; 7720 free (1 chunks); 472 used
-LOG:  level: 1; RowDescriptionContext: 8192 total in 1 blocks; 6880 free (0 chunks); 1312 used
-LOG:  level: 1; MessageContext: 16384 total in 2 blocks; 5152 free (0 chunks); 11232 used
-LOG:  level: 1; Operator class cache: 8192 total in 1 blocks; 512 free (0 chunks); 7680 used
-LOG:  level: 1; smgr relation table: 16384 total in 2 blocks; 4544 free (3 chunks); 11840 used
-LOG:  level: 1; TransactionAbortContext: 32768 total in 1 blocks; 32504 free (0 chunks); 264 used
+LOG:  level: 1; TopMemoryContext: 80800 total in 6 blocks; 14432 free (5 chunks); 66368 used
+LOG:  level: 2; pgstat TabStatusArray lookup hash table: 8192 total in 1 blocks; 1408 free (0 chunks); 6784 used
+LOG:  level: 2; TopTransactionContext: 8192 total in 1 blocks; 7720 free (1 chunks); 472 used
+LOG:  level: 2; RowDescriptionContext: 8192 total in 1 blocks; 6880 free (0 chunks); 1312 used
+LOG:  level: 2; MessageContext: 16384 total in 2 blocks; 5152 free (0 chunks); 11232 used
+LOG:  level: 2; Operator class cache: 8192 total in 1 blocks; 512 free (0 chunks); 7680 used
+LOG:  level: 2; smgr relation table: 16384 total in 2 blocks; 4544 free (3 chunks); 11840 used
+LOG:  level: 2; TransactionAbortContext: 32768 total in 1 blocks; 32504 free (0 chunks); 264 used
 ...
-LOG:  level: 1; ErrorContext: 8192 total in 1 blocks; 7928 free (3 chunks); 264 used
+LOG:  level: 2; ErrorContext: 8192 total in 1 blocks; 7928 free (3 chunks); 264 used
 LOG:  Grand total: 1651920 bytes in 201 blocks; 622360 free (88 chunks); 1029560 used
 </screen>
     If there are more than 100 child contexts under the same parent, the first
-- 
2.49.0

---

From: David Rowley <[email-masked]>
Date: Fri, 18 Apr 2025 21:23:03 +1200

> the documentation also be updated to use 1-based numbering for consistency?

Yeah. I failed to notice we had an example of the output.

Want to take care of it?

David

---

From: Fujii Masao <[email-masked]>
Date: Fri, 18 Apr 2025 18:45:39 +0900

> Yeah. I failed to notice we had an example of the output.

Yeah, I will if you're okay with that!

Regards,

---

From: Fujii Masao <[email-masked]>
Date: Mon, 21 Apr 2025 14:59:28 +0900

> Yeah, I will if you're okay with that!

Pushed. Thanks!

Regards,